fix: ignore target=_blank links
Also some docs on how to know when native confirmation dialog appears fixes #8482
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
- [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
- [x] This message body should clearly illustrate what problems it solves.
- [x] Ideally, include a test that fails without this PR but passes with it.
Tests
- [x] Run the tests with
pnpm testand lint the project withpnpm lintandpnpm check
Changesets
- [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.
🦋 Changeset detected
Latest commit: 1ce625e50b944541a06fa368e18b72b3b611f2a0
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @sveltejs/kit | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
Are there other targets we should also be ignoring? I'm aware of _self and _blank and (I think) _top plus whatever other custom names you want to have if you're using frames. What do we want to have happen when someone uses target='_self'
I know we probably don't want to think too hard about people using frames, but is there another check here we could perform that would cover more cases a little better? (I'm not saying this because I'm already pretty sure there is one. I'm just asking.)
I thought about this very briefly, too. From reading https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target it sounds like we should just ignore _self because it's the default, and for _top and _parent we would have to somehow find out if there's window.parent - I'll adjust the code. Better safe than revisit this later.
Is this logic right? We want to skip beforeNavigate when links open in a new tab, which IIUC means this:
<!-- should skip beforeNavigate -->
<a href="/other" target="_blank">...</a>
<a href="/other" target="other">...</a>
<!-- should NOT skip beforeNavigate -->
<a href="/other" target="_self">...</a>
<a href="/other" target="_top">...</a>
<a href="/other" target="_parent">...</a>
Actually on a closer read, I think the only change we need is to respect user-defined targets
Oh right, I forgot about those, yeah the if condition needs to be adjusted