fix: set the focus starting point without a fragment navigation

.changeset/focus-starting-point.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: set the focus starting point without a fragment navigation, which leaked a `hashchange` to app listeners

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

@@ -13,7 +13,7 @@ import { decode_pathname, strip_hash, make_trackable, normalize_path } from '../
 import { dev_fetch, initial_fetch, lock_fetch, subsequent_fetch, unlock_fetch } from './fetcher.js';
 import { parse_routes, parse_server_route } from './parse.js';
 import * as storage from './session-storage.js';
-import { blur_active_element, is_resetting_focus, reset_focus } from './focus.js';
+import { blur_active_element, reset_focus } from './focus.js';
 import { disable_scroll_handling, restore_scroll } from './scroll.js';
 import {
 	find_anchor,
@@ -2265,9 +2265,9 @@ async function finish_navigation(nav, nav_token, url, popped_scroll, reset, upda
 		return false;
 	}
 
-	const deep_linked = restore_scroll(url, reset, popped_scroll);
+	restore_scroll(url, reset, popped_scroll);
 	if (reset && document.activeElement === document.body) {
-		reset_focus(url, !deep_linked);
+		reset_focus(url);
 	}
 
 	is_navigating = false;
@@ -3341,8 +3341,6 @@ function _start_router() {
 	});
 
 	addEventListener('popstate', async (event) => {
-		if (is_resetting_focus()) return;
-
 		const history_metadata = get_history_metadata(event.state);
 
 		if (history_metadata?.historyIndex) {

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

@@ -1,15 +1,5 @@
 import { hash_routing } from '$app/paths/internal/client';
-import { get_hash_element, scroll_state } from './utils.js';
-
-/**
- * This flag is used to avoid client-side navigation when we're only using
- * `location.replace()` to set focus.
- */
-let resetting_focus = false;
-
-export function is_resetting_focus() {
-	return resetting_focus;
-}
+import { get_hash_element } from './utils.js';
 
 /**
  * Blurs the active element before the DOM update when a navigation resets focus, so that
@@ -29,59 +19,35 @@ export function blur_active_element(reset) {
 }
 
 /**
- * @param {URL} url
- * @param {boolean} [scroll]
+ * Sets the sequential focus navigation starting point to `element` without leaving it focused
+ * @param {Element} element
  */
-export function reset_focus(url, scroll = true) {
+function focus_element(element) {
+	const tabindex = element.getAttribute('tabindex');
+
+	element.setAttribute('tabindex', '-1');
+	/** @type {HTMLElement} */ (element).focus({ preventScroll: true, focusVisible: false });
+
+	// removing `tabindex` blurs it again, synchronously in Chromium and a frame later elsewhere
+	if (tabindex !== null) {
+		element.setAttribute('tabindex', tabindex);
+	} else {
+		element.removeAttribute('tabindex');
+	}
+}
+
+/** @param {URL} url */
+export function reset_focus(url) {
 	const autofocus = document.querySelector('[autofocus]');
 	if (autofocus) {
 		// @ts-ignore
 		autofocus.focus();
 	} else {
-		// Reset page selection and focus
-
-		// Mimic the browsers' behaviour and set the sequential focus navigation
-		// starting point to the fragment identifier.
-		const element = get_hash_element(url, hash_routing);
-		if (element) {
-			const { x, y } = scroll_state();
-
-			// focusing a non-focusable element is a no-op, so navigate to the fragment
-			// instead; see sveltejs/kit#16982 for the tabindex alternative
-			setTimeout(() => {
-				const history_state = history.state;
-
-				resetting_focus = true;
-				location.replace(new URL(`#${element.id}`, location.href));
-
-				// a fragment navigation nulls `history.state` (per spec; WebKit keeps it), so
-				// restore it. This also restores the original hash if we're using hash routing
-				history.replaceState(history_state, '', url);
-
-				// If scroll management has already happened earlier, we need to restore
-				// the scroll position after setting the sequential focus navigation starting point
-				if (scroll) scrollTo(x, y);
-				resetting_focus = false;
-			});
-		} else {
-			// If the ID doesn't exist, we try to mimic browsers' behaviour as closely
-			// as possible by targeting the first scrollable region. 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;
-			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');
-			}
-		}
+		// set the sequential focus navigation starting point to the fragment identifier, or to
+		// the first scrollable region when there is none. Not a perfect match for browsers:
+		// 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
+		focus_element(get_hash_element(url, hash_routing) ?? document.body);
 
 		// capture current selection, so we can compare the state after
 		// snapshot restoration and afterNavigate callbacks have run

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

@@ -1,15 +1,10 @@
-import { afterEach, beforeEach, expect, test, vi } from 'vitest';
+import { beforeEach, expect, test } from 'vitest';
 import { blur_active_element, reset_focus } from './focus.js';
 
 beforeEach(() => {
-	window.scrollTo = vi.fn();
 	document.body.innerHTML = '';
 });
 
-afterEach(() => {
-	vi.useRealTimers();
-});
-
 test('blur_active_element blurs a focused SVG element', () => {
 	document.body.innerHTML = '<svg tabindex="0"></svg>';
 	const svg = /** @type {SVGElement} */ (document.body.firstElementChild);
@@ -28,12 +23,10 @@ test('reset_focus focuses the body without leaving a tabindex behind', () => {
 	expect(document.body.hasAttribute('tabindex')).toBe(false);
 });
 
-test('reset_focus restores the scroll position after jumping to the hash target', () => {
-	vi.useFakeTimers();
-	Object.defineProperty(window, 'pageYOffset', { value: 400, configurable: true });
-	document.body.innerHTML = '<div id="a"></div>';
+test("reset_focus restores the hash target's tabindex", () => {
+	document.body.innerHTML = '<p id="a" tabindex="0"></p><p id="b"></p>';
 	reset_focus(new URL('/#a', location.href));
-	expect(window.scrollTo).not.toHaveBeenCalled();
-	vi.runAllTimers();
-	expect(window.scrollTo).toHaveBeenCalledWith(0, 400);
+	expect(document.getElementById('a')?.getAttribute('tabindex')).toBe('0');
+	reset_focus(new URL('/#b', location.href));
+	expect(document.getElementById('b')?.hasAttribute('tabindex')).toBe(false);
 });

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

@@ -14,23 +14,20 @@ export function disable_scroll_handling() {
  * @param {URL} url
  * @param {boolean} reset
  * @param {{ x: number; y: number } | null | undefined} popped_scroll
- * @returns {Element | null} the hash target, when that is what was scrolled into view
  */
 export function restore_scroll(url, reset, popped_scroll) {
-	/** @type {Element | null} */
-	let deep_linked = null;
-
 	if (reset && autoscroll) {
 		if (popped_scroll) {
 			scrollTo(popped_scroll.x, popped_scroll.y);
-		} else if ((deep_linked = get_hash_element(url, hash_routing))) {
-			deep_linked.scrollIntoView();
 		} else {
-			scrollTo(0, 0);
+			const element = get_hash_element(url, hash_routing);
+			if (element) {
+				element.scrollIntoView();
+			} else {
+				scrollTo(0, 0);
+			}
 		}
 	}
 
 	autoscroll = true;
-
-	return deep_linked;
 }

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

@@ -11,7 +11,7 @@ beforeEach(() => {
 
 test('restores a popped position ahead of the hash target', () => {
 	document.body.innerHTML = '<div id="a"></div>';
-	expect(restore_scroll(new URL('/#a', location.href), true, { x: 10, y: 20 })).toBe(null);
+	restore_scroll(new URL('/#a', location.href), true, { x: 10, y: 20 });
 	expect(window.scrollTo).toHaveBeenCalledWith(10, 20);
 	expect(Element.prototype.scrollIntoView).not.toHaveBeenCalled();
 });

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

@@ -109,7 +109,7 @@
 		await expect(page.locator('#input')).toBeFocused();
 	});
 
 	test('autofocus from previous page is ignored', async ({ page, clicknav }) => {
 		await page.addInitScript(`
 			window.active = null;
 			window.addEventListener('focusin', () => window.active = document.activeElement);
@@ -697,7 +697,7 @@
 		}
 	});
 
 	test('Scroll position is correct after going back from a shallow route', async ({
 		page,
 		scroll_to
 	}) => {
@@ -868,7 +868,7 @@
 		}
 	});
 
 	test('prefetches code programmatically with a dynamic route id', async ({ page, app }) => {
 		await page.goto('/routing/a');
 
 		await app.preloadCode('/routing/[slug]');
@@ -1179,7 +1179,7 @@
 		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 expect(page.locator('body')).toBeFocused();
 		await page.keyboard.press(tab);
 		await expect(page.locator('#button3')).toBeFocused();
 	});

packages/kit/test/apps/hash-based-routing/test/test.js

@@ -119,12 +119,14 @@ test.describe('hash based navigation', () => {
 	test('sequential focus navigation point is set correctly', async ({ page, browserName }) => {
 		const tab = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';
 		await page.goto('/#/focus');
+		await page.evaluate("addEventListener('hashchange', () => (window.hashchanged = true))");
 		await page.locator('a[href="#/focus/a#p"]').click();
 		await page.waitForURL('#/focus/a#p');
-		expect(await page.evaluate(() => (document.activeElement || {}).nodeName)).toBe('BODY');
+		await expect(page.locator('body')).toBeFocused();
 		await page.keyboard.press(tab);
 		await expect(page.locator('#button3')).toBeFocused();
 		await expect(page.locator('button[id="button3"]')).toBeFocused();
+		expect(await page.evaluate('window.hashchanged')).toBe(undefined);
 	});
 
 	test('does not look up an empty anchor id on navigation', async ({ page }) => {