Skip to content

Revert "fix(router): fix a problem with router not responding to back… - #30320

Closed
jasonaden wants to merge 1 commit into
angular:masterfrom
jasonaden:rollback_30160
Closed

jasonaden wants to merge 1 commit into
angular:masterfrom
jasonaden:rollback_30160

Conversation

@jasonaden

Copy link
Copy Markdown
Contributor

… button (#30160)"

This reverts commit 3327bd8.

This is being reverted because of a test failure in Google.

The test navigates to a route, then navigates to the same route again, which in the router codebase falls into a path that essentially marks it to do nothing. Upon marking it to do nothing, we attempt to get the state right for the next navigation. The second navigation had been kicked off using a value from this.transitions, which doesn't actually have the previous urlAfterRedirects value.

So the next navigation they are performing in the test is hitting back, which goes to the root. Because they happened to start on root, then navigate twice to the same place, then specifically go back to the root again, the back operation does nothing because the router thinks it’s already there.

The fix to this should be ensuring Router.getTransition() method returns the most recent NavigationTransition after all changes have been applied to it (the value that lands in the subscribe on this.navigations).

@jasonaden
jasonaden requested a review from a team May 8, 2019 00:06

@kara kara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jasonaden jasonaden added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels May 8, 2019
@jasonaden

Copy link
Copy Markdown
Contributor Author

merge-assistance to get this synced quickly.

@alxhub alxhub closed this in 8ced321 May 8, 2019
alxhub pushed a commit that referenced this pull request May 8, 2019
jasonaden added a commit to jasonaden/angular that referenced this pull request May 8, 2019
alxhub pushed a commit that referenced this pull request May 15, 2019
alxhub pushed a commit that referenced this pull request May 15, 2019
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants