fix: hash link to new page focuses the correct element

[eltigerchino]

fixes #3770

As suggested by mrkishi and jasonlyu:

This PR replaces the current scrollIntoView() with window.location.replace to scroll to and focus the deep linked element correctly since location.replace is almost 1:1 behaviour with natively clicking a hash link in the browser.

https://stackblitz.com/edit/sveltejs-kit-template-default-pxskqg?file=src%2Froutes%2F%2Blayout.svelte

Note:
Firefox has a bug where the history.state is set to null if we use location.replace with a hash that's the same as the current url hash. We circumvent this by manually restoring the history state after the fact.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• 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
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

[changeset-bot]

🦋 Changeset detected

Latest commit: e73396f

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

[benmccann]

@s3812497 the tests seem to be having an issue on this PR. I reran them to check if it's a fluke, but lots are still failing

[eltigerchino]

Thanks for catching this. Sorry, looks like my test was a fluke. It doesn't seem to work on firefox and safari. Back to the drawing board...

[eltigerchino]

@benmccann turns out it was an implementation issue. The focus was getting reset to the body after scrolling and focusing to the fragment identifier. It should work correctly now.

[eltigerchino]

Great. Now Firefox is being inconsistent. We might have to go with option 1 or 2:

1. Make the target focusable, focus it and revert it. the downside is it would cause the focus ring to show up. And it's not the same as the normal hash link behaviour. But it doesn't create any dummy element.
2. Append a dummy element before the target element if the target element isn't focusable. focus it and remove it.

It would also trigger the focus, focusin, blur, focusout events, and possibly affect styles because of the extra HTML element...
...but it works in all three browsers.

[eltigerchino]

Found a fix(?) for the issue where Firefox sets the history state to null if we call location.replace but with the same hash as before. This is also a potential alternative to #10032

Here's the link to the bug in the Firefox issue tracker https://bugzilla.mozilla.org/show_bug.cgi?id=1199924

[benmccann]

Here's the link to the bug in the Firefox issue tracker https://bugzilla.mozilla.org/show_bug.cgi?id=1199924

9 years ago - so we probably shouldn't wait for them to fix this 😆 Although they did just fix a 25 yr old bug https://news.ycombinator.com/item?id=40431444

[Rich-Harris]

preview: https://svelte-dev-git-preview-kit-10856-svelte.vercel.app/

this is an automated message