Skip to content

Fix findLinkFromClickTarget for SVG elements (fixes #1510)#1518

Open
Danish-Belal wants to merge 3 commits intohotwired:mainfrom
Danish-Belal:fix-svg-link-href-startsWith-1510
Open

Fix findLinkFromClickTarget for SVG elements (fixes #1510)#1518
Danish-Belal wants to merge 3 commits intohotwired:mainfrom
Danish-Belal:fix-svg-link-href-startsWith-1510

Conversation

@Danish-Belal
Copy link
Copy Markdown

SVGAElement exposes href as SVGAnimatedString which does not implement startsWith(). Use getAttribute('href') which returns a string for both HTMLAnchorElement and SVGAElement.

Add tests for clicking SVG links (same-origin and hash-only).

SVGAElement exposes href as SVGAnimatedString which does not implement
startsWith(). Use getAttribute('href') which returns a string for both
HTMLAnchorElement and SVGAElement.

Add tests for clicking SVG links (same-origin and hash-only).

Made-with: Cursor
…Target)

Add getLinkHrefString() helper and use in linkToTheSamePage to prevent
TypeError when hovering SVG links with hash href. Add prefetch test.

Made-with: Cursor
Copy link
Copy Markdown

@Samuel-Labagnere Samuel-Labagnere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR overall, lgtm. 👍🏻
Just added a few comments towards harmonizing your texts with how things are written elsewhere in the project.

@Danish-Belal
Copy link
Copy Markdown
Author

Thanks for the review, @Samuel-Labagnere . I’ve addressed all your feedback:

  1. getLinkHrefString comment (util.js)
    Updated the JSDoc to cover both element types and the difference between .href and getAttribute("href"):

  2. Navigation tests

Renamed "clicking a same-origin link inside an SVG element" → "following a same-origin SVG link"
Renamed "clicking an SVG link with hash-only href scrolls to anchor without a visit" → "following a same-origin SVG anchored link"
Renamed fixture link id from #svg-hash-link to #same-origin-anchored-svg-link
Simplified the SVG anchored link test to follow the same pattern as "following a same-origin anchored link"
3. Prefetch test
Renamed "it doesn't prefetch when hovering SVG link with hash href" → "it doesn't prefetch the page when SVG link has a hash as a href" to match the test above.

All changes are pushed. Please let me know if anything else needs adjustment.

@Danish-Belal
Copy link
Copy Markdown
Author

@Samuel-Labagnere Looking forward you to review and getting merge my PR.
Thank you.

@Samuel-Labagnere
Copy link
Copy Markdown

@Danish-Belal So far so good to me, now it's in the maintainers hands.

@Danish-Belal
Copy link
Copy Markdown
Author

Danish-Belal commented Mar 17, 2026

@packagethief
Looking forward for your action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants