Skip to content

Commit d25a5c7

Browse files
committed
Same-page anchor: Account for <a> nested in <svg>
Follow-up to [#1285][] While an `<a>` element is typically represented by an [HTMLAnchorElement][], an `<a>` nested inside an `<svg>` 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 `<a>` nested within an `<svg>`. 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 [e591ea9][]. [#1285]: #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 [e591ea9]: e591ea9#diff-9c0ec1b0a889e599f3ff81590864dd9dc65684a86b41f1da75acc53fafed12e3R251-R256
1 parent e5cbd84 commit d25a5c7

File tree

3 files changed

+28
-1
lines changed

3 files changed

+28
-1
lines changed

src/tests/fixtures/navigation.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ <h1>Navigation</h1>
4747
<p><a id="bare-target-link" href="/src/tests/fixtures/one.html" target>Bare target attribute link</a></p>
4848
<p><a id="same-origin-download-link" href="/intentionally_missing_fake_download.html" download="x.html">Same-origin download link</a></p>
4949
<svg width="600" height="100" viewbox="-300 -50 600 100"><text><a id="same-origin-link-inside-svg-element" href="/src/tests/fixtures/one.html">Same-origin link inside SVG element</a></text></svg>
50+
<svg width="600" height="100" viewbox="-300 -50 600 100"><text><a id="same-origin-targeted-link-inside-svg-element" href="/src/tests/fixtures/one.html" target="_blank">Same-origin targeted link inside SVG element</a></text></svg>
51+
<svg width="600" height="100" viewbox="-300 -50 600 100"><text><a id="same-page-link-inside-svg-element" href="#main">Same-page link inside SVG element</a></text></svg>
5052
<svg width="600" height="100" viewbox="-300 -50 600 100"><text><a id="cross-origin-link-inside-svg-element" href="about:blank">Cross-origin link inside SVG element</a></text></svg>
5153
<p><a id="same-origin-get-link-form" href="/src/tests/fixtures/navigation.html?a=one&b=two" data-turbo-method="get">Same-origin data-turbo-method=get link</a></p>
5254
<p><a id="same-origin-replace-post-link" href="/__turbo/redirect" data-turbo-method="post" data-turbo-action="replace">Same-origin data-turbo-action=replace link with post method</a></p>

src/tests/functional/navigation_tests.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,18 @@ test("following a same-origin [target] link", async ({ page }) => {
258258
expect(await visitAction(page)).toEqual("load")
259259
})
260260

261+
test("following a same-origin [target] link inside an SVG", async ({ page }) => {
262+
const errors = []
263+
await page.on("pageerror", e => errors.push(e))
264+
const link = page.locator("#same-origin-targeted-link-inside-svg-element")
265+
await link.focus()
266+
const [popup] = await Promise.all([page.waitForEvent("popup"), page.keyboard.press("Enter")])
267+
268+
expect(pathname(popup.url())).toEqual("/src/tests/fixtures/one.html")
269+
expect(await visitAction(page)).toEqual("load")
270+
expect(errors, "raised an error").toHaveLength(0)
271+
})
272+
261273
test("following a _self [target] link", async ({ page }) => {
262274
await page.click("#self-targeted-link")
263275

@@ -301,6 +313,18 @@ test("following a same-origin link inside an SVG element", async ({ page }) => {
301313
expect(await visitAction(page)).toEqual("load")
302314
})
303315

316+
test("following a same-page link inside an SVG element", async ({ page }) => {
317+
const errors = []
318+
await page.on("pageerror", e => errors.push(e))
319+
const link = page.locator("#same-page-link-inside-svg-element")
320+
await link.focus()
321+
await page.keyboard.press("Enter")
322+
323+
await expect(page).toHaveURL(withHash("#main"))
324+
expect(await isScrolledToSelector(page, "#main"), "scrolled to #main").toEqual(true)
325+
expect(errors, "raised an error").toHaveLength(0)
326+
})
327+
304328
test("following a cross-origin link inside an SVG element", async ({ page }) => {
305329
const link = page.locator("#cross-origin-link-inside-svg-element")
306330
await link.focus()

src/util.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,12 +248,13 @@ export function findLinkFromClickTarget(target) {
248248
const link = findClosestRecursively(target, "a[href], a[xlink\\:href]")
249249

250250
if (!link) return null
251-
if (link.href.startsWith("#")) return null
252251
if (link.hasAttribute("download")) return null
253252

254253
const linkTarget = link.getAttribute("target")
255254
if (linkTarget && linkTarget !== "_self") return null
256255

256+
if (/\A#/.test(link.getAttribute("href"))) return null
257+
257258
return link
258259
}
259260

0 commit comments

Comments
 (0)