fix: hash link to new page focuses the correct element (#10856)

* use window.location to scroll to deep linked element

* changeset

* prettier

* fix

* fix scroll position being changed

* prettier

* found a fix that works!

* improve some old related tests

* forgot to restore scroll pos

Author: eltigerchino

.changeset/big-pants-peel.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: correctly set sequential focus navigation starting point after navigation

packages/kit/src/runtime/client/client.js

@@ -2800,22 +2800,42 @@ function reset_focus() {
 		autofocus.focus();
 	} else {
 		// Reset page selection and focus
-		// We try to mimic browsers' behaviour as closely as possible by targeting the
-		// first scrollable region, but unfortunately it's not a perfect match — e.g.
-		// shift-tabbing won't immediately cycle up from the end of the page on Chromium
-		// See https://html.spec.whatwg.org/multipage/interaction.html#get-the-focusable-area
-		const root = document.body;
-		const tabindex = root.getAttribute('tabindex');
-
-		root.tabIndex = -1;
-		// @ts-expect-error
-		root.focus({ preventScroll: true, focusVisible: false });
+		if (location.hash && document.querySelector(location.hash)) {
+			const { x, y } = scroll_state();
 
-		// restore `tabindex` as to prevent `root` from stealing input from elements
-		if (tabindex !== null) {
-			root.setAttribute('tabindex', tabindex);
+			setTimeout(() => {
+				const history_state = history.state;
+				// Mimic the browsers' behaviour and set the sequential focus navigation
+				// starting point to the fragment identifier
+				location.replace(location.hash);
+				// but Firefox has a bug that sets the history state as null so we
+				// need to restore the history state
+				// See https://bugzilla.mozilla.org/show_bug.cgi?id=1199924
+				history.replaceState(history_state, '', location.hash);
+
+				// Scroll management has already happened earlier so we need to restore
+				// the scroll position after setting the sequential focus navigation starting point
+				scrollTo(x, y);
+			});
 		} else {
-			root.removeAttribute('tabindex');
+			// We try to mimic browsers' behaviour as closely as possible by targeting the
+			// first scrollable region, but unfortunately it's not a perfect match — e.g.
+			// shift-tabbing won't immediately cycle up from the end of the page on Chromium
+			// See https://html.spec.whatwg.org/multipage/interaction.html#get-the-focusable-area
+			const root = document.body;
+			const tabindex = root.getAttribute('tabindex');
+
+			root.tabIndex = -1;
+			// @ts-expect-error options.focusVisible is only supported in Firefox
+			// See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#browser_compatibility
+			root.focus({ preventScroll: true, focusVisible: false });
+
+			// restore `tabindex` as to prevent `root` from stealing input from elements
+			if (tabindex !== null) {
+				root.setAttribute('tabindex', tabindex);
+			} else {
+				root.removeAttribute('tabindex');
+			}
 		}
 
 		// capture current selection, so we can compare the state after

packages/kit/test/apps/basics/src/routes/routing/focus/+page.svelte

@@ -0,0 +1 @@
+<a href="/routing/focus/a#p">click me!</a>

packages/kit/test/apps/basics/src/routes/routing/focus/a/+page.svelte

@@ -0,0 +1,4 @@
+<button>button 1</button>
+<button>button 2</button>
+<p id="p">cannot be focused</p>
+<button id="button3">button 3</button>

packages/kit/test/apps/basics/test/cross-platform/client.test.js

@@ -806,14 +806,31 @@ test.describe('Routing', () => {
 		await expect(page.getByRole('textbox')).toBeVisible();
 	});
 
-	test('back button returns to previous route when previous route has been navigated to via hash anchor', async ({
+	test('sequential focus navigation starting point is set correctly on navigation', async ({
 		page,
-		clicknav
+		browserName
+	}) => {
+		const tab = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';
+		await page.goto('/routing/focus');
+		await page.locator('[href="/routing/focus/a#p"]').click();
+		await page.waitForURL('**/routing/focus/a#p');
+		expect(await page.evaluate(() => (document.activeElement || {}).nodeName)).toBe('BODY');
+		await page.keyboard.press(tab);
+		await expect(page.locator('#button3')).toBeFocused();
+	});
+
+	test('back button returns to previous route when previous route was navigated to via hash anchor', async ({
+		page,
+		clicknav,
+		baseURL
 	}) => {
 		await page.goto('/routing/hashes/a');
 
 		await page.locator('[href="#hash-target"]').click();
+		expect(page.url()).toBe(`${baseURL}/routing/hashes/a#hash-target`);
+
 		await clicknav('[href="/routing/hashes/b"]');
+		expect(await page.textContent('h1')).toBe('b');
 
 		await expect(page.locator('h1')).toHaveText('b');
 		await page.goBack();
@@ -828,10 +845,13 @@ test.describe('Routing', () => {
 		await page.goto('/routing/hashes/a');
 
 		await clicknav('[href="#hash-target"]');
+		expect(page.url()).toBe(`${baseURL}/routing/hashes/a#hash-target`);
+
 		await clicknav('[href="#replace-state"]');
+		expect(page.url()).toBe(`${baseURL}/routing/hashes/a#replace-state`);
 
 		await page.goBack();
-		expect(await page.url()).toBe(`${baseURL}/routing/hashes/a`);
+		expect(page.url()).toBe(`${baseURL}/routing/hashes/a`);
 	});
 
 	test('does not normalize external path', async ({ page, start_server }) => {