Fix findLinkFromClickTarget for SVG elements (fixes #1510)

src/observers/link_prefetch_observer.js

@@ -2,7 +2,8 @@ import { getLocationForLink } from "../core/url"
 import {
   dispatch,
   getMetaContent,
-  findClosestRecursively
+  findClosestRecursively,
+  getLinkHrefString
 } from "../util"
 
 import { FetchMethod, FetchRequest } from "../http/fetch_request"
@@ -158,7 +159,7 @@ const unfetchableLink = (link) => {
 }
 
 const linkToTheSamePage = (link) => {
-  return (link.pathname + link.search === document.location.pathname + document.location.search) || link.href.startsWith("#")
+  return (link.pathname + link.search === document.location.pathname + document.location.search) || getLinkHrefString(link).startsWith("#")
 }
 
 const linkOptsOut = (link) => {

src/tests/fixtures/hover_to_prefetch.html

@@ -42,6 +42,12 @@
     <a href="http://localhost:9000/src/tests/fixtures/prefetched.html" id="anchor_with_whole_url"
       >Hover to prefetch me</a>
     <a href="#some_anchor" id="anchor_with_hash">Won't prefetch when hovering me</a>
+    <svg width="200" height="40" style="display: block">
+      <a href="#svg_anchor" id="anchor_with_hash_inside_svg">
+        <rect width="200" height="40" fill="#e0e0e0"/>
+        <text x="10" y="25" fill="black">SVG hash link</text>
+      </a>
+    </svg>
     <a href="ftp://example.com" id="anchor_with_ftp_protocol">Won't prefetch when hovering me</a>
     <a href="/src/tests/fixtures/prefetched.html" target="prefetch_iframe" id="anchor_with_iframe_target"
       >Won't prefetch when hovering me</a>

src/tests/fixtures/navigation.html

@@ -48,6 +48,7 @@ <h1>Navigation</h1>
       <p><a id="same-origin-download-link" href="/intentionally_missing_fake_download.html" download="x.html">Same-origin download link</a></p>
       <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>
       <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>
+      <svg width="600" height="100" viewbox="-300 -50 600 100"><text><a id="same-origin-anchored-svg-link" href="#main">SVG link with hash-only href</a></text></svg>
       <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>
       <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>
       <p><a id="link-to-disabled-frame" href="/src/tests/fixtures/frames/hello.html" data-turbo-frame="hello">Disabled turbo-frame</a></p>

src/tests/functional/link_prefetch_observer_tests.js

@@ -121,6 +121,11 @@ test("it doesn't prefetch the page when link has a hash as a href", async ({ pag
   await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_hash" })
 })
 
+test("it doesn't prefetch the page when SVG link has a hash as a href", async ({ page }) => {
+  await goTo({ page, path: "/hover_to_prefetch.html" })
+  await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_hash_inside_svg" })
+})
+
 test("it doesn't prefetch the page when link has a ftp protocol", async ({ page }) => {
   await goTo({ page, path: "/hover_to_prefetch.html" })
   await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_ftp_protocol" })

src/tests/functional/navigation_tests.js

@@ -310,6 +310,22 @@ test("following a cross-origin link inside an SVG element", async ({ page }) =>
   expect(await visitAction(page)).toEqual("load")
 })
 
+test("following a same-origin SVG link", async ({ page }) => {
+  await page.click("#same-origin-link-inside-svg-element")
+
+  await expect(page).toHaveURL(withPathname("/src/tests/fixtures/one.html"))
+  expect(await visitAction(page)).toEqual("load")
+})
+
+test("following a same-origin SVG anchored link", async ({ page }) => {
+  await page.click("#same-origin-anchored-svg-link")
+
+  await expect(page).toHaveURL(withPathname("/src/tests/fixtures/navigation.html"))
+  await expect(page).toHaveURL(withHash("#main"))
+  expect(await visitAction(page)).toEqual("load")
+  expect(await isScrolledToSelector(page, "#main"), "scrolled to #main").toEqual(true)
+})
+
 test("clicking the back button", async ({ page }) => {
   await page.click("#same-origin-unannotated-link")
   await nextEventNamed(page, "turbo:load")

src/util.js

@@ -249,11 +249,21 @@ export function doesNotTargetIFrame(name) {
   }
 }
 
+/**
+ * Returns consistently the href attribute value as a string for both HTMLAnchorElement and SVGAElement.
+ * HTMLAnchorElement href property returns an absolute URL if the attribute contains a valid relative URL.
+ * SVGAElement exposes href as SVGAnimatedString which does not implement String methods.
+ * getAttribute() will return the proper value of the attribute in both cases.
+ */
+export function getLinkHrefString(link) {
+  return link.getAttribute("href") ?? link.getAttribute("xlink:href") ?? ""
+}
+
 export function findLinkFromClickTarget(target) {
   const link = findClosestRecursively(target, "a[href], a[xlink\\:href]")
 
   if (!link) return null
-  if (link.href.startsWith("#")) return null
+  if (getLinkHrefString(link).startsWith("#")) return null
   if (link.hasAttribute("download")) return null
 
   const linkTarget = link.getAttribute("target")