From 9951bff30f7b9f5c634c78528da83747a8fb808d Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Mon, 2 Feb 2026 15:37:25 -0500 Subject: [PATCH] Same-page anchor: Account for `` nested in `` Follow-up to [#1285][] While an `` element is typically represented by an [HTMLAnchorElement][], an `` nested inside an `` element is instead an [SVGAElement][]. Similarly, while [HTMLAnchorElement.href][] returns a `String`, the [SVGAElement.href][] returns an instance of [SVGAnimatedString][], which cannot be supported by neither calls to [String.startsWith][] nor [RegExp.test][]. This commit adds explicit test coverage for a same-page navigation from an `` nested within an ``. To ensure that the value is a `String`, replace `.href` with `getAttribute("href")`. This restores the implementation to be closer to what was defined in [e591ea9e][]. [#1285]: https://github.com/hotwired/turbo/pull/1285 [HTMLAnchorElement]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLAnchorElement [SVGAElement]: https://developer.mozilla.org/en-US/docs/Web/API/SVGAElement [HTMLAnchorElement.href]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLAnchorElement/href [SVGAElement.href]: https://developer.mozilla.org/en-US/docs/Web/API/SVGAElement/href [SVGAnimatedString]: https://developer.mozilla.org/en-US/docs/Web/API/SVGAnimatedString [String.startsWith]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith [RegExp.test]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test [e591ea9e]: https://github.com/hotwired/turbo/pull/1285/changes/e591ea9eb087a308f8d6997dc3ffe051058e3ec9#diff-9c0ec1b0a889e599f3ff81590864dd9dc65684a86b41f1da75acc53fafed12e3R251-R256 --- src/tests/fixtures/navigation.html | 2 ++ src/tests/functional/navigation_tests.js | 45 +++++++++++++++++++++++- src/util.js | 4 ++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/tests/fixtures/navigation.html b/src/tests/fixtures/navigation.html index 81e98793b..94a387b3e 100644 --- a/src/tests/fixtures/navigation.html +++ b/src/tests/fixtures/navigation.html @@ -47,6 +47,8 @@

Navigation

Bare target attribute link

Same-origin download link

Same-origin link inside SVG element + Same-origin targeted link inside SVG element + Same-page link inside SVG element Cross-origin link inside SVG element

Same-origin data-turbo-method=get link

Same-origin data-turbo-action=replace link with post method

diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js index 650590688..5338d2d1c 100644 --- a/src/tests/functional/navigation_tests.js +++ b/src/tests/functional/navigation_tests.js @@ -258,6 +258,18 @@ test("following a same-origin [target] link", async ({ page }) => { expect(await visitAction(page)).toEqual("load") }) +test("following a same-origin [target] link inside an SVG", async ({ page }) => { + const errors = [] + await page.on("pageerror", e => errors.push(e)) + const link = page.locator("#same-origin-targeted-link-inside-svg-element") + await link.focus() + const [popup] = await Promise.all([page.waitForEvent("popup"), page.keyboard.press("Enter")]) + + expect(pathname(popup.url())).toEqual("/src/tests/fixtures/one.html") + expect(await visitAction(page)).toEqual("load") + expect(errors, "raised an error").toHaveLength(0) +}) + test("following a _self [target] link", async ({ page }) => { await page.click("#self-targeted-link") @@ -298,7 +310,19 @@ test("following a same-origin link inside an SVG element", async ({ page }) => { await page.keyboard.press("Enter") await expect(page).toHaveURL(withPathname("/src/tests/fixtures/one.html")) - expect(await visitAction(page)).toEqual("load") + expect(await visitAction(page)).toEqual("advance") +}) + +test("following a same-page link inside an SVG element", async ({ page }) => { + const errors = [] + await page.on("pageerror", e => errors.push(e)) + const link = page.locator("#same-page-link-inside-svg-element") + await link.focus() + await page.keyboard.press("Enter") + + await expect(page).toHaveURL(withHash("#main")) + expect(await isScrolledToSelector(page, "#main"), "scrolled to #main").toEqual(true) + expect(errors, "raised an error").toHaveLength(0) }) test("following a cross-origin link inside an SVG element", async ({ page }) => { @@ -425,6 +449,25 @@ test("same-page anchor visits do not trigger visit events", async ({ page }) => } }) +test("same-page anchor visits within SVG elements do not trigger visit events", async ({ page }) => { + const events = [ + "turbo:before-visit", + "turbo:visit", + "turbo:before-cache", + "turbo:before-render", + "turbo:render", + "turbo:load" + ] + + for (const eventName in events) { + await page.goto("/src/tests/fixtures/navigation.html") + const link = page.locator("#same-origin-link-inside-svg-element") + await link.focus() + await page.keyboard.press("Enter") + expect(await noNextEventNamed(page, eventName), `same-page links do not trigger ${eventName} events`).toEqual(true) + } +}) + test("correct referrer header", async ({ page }) => { page.click("#headers-link") await nextEventNamed(page, "turbo:load") diff --git a/src/util.js b/src/util.js index 5a9024a1f..dc654683c 100644 --- a/src/util.js +++ b/src/util.js @@ -248,12 +248,14 @@ export function findLinkFromClickTarget(target) { const link = findClosestRecursively(target, "a[href], a[xlink\\:href]") if (!link) return null - if (link.href.startsWith("#")) return null if (link.hasAttribute("download")) return null const linkTarget = link.getAttribute("target") if (linkTarget && linkTarget !== "_self") return null + const linkHref = link.getAttribute("href") + if (linkHref && linkHref.startsWith("#")) return null + return link }