diff --git a/.changeset/focus-starting-point.md b/.changeset/focus-starting-point.md new file mode 100644 index 000000000000..8f90475687c9 --- /dev/null +++ b/.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 diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 86ed57e4e400..439e2d80121a 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/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) { diff --git a/packages/kit/src/runtime/client/focus.js b/packages/kit/src/runtime/client/focus.js index 6fc8d7d74960..d8273ec0982f 100644 --- a/packages/kit/src/runtime/client/focus.js +++ b/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 diff --git a/packages/kit/src/runtime/client/focus.spec.js b/packages/kit/src/runtime/client/focus.spec.js index cbfd16e6bc90..8be75d447b35 100644 --- a/packages/kit/src/runtime/client/focus.spec.js +++ b/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 = ''; 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 = '
'; +test("reset_focus restores the hash target's tabindex", () => { + document.body.innerHTML = '

'; 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); }); diff --git a/packages/kit/src/runtime/client/scroll.js b/packages/kit/src/runtime/client/scroll.js index 5a6a63d99932..93fdde332640 100644 --- a/packages/kit/src/runtime/client/scroll.js +++ b/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; } diff --git a/packages/kit/src/runtime/client/scroll.spec.js b/packages/kit/src/runtime/client/scroll.spec.js index b7dd2986e79d..897bb21163ee 100644 --- a/packages/kit/src/runtime/client/scroll.spec.js +++ b/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 = '
'; - 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(); }); diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index c00406b4248b..3157edf15d6c 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -1179,7 +1179,7 @@ test.describe('Routing', () => { 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(); }); diff --git a/packages/kit/test/apps/hash-based-routing/test/test.js b/packages/kit/test/apps/hash-based-routing/test/test.js index 648d7cd27db8..9f46cb120fa2 100644 --- a/packages/kit/test/apps/hash-based-routing/test/test.js +++ b/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 }) => {