From c1acbc9dd112d4f2135ad08425aad55f1527b06b Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:24:25 -0400 Subject: [PATCH] visitAllLinks: skip links the SPA router never handles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pnpm-lock.yaml | 4 ++++ test-app/app/templates/application.gts | 6 +++++ .../tests/acceptance/visit-all-links-test.ts | 19 ++++++++++++++++ test-support/package.json | 3 ++- test-support/src/routing/visit-all.ts | 22 ++++++++++++++++++- 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3ad44b2..1b9755c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -135,6 +135,9 @@ importers: ember-source: specifier: '>= 3.28' version: 5.12.0(@glimmer/component@2.1.1)(@glint/template@1.7.4)(rsvp@4.8.5)(webpack@5.104.1) + should-handle-link: + specifier: ^1.2.2 + version: 1.3.0 devDependencies: '@babel/core': specifier: ^7.23.6 @@ -8421,6 +8424,7 @@ snapshots: '@embroider/addon-shim': 1.10.2 ember-source: 7.1.0(@glimmer/component@2.1.1) qunit: 2.25.0 + should-handle-link: 1.3.0 transitivePeerDependencies: - '@babel/core' - '@glint/template' diff --git a/test-app/app/templates/application.gts b/test-app/app/templates/application.gts index 8ac3fb7..a8daeb3 100644 --- a/test-app/app/templates/application.gts +++ b/test-app/app/templates/application.gts @@ -7,4 +7,10 @@ here here / here + + {{! non-SPA links: handled by the browser, not the router — visitAllLinks must skip them }} + new tab + download + external + email diff --git a/test-app/tests/acceptance/visit-all-links-test.ts b/test-app/tests/acceptance/visit-all-links-test.ts index 161803e..1bfa216 100644 --- a/test-app/tests/acceptance/visit-all-links-test.ts +++ b/test-app/tests/acceptance/visit-all-links-test.ts @@ -20,4 +20,23 @@ module('All Links', function (hooks) { `multiple usages does not visit different numbers of links (${size1} === ${size2})`, ); }); + + test('non-SPA links are skipped', async function (assert) { + // The application template renders target="_blank", download, + // rel="external", and mailto: links. The browser handles those natively — + // clicking them can never change currentURL() — so a crawl that clicked + // them would report failed navigations. Every URL the crawl does visit + // must be one the router can serve. + const visited: string[] = []; + + await visitAllLinks((url) => { + visited.push(url); + + const isNonSPA = url.startsWith('mailto:') || url.endsWith('.html'); + + assert.false(isNonSPA, `${url} is a route the router handles`); + }); + + assert.ok(visited.length > 0, 'the SPA links were still visited'); + }); }); diff --git a/test-support/package.json b/test-support/package.json index 1288cdd..25c89e3 100644 --- a/test-support/package.json +++ b/test-support/package.json @@ -53,7 +53,8 @@ }, "dependencies": { "@ember/test-helpers": "^4.0.4 || ^5.2.2", - "@embroider/addon-shim": "^1.8.7" + "@embroider/addon-shim": "^1.8.7", + "should-handle-link": "^1.2.2" }, "devDependencies": { "@babel/core": "^7.23.6", diff --git a/test-support/src/routing/visit-all.ts b/test-support/src/routing/visit-all.ts index 843d2e7..2a1febd 100644 --- a/test-support/src/routing/visit-all.ts +++ b/test-support/src/routing/visit-all.ts @@ -8,6 +8,7 @@ import { findAll, } from '@ember/test-helpers'; import QUnit from 'qunit'; +import { shouldHandle } from 'should-handle-link'; import type Owner from '@ember/owner'; import type RouterService from '@ember/routing/router-service'; @@ -23,10 +24,29 @@ function findInAppLinks(): InAppLink[] { const allAnchorsOnThePage = findAll('a'); for (const a of allAnchorsOnThePage) { + // `findAll('a')` can also match SVG ``, whose `href` is an + // SVGAnimatedString the router (and shouldHandle) can't work with. + if (!(a instanceof HTMLAnchorElement)) continue; + const href = a.getAttribute('href'); if (!href) continue; - if (href.startsWith('http')) continue; + + /** + * Links the SPA's router never handles are handled natively by the + * browser instead (new tab/window via `target`, download dialog, + * `mailto:`/`tel:`, cross-origin, `rel="external"`). Clicking them in a + * test can't change `currentURL()`, so they'd always be reported as + * failed navigations. + * + * `shouldHandle` is the predicate ember-primitives' @properLinks uses to + * decide whether the router handles a click, so the crawler visits + * exactly the set of links the router would. The fabricated click event + * carries the "plain left click" defaults (button 0, no modifier keys). + */ + if (!shouldHandle(window.location.href, a, new MouseEvent('click'))) { + continue; + } const current = new URL(currentURL(), window.location.origin); const url = new URL(href, current);