New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(router): If users are using the Alt key when clicking the router links, prioritize browser’s default behavior #38375
Conversation
Thanks @yuheiy, this is a really great summary of the behavior in each browser! Would you mind copying that summary in the PR description into the commit message? We try to make all the information immediately available there rather than having to try to find the associated PR after merging. Can you also run |
I’ve updated the commit message and added the golden! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: public-api
reviewed-for: fw-router
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-for: public-api
@atscott @AndrewKushnir if we don't consider |
Yea, we probably should mark it as internal. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-for: public-api
The addition of /** @nodoc */
in a recent commit has caused a conflict. Can you please rebase?
@petebacondarwin I rebased. |
Caretaker - merge-assistance: the broken test is unrelated to this PR. |
Sorry I was a bit to fast with the merge label. This change needs to go through presubmit against the internal Google repository. |
…links, prioritize browser’s default behavior In most browsers, clicking links with the Alt key has a special behavior, for example, Chrome downloads the target resource. As with other modifier keys, the router should stop the original navigation to avoid preventing the browser’s default behavior. When users click a link while holding the Alt key together, the browsers behave as follows. Windows 10: | Browser | Behavior | |:-----------|:--------------------------------------------| | Chrome 84 | Download the target resource | | Firefox 79 | Prevent navigation and therefore do nothing | | Edge 84 | Download the target resource | | IE 11 | No impact | macOS Catalina: | Browser | Behavior | |:-----------|:--------------------------------------------| | Chrome 84 | Download the target resource | | Firefox 79 | Prevent navigation and therefore do nothing | | Safari 13 | Download the target resource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Reviewed-for: public-api
…links, prioritize browser’s default behavior (#38375) In most browsers, clicking links with the Alt key has a special behavior, for example, Chrome downloads the target resource. As with other modifier keys, the router should stop the original navigation to avoid preventing the browser’s default behavior. When users click a link while holding the Alt key together, the browsers behave as follows. Windows 10: | Browser | Behavior | |:-----------|:--------------------------------------------| | Chrome 84 | Download the target resource | | Firefox 79 | Prevent navigation and therefore do nothing | | Edge 84 | Download the target resource | | IE 11 | No impact | macOS Catalina: | Browser | Behavior | |:-----------|:--------------------------------------------| | Chrome 84 | Download the target resource | | Firefox 79 | Prevent navigation and therefore do nothing | | Safari 13 | Download the target resource | PR Close #38375
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
When users click a link while holding the Alt key together, the browsers behave as follows.
Windows 10:
macOS Catalina:
The Alt key has no impact on IE, but on most other browsers it does. As with other modifier keys, the router should stop the original navigation to avoid preventing the browser’s default behavior.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
This doesn't have an associate test for a few reasons:
true
specifically for the meta key and simply expecttrue
back. This is akin to a change detector test