Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Don’t prevent the browser’s default behavior corresponding to the Alt key #1372

Merged
merged 1 commit into from
Aug 29, 2020
Merged

Don’t prevent the browser’s default behavior corresponding to the Alt key #1372

merged 1 commit into from
Aug 29, 2020

Conversation

yuheiy
Copy link
Contributor

@yuheiy yuheiy commented Aug 8, 2020

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

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR relates to an outstanding issue, so please reference it in your PR, or create an explanatory one for discussion. In many cases features are absent for a reason.
  • This message body should clearly illustrate what problems it solves. If there are related issues, remember to reference them.
  • Ideally, include a test that fails without this PR but passes with it. PRs will only be merged once they pass CI. (Remember to npm run lint!)

Tests

  • Run the tests tests with npm test or yarn test)

… key

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                |
@benmccann
Copy link
Member

It might be a good idea to submit this PR to page.js as well: https://github.com/visionmedia/page.js/blob/master/page.js#L748

  • they're more familiar with this block of code and it would give confidence that this is the correct fix
  • it would help us keep this block of code in sync with the upstream version

@yuheiy
Copy link
Contributor Author

yuheiy commented Aug 9, 2020

I’ve already submitted. visionmedia/page.js#568

@Conduitry
Copy link
Member

Does the "No impact" for IE 11 mean that typically alt-clicking a link performs regular navigation, and that with this change we'd no longer be intercepting these clicks and it would perform a regular page navigation instead of an SPA-type navigation?

Even if so, I think it's probably overall a good idea to make this change - I just want to be clear on what its effect is.

@yuheiy
Copy link
Contributor Author

yuheiy commented Aug 9, 2020

That’s right. In IE 11, alt-clicking has the same behavior as when the user is not using alt.

@yuheiy
Copy link
Contributor Author

yuheiy commented Aug 9, 2020

So IE 11 performs a regular page navigation.

@benmccann benmccann merged commit 17470a4 into sveltejs:master Aug 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants