chore: give scroll and focus their own state and tests

.changeset/blur-svg-focus.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: blur focused SVG elements before the DOM update on navigation

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

@@ -14,7 +14,7 @@ import { dev_fetch, initial_fetch, lock_fetch, subsequent_fetch, unlock_fetch }
 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 { disable_scroll_handling, reset_scroll_and_focus } from './scroll.js';
+import { disable_scroll_handling, restore_scroll } from './scroll.js';
 import {
 	find_anchor,
 	resolve_url,
@@ -540,15 +540,15 @@ async function _start(_app, _target, data) {
 	// if we reload the page, or Cmd-Shift-T back to it,
 	// recover scroll position
 	const scroll = history_info[current_history_index]?.scroll;
-	function restore_scroll() {
+	function restore_reload_scroll() {
 		if (scroll) {
 			history.scrollRestoration = 'manual';
 			scrollTo(scroll.x, scroll.y);
 		}
 	}
 
 	if (data) {
-		restore_scroll();
+		restore_reload_scroll();
 
 		await _hydrate(target, data);
 	} else {
@@ -560,7 +560,7 @@ async function _start(_app, _target, data) {
 			persist_state: history_metadata?.persistState ?? false
 		});
 
-		restore_scroll();
+		restore_reload_scroll();
 	}
 
 	_start_router();
@@ -2256,16 +2256,17 @@ async function run_on_navigate_callbacks(navigation) {
  * @param {Promise<void> | undefined} updated
  */
 async function finish_navigation(nav, nav_token, url, popped_scroll, reset, updated) {
-	const active_element = document.activeElement;
-
 	await updated;
 
 	if (navigation_token !== nav_token) {
 		nav.reject(new Error('navigation aborted'));
 		return false;
 	}
 
-	reset_scroll_and_focus(url, reset ? popped_scroll : scroll_state(), reset, active_element);
+	const deep_linked = restore_scroll(url, reset, popped_scroll);
+	if (reset && document.activeElement === document.body) {
+		reset_focus(url, !deep_linked);
+	}
 
 	is_navigating = false;
 

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

@@ -11,14 +11,20 @@ export function is_resetting_focus() {
 	return resetting_focus;
 }
 
-/** @param {boolean} reset */
+/**
+ * Blurs the active element before the DOM update when a navigation resets focus, so that
+ * blur/focusout handlers run while the outgoing component's data is still valid (#14575)
+ * @param {boolean} reset
+ */
 export function blur_active_element(reset) {
+	const element = document.activeElement;
+
 	if (
 		reset &&
-		document.activeElement instanceof HTMLElement &&
-		document.activeElement !== document.body
+		(element instanceof HTMLElement || element instanceof SVGElement) &&
+		element !== document.body
 	) {
-		document.activeElement.blur();
+		element.blur();
 	}
 }
 
@@ -40,17 +46,16 @@ export function reset_focus(url, scroll = true) {
 		if (element) {
 			const { x, y } = scroll_state();
 
-			// `element.focus()` doesn't work on Safari and Firefox Ubuntu so we need
-			// to use this hack with `location.replace()` instead.
+			// 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));
 
-				// Firefox has a bug that sets the history state to `null` so we need to
-				// restore it after. See https://bugzilla.mozilla.org/show_bug.cgi?id=1199924
-				// This is also needed to restore the original hash if we're using hash routing
+				// 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
@@ -97,8 +102,8 @@ export function reset_focus(url, scroll = true) {
 					const a = ranges[i];
 					const b = selection.getRangeAt(i);
 
-					// we need to do a deep comparison rather than just `a !== b` because
-					// Safari behaves differently to other browsers
+					// compare field by field: a range modified in place keeps its identity,
+					// and Safari before 17 returned a new Range object on every getRangeAt()
 					if (
 						a.commonAncestorContainer !== b.commonAncestorContainer ||
 						a.startContainer !== b.startContainer ||

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

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, expect, test, vi } from 'vitest';
-import { reset_focus } from './focus.js';
+import { blur_active_element, reset_focus } from './focus.js';
 
 beforeEach(() => {
 	window.scrollTo = vi.fn();
@@ -10,6 +10,16 @@ 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);
+	svg.focus();
+	expect(document.activeElement).toBe(svg);
+
+	blur_active_element(true);
+	expect(document.activeElement).toBe(document.body);
+});
+
 test('reset_focus focuses the body without leaving a tabindex behind', () => {
 	document.body.innerHTML = '<input>';
 	/** @type {HTMLInputElement} */ (document.body.firstElementChild).focus();

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

@@ -1,39 +1,36 @@
 import { hash_routing } from '$app/paths/internal/client';
-import { reset_focus } from './focus.js';
 import { get_hash_element } from './utils.js';
 
 let autoscroll = true;
 
+/** Disables scroll handling for the next `restore_scroll` */
 export function disable_scroll_handling() {
 	autoscroll = false;
 }
 
 /**
+ * After a navigation that resets, scrolls to `popped_scroll` ?? the hash target ?? the top,
+ * unless `disable_scroll_handling` was called since the last navigation
  * @param {URL} url
- * @param {{ x: number; y: number } | null | undefined} scroll
  * @param {boolean} reset
- * @param {Element | null} active_element
+ * @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 reset_scroll_and_focus(url, scroll, reset, active_element) {
+export function restore_scroll(url, reset, popped_scroll) {
 	/** @type {Element | null} */
 	let deep_linked = null;
 
-	if (autoscroll) {
-		if (scroll) {
-			scrollTo(scroll.x, scroll.y);
+	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 changed_focus =
-		document.activeElement !== active_element && document.activeElement !== document.body;
-
-	if (reset && !changed_focus) {
-		reset_focus(url, !deep_linked);
-	}
-
 	autoscroll = true;
+
+	return deep_linked;
 }

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

@@ -1,5 +1,5 @@
 import { beforeEach, expect, test, vi } from 'vitest';
-import { disable_scroll_handling, reset_scroll_and_focus } from './scroll.js';
+import { disable_scroll_handling, restore_scroll } from './scroll.js';
 
 const url = new URL('/', location.href);
 
@@ -11,20 +11,20 @@ beforeEach(() => {
 
 test('restores a popped position ahead of the hash target', () => {
 	document.body.innerHTML = '<div id="a"></div>';
-	reset_scroll_and_focus(new URL('/#a', location.href), { x: 10, y: 20 }, true, document.body);
+	expect(restore_scroll(new URL('/#a', location.href), true, { x: 10, y: 20 })).toBe(null);
 	expect(window.scrollTo).toHaveBeenCalledWith(10, 20);
 	expect(Element.prototype.scrollIntoView).not.toHaveBeenCalled();
 });
 
 test('disable_scroll_handling is consumed by the next navigation, reset or not', () => {
 	disable_scroll_handling();
-	reset_scroll_and_focus(url, null, true, document.body);
+	restore_scroll(url, true, null);
 	expect(window.scrollTo).not.toHaveBeenCalled();
-	reset_scroll_and_focus(url, null, true, document.body);
+	restore_scroll(url, true, null);
 	expect(window.scrollTo).toHaveBeenCalledTimes(1);
 
 	disable_scroll_handling();
-	reset_scroll_and_focus(url, null, false, document.body);
-	reset_scroll_and_focus(url, null, true, document.body);
+	restore_scroll(url, false, null);
+	restore_scroll(url, true, null);
 	expect(window.scrollTo).toHaveBeenCalledTimes(2);
 });

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

@@ -239,31 +239,14 @@ export function is_external_url(url, base, hash_routing) {
 }
 
 /**
- * @param {URL} url
- * @param {boolean} hash_routing
- * @returns {string}
- */
-export function get_id(url, hash_routing) {
-	let id;
-
-	if (hash_routing) {
-		const [, , second] = url.hash.split('#', 3);
-		id = second ?? '';
-	} else {
-		id = url.hash.slice(1);
-	}
-
-	return decodeURIComponent(id);
-}
-
-/**
+ * The element a URL's fragment points at, if any. Under hash routing the fragment sits after the route
  * @param {URL} url
  * @param {boolean} hash_routing
  * @returns {Element | null}
  */
 export function get_hash_element(url, hash_routing) {
-	const id = get_id(url, hash_routing);
-	return id ? document.getElementById(id) : null;
+	const id = hash_routing ? (url.hash.split('#', 3)[2] ?? '') : url.hash.slice(1);
+	return id ? document.getElementById(decodeURIComponent(id)) : null;
 }
 
 /** @type {Set<string> | null} */