From 2b735adaac8d17821fccef482a97f1c91c61b1dd Mon Sep 17 00:00:00 2001 From: Nic Polumeyv <162764842+Nic-Polumeyv@users.noreply.github.com> Date: Sun, 30 Aug 2026 23:19:35 -0400 Subject: [PATCH 1/2] chore: move scroll restoration and focus reset out of client.js --- packages/kit/src/runtime/client/client.js | 177 +----------------- packages/kit/src/runtime/client/focus.js | 120 ++++++++++++ packages/kit/src/runtime/client/focus.spec.js | 28 +++ packages/kit/src/runtime/client/scroll.js | 39 ++++ .../kit/src/runtime/client/scroll.spec.js | 30 +++ packages/kit/src/runtime/client/utils.js | 28 +++ 6 files changed, 249 insertions(+), 173 deletions(-) create mode 100644 packages/kit/src/runtime/client/focus.js create mode 100644 packages/kit/src/runtime/client/focus.spec.js create mode 100644 packages/kit/src/runtime/client/scroll.js create mode 100644 packages/kit/src/runtime/client/scroll.spec.js diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 27cf82375ec8..316624179610 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -13,6 +13,8 @@ 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 { disable_scroll_handling, reset_scroll_and_focus } from './scroll.js'; import { find_anchor, resolve_url, @@ -170,47 +172,6 @@ function set_history_options(index, options) { }; } -/** @param {boolean} reset */ -function blur_active_element(reset) { - if ( - reset && - document.activeElement instanceof HTMLElement && - document.activeElement !== document.body - ) { - document.activeElement.blur(); - } -} - -/** - * @param {URL} url - * @param {{ x: number; y: number } | null | undefined} scroll - * @param {boolean} reset - * @param {Element | null} active_element - */ -function reset_scroll_and_focus(url, scroll, reset, active_element) { - /** @type {Element | null} */ - let deep_linked = null; - - if (autoscroll) { - if (scroll) { - scrollTo(scroll.x, scroll.y); - } else if ((deep_linked = get_hash_element(url))) { - 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; -} - /** * @param {number} current_history_index * @param {number} current_navigation_index @@ -372,7 +333,6 @@ let current = { /** this being true means we SSR'd */ let hydrated = false; let started = false; -let autoscroll = true; let updating = false; let is_navigating = false; /** @type {HistoryMetadata | null} */ @@ -2623,7 +2583,7 @@ export function disableScrollHandling() { } if (updating || !started) { - autoscroll = false; + disable_scroll_handling(); } } @@ -3380,7 +3340,7 @@ function _start_router() { }); addEventListener('popstate', async (event) => { - if (resetting_focus) return; + if (is_resetting_focus()) return; const history_metadata = get_history_metadata(event.state); @@ -3779,109 +3739,6 @@ function deserialize_uses(uses) { }; } -/** - * This flag is used to avoid client-side navigation when we're only using - * `location.replace()` to set focus. - */ -let resetting_focus = false; - -/** - * @param {URL} url - * @param {boolean} [scroll] - */ -function reset_focus(url, scroll = true) { - 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); - 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. - 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 - 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'); - } - } - - // capture current selection, so we can compare the state after - // snapshot restoration and afterNavigate callbacks have run - const selection = getSelection(); - - if (selection && selection.type !== 'None') { - /** @type {Range[]} */ - const ranges = []; - - for (let i = 0; i < selection.rangeCount; i += 1) { - ranges.push(selection.getRangeAt(i)); - } - - setTimeout(() => { - if (selection.rangeCount !== ranges.length) return; - - for (let i = 0; i < selection.rangeCount; i += 1) { - 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 - if ( - a.commonAncestorContainer !== b.commonAncestorContainer || - a.startContainer !== b.startContainer || - a.endContainer !== b.endContainer || - a.startOffset !== b.startOffset || - a.endOffset !== b.endOffset - ) { - return; - } - } - - // if the selection hasn't changed (as a result of an element being (auto)focused, - // or a programmatic selection, we reset everything as part of the navigation) - // fixes https://github.com/sveltejs/kit/issues/8439 - selection.removeAllRanges(); - }); - } - } -} - /** * @template {NavigationType} T * @param {import('./types.js').NavigationState} current @@ -3955,32 +3812,6 @@ function decode_hash(url) { return new_url; } -/** - * @param {URL} url - * @returns {string} - */ -function get_id(url) { - let id; - - if (app.hash) { - const [, , second] = url.hash.split('#', 3); - id = second ?? ''; - } else { - id = url.hash.slice(1); - } - - return decodeURIComponent(id); -} - -/** - * @param {URL} url - * @returns {Element | null} - */ -function get_hash_element(url) { - const id = get_id(url); - return id ? document.getElementById(id) : null; -} - if (DEV) { // Nasty hack to silence harmless warnings the user can do nothing about const console_warn = console.warn; diff --git a/packages/kit/src/runtime/client/focus.js b/packages/kit/src/runtime/client/focus.js new file mode 100644 index 000000000000..4a9bbdbeeb7c --- /dev/null +++ b/packages/kit/src/runtime/client/focus.js @@ -0,0 +1,120 @@ +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; +} + +/** @param {boolean} reset */ +export function blur_active_element(reset) { + if ( + reset && + document.activeElement instanceof HTMLElement && + document.activeElement !== document.body + ) { + document.activeElement.blur(); + } +} + +/** + * @param {URL} url + * @param {boolean} [scroll] + */ +export function reset_focus(url, scroll = true) { + 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(); + + // `element.focus()` doesn't work on Safari and Firefox Ubuntu so we need + // to use this hack with `location.replace()` instead. + 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 + 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'); + } + } + + // capture current selection, so we can compare the state after + // snapshot restoration and afterNavigate callbacks have run + const selection = getSelection(); + + if (selection && selection.type !== 'None') { + /** @type {Range[]} */ + const ranges = []; + + for (let i = 0; i < selection.rangeCount; i += 1) { + ranges.push(selection.getRangeAt(i)); + } + + setTimeout(() => { + if (selection.rangeCount !== ranges.length) return; + + for (let i = 0; i < selection.rangeCount; i += 1) { + 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 + if ( + a.commonAncestorContainer !== b.commonAncestorContainer || + a.startContainer !== b.startContainer || + a.endContainer !== b.endContainer || + a.startOffset !== b.startOffset || + a.endOffset !== b.endOffset + ) { + return; + } + } + + // if the selection hasn't changed (as a result of an element being (auto)focused, + // or a programmatic selection, we reset everything as part of the navigation) + // fixes https://github.com/sveltejs/kit/issues/8439 + selection.removeAllRanges(); + }); + } + } +} diff --git a/packages/kit/src/runtime/client/focus.spec.js b/packages/kit/src/runtime/client/focus.spec.js new file mode 100644 index 000000000000..b27c35d3273d --- /dev/null +++ b/packages/kit/src/runtime/client/focus.spec.js @@ -0,0 +1,28 @@ +import { afterEach, beforeEach, expect, test, vi } from 'vitest'; +import { reset_focus } from './focus.js'; + +beforeEach(() => { + window.scrollTo = vi.fn(); + document.body.innerHTML = ''; +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +test('reset_focus focuses the body without leaving a tabindex behind', () => { + document.body.innerHTML = ''; + reset_focus(new URL('/', location.href)); + expect(document.activeElement).toBe(document.body); + 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 = '
'; + reset_focus(new URL('/#a', location.href)); + expect(window.scrollTo).not.toHaveBeenCalled(); + vi.runAllTimers(); + expect(window.scrollTo).toHaveBeenCalledWith(0, 400); +}); diff --git a/packages/kit/src/runtime/client/scroll.js b/packages/kit/src/runtime/client/scroll.js new file mode 100644 index 000000000000..ce384ed9eeb5 --- /dev/null +++ b/packages/kit/src/runtime/client/scroll.js @@ -0,0 +1,39 @@ +import { hash_routing } from '$app/paths/internal/client'; +import { reset_focus } from './focus.js'; +import { get_hash_element } from './utils.js'; + +let autoscroll = true; + +export function disable_scroll_handling() { + autoscroll = false; +} + +/** + * @param {URL} url + * @param {{ x: number; y: number } | null | undefined} scroll + * @param {boolean} reset + * @param {Element | null} active_element + */ +export function reset_scroll_and_focus(url, scroll, reset, active_element) { + /** @type {Element | null} */ + let deep_linked = null; + + if (autoscroll) { + if (scroll) { + scrollTo(scroll.x, 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; +} diff --git a/packages/kit/src/runtime/client/scroll.spec.js b/packages/kit/src/runtime/client/scroll.spec.js new file mode 100644 index 000000000000..182d8045a0b0 --- /dev/null +++ b/packages/kit/src/runtime/client/scroll.spec.js @@ -0,0 +1,30 @@ +import { beforeEach, expect, test, vi } from 'vitest'; +import { disable_scroll_handling, reset_scroll_and_focus } from './scroll.js'; + +const url = new URL('/', location.href); + +beforeEach(() => { + window.scrollTo = vi.fn(); + Element.prototype.scrollIntoView = vi.fn(); + document.body.innerHTML = ''; +}); + +test('restores a popped position ahead of the hash target', () => { + document.body.innerHTML = '
'; + reset_scroll_and_focus(new URL('/#a', location.href), { x: 10, y: 20 }, true, document.body); + 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); + expect(window.scrollTo).not.toHaveBeenCalled(); + reset_scroll_and_focus(url, null, true, document.body); + 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); + expect(window.scrollTo).toHaveBeenCalledTimes(2); +}); diff --git a/packages/kit/src/runtime/client/utils.js b/packages/kit/src/runtime/client/utils.js index 0e280d36505d..0b616bd0aa25 100644 --- a/packages/kit/src/runtime/client/utils.js +++ b/packages/kit/src/runtime/client/utils.js @@ -238,6 +238,34 @@ export function is_external_url(url, base, hash_routing) { return false; } +/** + * @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); +} + +/** + * @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; +} + /** @type {Set | null} */ let seen = null; From 01f7ffb627052025412c596a1ca1ebb0cf40600c Mon Sep 17 00:00:00 2001 From: Nic Polumeyv <162764842+Nic-Polumeyv@users.noreply.github.com> Date: Tue, 1 Sep 2026 14:18:01 -0400 Subject: [PATCH 2/2] test: focus the input the body-focus test resets from --- packages/kit/src/runtime/client/focus.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/client/focus.spec.js b/packages/kit/src/runtime/client/focus.spec.js index b27c35d3273d..deec4a327b51 100644 --- a/packages/kit/src/runtime/client/focus.spec.js +++ b/packages/kit/src/runtime/client/focus.spec.js @@ -12,6 +12,7 @@ afterEach(() => { test('reset_focus focuses the body without leaving a tabindex behind', () => { document.body.innerHTML = ''; + /** @type {HTMLInputElement} */ (document.body.firstElementChild).focus(); reset_focus(new URL('/', location.href)); expect(document.activeElement).toBe(document.body); expect(document.body.hasAttribute('tabindex')).toBe(false);