visitAllLinks: skip links the SPA router never handles - #131
Merged
Merged
Conversation
Anchors with target (e.g. _blank), download, rel="external", or a
non-http(s) scheme (mailto:, tel:) are handled natively by the browser
— a new tab, a download dialog, an external protocol handler. Clicking
them in a test can never change currentURL(), so the crawl reported
every one of them as a failed navigation ('Navigation was successful'
pushResult with the unchanged URL as actual).
Found crawling AuditBoard's docs app, whose homepage links a statically
hosted report via target="_blank".
Rather than hand-mirroring the rules, the crawler uses
should-handle-link directly — the predicate ember-primitives'
@properLinks uses to decide whether the router handles a click — with a
fabricated plain left-click MouseEvent to satisfy the event-modifier
checks. Its origin comparison also subsumes the old startsWith('http')
heuristic (and covers mailto:/tel: via the null origin). SVG anchors
are skipped: their href is an SVGAnimatedString neither the router nor
shouldHandle can work with.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NullVoxPopuli
force-pushed
the
skip-non-spa-links
branch
from
July 28, 2026 16:25
0222ac4 to
c1acbc9
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
findInAppLinkscollected every same-origin anchor, but anchors withtarget(e.g._blank),download,rel="external", or a non-http(s) scheme (mailto:,tel:) are handled natively by the browser — a new tab, a download dialog, an external protocol handler. Clicking them under the test harness can never changecurrentURL(), so the crawl reported each one as a failed navigation (Navigation was successfulwith the unchanged URL asactual).Found crawling AuditBoard's docs app, whose homepage links a statically hosted audit report via
target="_blank"— a perfectly working link that failed the crawl.Instead of hand-mirroring the rules, the crawler now uses
should-handle-linkdirectly — the predicate@properLinksuses to decide whether the router handles a click — with a fabricated plain left-clickMouseEventto satisfy the event-modifier checks. So the crawler visits exactly the set of links the router would handle, and stays in lockstep with properLinks as its rules evolve. Its origin comparison also subsumes the oldstartsWith('http')heuristic (and coversmailto:/tel:via thenullorigin). SVG<a>elements are skipped — theirhrefis anSVGAnimatedStringneither the router norshouldHandlecan work with.Tests: the test-app's application template now renders all four non-SPA link kinds; without the fix, both All Links acceptance tests fail (the crawler clicks the
target="_blank"link and reports a failed navigation), with it the suite is 6/6.🤖 Generated with Claude Code