From d4a4308fee3f71961c2d0a08db4564b7370d72b9 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 5 Aug 2026 15:42:06 +0530 Subject: [PATCH 1/4] fix(test): add a shared browser nav guard so an escaped click cannot abort the run web-test-runner aborts the whole SESSION, not one file, when the page navigates, so any browser test that clicks a real anchor or submits a real form is a single point of failure for all 60+ browser test files. When the router loses the race to intercept, the run dies with 0 failed plus exit 1, which reads as an infrastructure blip rather than a test problem. The guard listens on window in the BUBBLE phase, which is the load-bearing detail. Window bubble is the last step of the propagation path, so it runs after the router's document-level listeners and preventDefault still cancels the default action. A capture-phase guard would set defaultPrevented before the router ever saw the event, and the router returns early on that flag, so every guarded router test would pass while testing nothing. Forms are blocked on submit rather than on the submit control's click, since cancelling that click would stop the form from ever submitting and the router would never see it. --- .../test/routing/browser/nav-guard.test.js | 159 ++++++++++++++++++ test/browser-nav-guard.js | 79 +++++++++ 2 files changed, 238 insertions(+) create mode 100644 packages/core/test/routing/browser/nav-guard.test.js create mode 100644 test/browser-nav-guard.js diff --git a/packages/core/test/routing/browser/nav-guard.test.js b/packages/core/test/routing/browser/nav-guard.test.js new file mode 100644 index 000000000..401136fd4 --- /dev/null +++ b/packages/core/test/routing/browser/nav-guard.test.js @@ -0,0 +1,159 @@ +/** + * The shared browser-test navigation guard (#1135) does its job WITHOUT + * suppressing the router. + * + * This file exists because the guard's phase is a silent correctness trap. A + * capture-phase guard blocks navigation just as well, so the blocking tests + * below pass either way; what it also does is set `defaultPrevented` before the + * router's document-level bubble listener runs, and the router returns + * immediately on that flag. Every guarded router test would then pass while + * testing nothing at all. The "router still runs" tests are the regression test + * for exactly that, and are the reason this is a test rather than a comment. + * + * The two halves need DIFFERENT fixtures, which is the non-obvious part: + * + * - Blocking is proved with `data-no-router`, which the router deliberately + * ignores (`packages/core/src/router-client.js`). The guard is then the ONLY + * thing standing between the click and a real document load, so an unchanged + * `location` is attributable to the guard alone. + * - It CANNOT be proved on a plain link, because a successful soft navigation + * calls `history.pushState` and legitimately changes `location.pathname`. An + * "unchanged pathname" assertion there fails against a perfectly healthy + * router, which is exactly what it did when first written that way. + */ +import { html } from '../../../src/html.js'; +import { render } from '../../../src/render-client.js'; +import { enableClientRouter } from '../../../src/router-client.js'; + +import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +const tick = () => new Promise((r) => setTimeout(r, 0)); + +suite('Browser-test nav guard (#1135)', () => { + let container, origFetch, fetched, bOpen, bClose, guard, navigated, onNavigate, origHref; + + function setup() { + enableClientRouter(); // idempotent; ensures the document listeners are attached + guard = installNavGuard(); + origHref = location.href; + container = document.createElement('div'); + // A live keyed boundary pair (#1015) so an intercepted nav swaps softly + // rather than degrading, which the guard could not block. + bOpen = document.createComment('wj:children:/:/'); + bClose = document.createComment('/wj:children:/'); + document.body.appendChild(bOpen); + document.body.appendChild(container); + document.body.appendChild(bClose); + navigated = []; + onNavigate = (e) => navigated.push(e.detail && e.detail.url); + document.addEventListener('webjs:navigate', onNavigate); + fetched = []; + origFetch = window.fetch; + window.fetch = (url) => { + fetched.push(String(url)); + return Promise.resolve(new Response('

x

', { + headers: { 'content-type': 'text/html', 'x-webjs-build': '' }, + })); + }; + } + function teardown() { + document.removeEventListener('webjs:navigate', onNavigate); + window.fetch = origFetch; + container.remove(); + bOpen.remove(); + bClose.remove(); + guard.remove(); + // A committed soft nav pushState'd a fake URL. Put the runner's own URL + // back so it does not leak into the next test or file. + history.replaceState(null, '', origHref); + } + + /** Resolve when the router settles, so teardown never runs mid-swap. */ + function awaitNavigation(timeoutMs = 2000) { + return new Promise((resolve) => { + let settled = false; + let timer; + const settle = () => { + if (settled) return; + settled = true; + clearTimeout(timer); + document.removeEventListener('webjs:navigate', settle); + document.removeEventListener('webjs:navigation-fallback', settle); + setTimeout(resolve, 0); + }; + timer = setTimeout(settle, timeoutMs); + document.addEventListener('webjs:navigate', settle); + document.addEventListener('webjs:navigation-fallback', settle); + }); + } + + test('blocks the default activation of a link the router ignores', async () => { + setup(); + const before = location.pathname; + try { + render(html`go`, container); + container.querySelector('a').click(); + await tick(); + // The router returned early on `data-no-router`, so nothing but the guard + // stopped this click. Reaching this line at all is already half the + // proof: without the guard the runner page navigates and the whole + // session is torn down before any assertion runs. + assert.equal(fetched.length, 0, 'the router must ignore a data-no-router link'); + assert.equal(location.pathname, before, + 'the guard must block the default anchor activation'); + } finally { teardown(); } + }); + + test('blocks the default submission of a form the router ignores', async () => { + setup(); + const before = location.pathname; + try { + render(html`
`, container); + container.querySelector('button').click(); + await tick(); + assert.equal(fetched.length, 0, 'the router must ignore a data-no-router form'); + assert.equal(location.pathname, before, + 'the guard must block the default form submission'); + } finally { teardown(); } + }); + + test('does NOT suppress the router on a plain link (capture-phase regression)', async () => { + setup(); + try { + render(html`go`, container); + const settled = awaitNavigation(); + container.querySelector('a').click(); + await settled; + // A capture-phase guard would trip the router's `defaultPrevented` early + // return, leaving `fetched` empty and committing no swap, while the + // blocking tests above still passed. + assert.ok(fetched.some((u) => u.includes('/nav-guard-target')), + 'the guard must NOT suppress the router (it still fetches the target)'); + assert.ok(navigated.some((u) => u && String(u).includes('/nav-guard-target')), + 'the guard must NOT suppress the router (the soft swap still commits)'); + assert.equal(guard.fallbacks.length, 0, + `the nav must be soft, not degraded (cause: ${guard.fallbacks.length ? guard.fallbacks[0].cause : 'none'})`); + } finally { teardown(); } + }); + + test('does NOT suppress the router on a plain form submission', async () => { + setup(); + try { + render(html`
`, container); + // Wait for a real settle, not a fixed number of macrotasks. A bare + // `tick()` let teardown remove the live boundary pair while the + // submission's swap was still running, which is the + // `no-shared-boundary` degradation, and a degradation assigns + // `location.href`, which no guard can cancel. Firefox lost that race + // every run; Chromium and WebKit happened to win it. + const settled = awaitNavigation(); + container.querySelector('button').click(); + await settled; + assert.ok(fetched.some((u) => u.includes('/nav-guard-form')), + 'the guard must NOT suppress the router (it still posts the form)'); + assert.equal(guard.fallbacks.length, 0, + `the submission must be soft, not degraded (cause: ${guard.fallbacks.length ? guard.fallbacks[0].cause : 'none'})`); + } finally { teardown(); } + }); +}); diff --git a/test/browser-nav-guard.js b/test/browser-nav-guard.js new file mode 100644 index 000000000..cfa67b2be --- /dev/null +++ b/test/browser-nav-guard.js @@ -0,0 +1,79 @@ +/** + * Shared navigation guard for browser tests (#1135). Sibling of + * `test/browser-assert.js` (#777) and with the same "one source of truth for + * browser tests" role. + * + * The problem it solves: web-test-runner aborts the ENTIRE session, not one + * file, when the page navigates. A browser test that clicks a real `` + * or submits a real `
` is therefore a single point of failure for all 60+ + * browser test files. Whenever the router loses the race to intercept (a slower + * engine, a slow module load, an unlucky tick), the browser performs the real + * navigation and the run dies with `0 failed` plus exit 1, which reads as an + * infrastructure blip rather than a test problem. + * + * This makes the browser's default activation structurally impossible while + * leaving the router fully exercised, so an interception gap FAILS one test on + * its own assertion instead of taking down the run. + * + * ## The phase is load-bearing: `window`, BUBBLE, never capture + * + * The router registers its `click` / `submit` listeners on `document` in the + * bubble phase (`packages/core/src/router-client.js`), and returns immediately + * when `e.defaultPrevented` is already set (the `#150` / `#153` contract: a + * component's own `@click` must be able to opt out). + * + * `window` bubble is the LAST step of the propagation path, so it runs after + * every document-level listener, and `preventDefault()` still cancels the + * default action because that action runs only once dispatch completes. It also + * needs no registration-order contract with `enableClientRouter()`. + * + * A CAPTURE-phase guard would set `defaultPrevented` BEFORE the router ever saw + * the event, so every guarded router test would silently stop testing the + * router while still passing. That is a silent no-op, which is the worst + * possible failure here, and it is why `nav-guard.test.js` asserts the router + * still ran rather than trusting this by inspection. + * + * ## What it cannot do + * + * It cannot stop the router's own `location.href` assignment on a degradation: + * `preventDefault` cancels a default action, not a script assignment. The + * `fallbacks` array is the coverage for that second channel. Every reload site + * dispatches `webjs:navigation-fallback` with a stable `cause` immediately + * beforehand, so a test asserts `fallbacks` is empty and names the cause. + * Removing the conditions that cause a degradation is the fixture work in + * #1053. + * + * @returns {{ fallbacks: Array<{cause: string, href: string, willReload: boolean}>, remove: () => void }} + */ +export function installNavGuard() { + /** @type {Array<{cause: string, href: string, willReload: boolean}>} */ + const fallbacks = []; + + const onClick = (e) => { + // `closest` is guarded because the target can be a text node or the + // document itself, neither of which has it. + const anchor = e.target && e.target.closest && e.target.closest('a[href]'); + if (anchor) e.preventDefault(); + }; + + // Forms need the `submit` event, NOT the click on the submit control. The + // form's default action fires on submit, and cancelling the button's click + // would stop the form from ever submitting, so the router's own submit + // listener would never run and the test would assert nothing. + const onSubmit = (e) => { e.preventDefault(); }; + + const onFallback = (e) => { fallbacks.push(e.detail); }; + + window.addEventListener('click', onClick); + window.addEventListener('submit', onSubmit); + document.addEventListener('webjs:navigation-fallback', onFallback); + + return { + fallbacks, + remove() { + window.removeEventListener('click', onClick); + window.removeEventListener('submit', onSubmit); + document.removeEventListener('webjs:navigation-fallback', onFallback); + }, + }; +} From 32b30c11c641a478bf4682f1c63212172c2887da Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 5 Aug 2026 15:45:28 +0530 Subject: [PATCH 2/4] fix(test): install the shared nav guard in every suite that clicks a real link Nine router suites click real anchors or submit real forms, so each was a single point of failure for the whole browser run. They now install the shared guard per test. form-action-submit had hand-rolled the identical window-bubble listener with the identical reasoning, which is the duplication the shared module replaces. router-js-handled now reads its degradation events off the guard rather than registering a second listener for the same thing. The two website suites keep their capture-phase blockNav and gain a comment saying why: capture suppresses the router on purpose there, because those tests exercise a menu rather than navigation, and a live router would issue real page fetches. --- .../routing/browser/fetch-revalidates.test.js | 6 +++++ .../browser/form-action-submit.test.js | 26 +++++++------------ .../routing/browser/frame-missing.test.js | 6 +++++ .../routing/browser/frame-targeting.test.js | 10 +++++-- .../routing/browser/navigation-error.test.js | 6 +++++ .../test/routing/browser/query-params.test.js | 6 +++++ .../routing/browser/router-js-handled.test.js | 19 ++++++++------ .../test/routing/browser/submit-state.test.js | 6 +++++ .../view-transition-head-and-suspense.test.js | 8 ++++++ .../components/browser/docs-drawer.test.js | 7 +++++ .../components/browser/site-nav-menu.test.js | 7 +++++ 11 files changed, 80 insertions(+), 27 deletions(-) diff --git a/packages/core/test/routing/browser/fetch-revalidates.test.js b/packages/core/test/routing/browser/fetch-revalidates.test.js index d79b18e05..a6301b76b 100644 --- a/packages/core/test/routing/browser/fetch-revalidates.test.js +++ b/packages/core/test/routing/browser/fetch-revalidates.test.js @@ -16,6 +16,10 @@ import { enableClientRouter, disableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 0)); async function settle() { for (let i = 0; i < 4; i++) await tick(); } @@ -23,6 +27,7 @@ suite('Client router: fetches revalidate instead of trusting the HTTP cache (#11 let container, origFetch, calls; function setup() { + navGuard = installNavGuard(); enableClientRouter(); container = document.createElement('div'); container.innerHTML = @@ -45,6 +50,7 @@ suite('Client router: fetches revalidate instead of trusting the HTTP cache (#11 }; } function teardown() { + navGuard.remove(); window.fetch = origFetch; container.remove(); disableClientRouter(); diff --git a/packages/core/test/routing/browser/form-action-submit.test.js b/packages/core/test/routing/browser/form-action-submit.test.js index 6d1907132..b099c9af6 100644 --- a/packages/core/test/routing/browser/form-action-submit.test.js +++ b/packages/core/test/routing/browser/form-action-submit.test.js @@ -23,26 +23,16 @@ import { render } from '../../../src/render-client.js'; import { enableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; const tick = () => new Promise((r) => setTimeout(r, 20)); suite('Client router: bound form submissions (#1155)', () => { - // LAST-RESORT navigation backstop for a loaded runner: if the router under - // pressure fails a boundary scan and degrades to a full page load, the - // native form submission would navigate the WTR page away and kill the - // whole file ("Tests were interrupted..."). A BUBBLE-phase listener at the - // window level fires after the router's document-level handling had its - // chance (a capture listener here would fire FIRST and break every - // router-handled submission): when the event is still not - // default-prevented by the time it bubbles out to the window, cancel it so - // the ASSERTIONS fail visibly instead of the page dying. Never interferes - // with router-handled submissions (those are already default-prevented). - window.addEventListener( - 'submit', - (e) => { - if (!e.defaultPrevented) e.preventDefault(); - }, - false, - ); + // The navigation backstop this suite used to declare inline now lives in the + // shared guard (#1135), which is the same window-bubble listener with the + // same reasoning, so every browser suite gets it rather than the few that + // hand-rolled a copy. See `test/browser-nav-guard.js` for why the phase is + // window bubble and never capture. + let navGuard; let container, origFetch, calls; // When a test redefines window.location.href (to detect a full-page reload), @@ -52,6 +42,7 @@ suite('Client router: bound form submissions (#1155)', () => { let bOpen, bClose; function setup(responder) { + navGuard = installNavGuard(); enableClientRouter(); // idempotent container = document.createElement('div'); // Bracket the container with a live keyed boundary pair (#1015): the swap @@ -71,6 +62,7 @@ suite('Client router: bound form submissions (#1155)', () => { }; } function teardown() { + navGuard.remove(); window.fetch = origFetch; if (restoreHref) { try { restoreHref(); } catch { /* ignore */ } restoreHref = null; } container.remove(); diff --git a/packages/core/test/routing/browser/frame-missing.test.js b/packages/core/test/routing/browser/frame-missing.test.js index 346b7ea68..ab7e72253 100644 --- a/packages/core/test/routing/browser/frame-missing.test.js +++ b/packages/core/test/routing/browser/frame-missing.test.js @@ -25,6 +25,10 @@ import { enableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 0)); /** Wait for the router's async navigation pipeline to settle. */ @@ -38,6 +42,7 @@ suite('Client router: frame-missing contract (#251)', () => { let container, origFetch, origWarn, warnings; function setup() { + navGuard = installNavGuard(); enableClientRouter(); // idempotent; ensures the document listeners are attached container = document.createElement('div'); // Sibling content that lives OUTSIDE the frame. If the document is @@ -65,6 +70,7 @@ suite('Client router: frame-missing contract (#251)', () => { console.warn = (...a) => { warnings.push(a.join(' ')); }; } function teardown() { + navGuard.remove(); window.fetch = origFetch; console.warn = origWarn; container.remove(); diff --git a/packages/core/test/routing/browser/frame-targeting.test.js b/packages/core/test/routing/browser/frame-targeting.test.js index fc83a5a3b..085563129 100644 --- a/packages/core/test/routing/browser/frame-targeting.test.js +++ b/packages/core/test/routing/browser/frame-targeting.test.js @@ -23,6 +23,10 @@ import { } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 0)); async function settle() { await tick(); await tick(); await tick(); } @@ -34,6 +38,7 @@ suite('Client router: external targeting (#252)', () => { let container; function setup() { + navGuard = installNavGuard(); enableClientRouter(); // idempotent container = document.createElement('div'); container.innerHTML = @@ -53,7 +58,7 @@ suite('Client router: external targeting (#252)', () => { ''; document.body.appendChild(container); } - function teardown() { container.remove(); } + function teardown() { navGuard.remove(); container.remove(); } test('an external data-webjs-frame link (not nested) resolves the external frame id', () => { setup(); @@ -138,6 +143,7 @@ suite('Client router: aria-busy lifecycle (#252)', () => { let container, origFetch; function setup() { + navGuard = installNavGuard(); enableClientRouter(); container = document.createElement('div'); container.innerHTML = @@ -148,7 +154,7 @@ suite('Client router: aria-busy lifecycle (#252)', () => { document.body.appendChild(container); origFetch = window.fetch; } - function teardown() { window.fetch = origFetch; container.remove(); } + function teardown() { navGuard.remove(); window.fetch = origFetch; container.remove(); } test('aria-busy is true during the fetch and false after a successful swap, with start+finish events', async () => { setup(); diff --git a/packages/core/test/routing/browser/navigation-error.test.js b/packages/core/test/routing/browser/navigation-error.test.js index 6ca7cd686..04905ce12 100644 --- a/packages/core/test/routing/browser/navigation-error.test.js +++ b/packages/core/test/routing/browser/navigation-error.test.js @@ -28,6 +28,10 @@ import { enableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 0)); /** Wait for the router's async navigation pipeline to settle. */ @@ -42,6 +46,7 @@ suite('Client router: in-place navigation-error recovery (#249)', () => { let container, origFetch; function setup() { + navGuard = installNavGuard(); enableClientRouter(); // idempotent; ensures the document listeners are attached container = document.createElement('div'); // Outer chrome that lives OUTSIDE the children slot. Its survival is @@ -57,6 +62,7 @@ suite('Client router: in-place navigation-error recovery (#249)', () => { origFetch = window.fetch; } function teardown() { + navGuard.remove(); window.fetch = origFetch; container.remove(); } diff --git a/packages/core/test/routing/browser/query-params.test.js b/packages/core/test/routing/browser/query-params.test.js index c95bc72b1..c605a069f 100644 --- a/packages/core/test/routing/browser/query-params.test.js +++ b/packages/core/test/routing/browser/query-params.test.js @@ -21,6 +21,10 @@ import { } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 25)); /** Poll `location.search` until it equals `want` (a real popstate/pushState is * async), returning the final value so the assertion message is useful. */ @@ -44,6 +48,7 @@ suite('Client router: query-string preservation (#639)', () => { let origFetch, calls, before, container; function setup(responder) { + navGuard = installNavGuard(); enableClientRouter(); // idempotent _resetPrefetch(); document.body.innerHTML = 'before'; @@ -58,6 +63,7 @@ suite('Client router: query-string preservation (#639)', () => { }; } function teardown() { + navGuard.remove(); window.fetch = origFetch; // Restore history so a later test starts on the original URL. try { history.replaceState(null, '', before); } catch { /* ignore */ } diff --git a/packages/core/test/routing/browser/router-js-handled.test.js b/packages/core/test/routing/browser/router-js-handled.test.js index 1a408f919..824743e08 100644 --- a/packages/core/test/routing/browser/router-js-handled.test.js +++ b/packages/core/test/routing/browser/router-js-handled.test.js @@ -19,6 +19,7 @@ import { render } from '../../../src/render-client.js'; import { enableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; const tick = () => new Promise((r) => setTimeout(r, 0)); /** @@ -88,10 +89,16 @@ function awaitNavigation(timeoutMs = 2000) { } suite('Client router: JS-handled links/forms are not hijacked (#150, #153)', () => { - let container, origFetch, fetched, bOpen, bClose, fallbacks, navigated, onFallback, onNavigate; + let container, origFetch, fetched, bOpen, bClose, fallbacks, navigated, navGuard, onNavigate; function setup() { enableClientRouter(); // idempotent; ensures the document listeners are attached + // The shared guard (#1135) blocks the browser's default anchor activation + // so an interception gap fails THIS test instead of navigating the runner + // page and aborting the whole session. It also collects the degradation + // events these tests assert on, so there is no second listener here. + navGuard = installNavGuard(); + fallbacks = navGuard.fallbacks; container = document.createElement('div'); // Bracket the container with a live keyed boundary pair (#1015) and return // a boundary-carrying body, so an intercepted nav swaps softly instead of @@ -101,14 +108,10 @@ suite('Client router: JS-handled links/forms are not hijacked (#150, #153)', () document.body.appendChild(bOpen); document.body.appendChild(container); document.body.appendChild(bClose); - // Record both router diagnostics (#1114) for the life of the test. A - // degradation carries a stable `cause` slug, which is the entire diagnosis - // when one of these tests reds, so it has to reach the assertion message. - fallbacks = []; + // The commit signal (#1114). Its degradation counterpart comes from the + // guard above. navigated = []; - onFallback = (e) => fallbacks.push(e.detail); onNavigate = (e) => navigated.push(e.detail && e.detail.url); - document.addEventListener('webjs:navigation-fallback', onFallback); document.addEventListener('webjs:navigate', onNavigate); fetched = []; origFetch = window.fetch; @@ -120,7 +123,7 @@ suite('Client router: JS-handled links/forms are not hijacked (#150, #153)', () }; } function teardown() { - document.removeEventListener('webjs:navigation-fallback', onFallback); + navGuard.remove(); document.removeEventListener('webjs:navigate', onNavigate); window.fetch = origFetch; container.remove(); diff --git a/packages/core/test/routing/browser/submit-state.test.js b/packages/core/test/routing/browser/submit-state.test.js index 3347ef043..e12b9dcf7 100644 --- a/packages/core/test/routing/browser/submit-state.test.js +++ b/packages/core/test/routing/browser/submit-state.test.js @@ -21,6 +21,10 @@ import { render } from '../../../src/render-client.js'; import { enableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 20)); function htmlResponse(body, status = 200) { @@ -38,6 +42,7 @@ suite('Client router: form submission-state events + aria-busy (#246)', () => { let bOpen, bClose; function setup() { + navGuard = installNavGuard(); enableClientRouter(); // idempotent container = document.createElement('div'); // Bracket the container with a live keyed boundary pair (#1015): the @@ -56,6 +61,7 @@ suite('Client router: form submission-state events + aria-busy (#246)', () => { }); } function teardown() { + navGuard.remove(); window.fetch = origFetch; container.remove(); bOpen.remove(); diff --git a/packages/core/test/routing/browser/view-transition-head-and-suspense.test.js b/packages/core/test/routing/browser/view-transition-head-and-suspense.test.js index 5e021a9e1..3dc962b5d 100644 --- a/packages/core/test/routing/browser/view-transition-head-and-suspense.test.js +++ b/packages/core/test/routing/browser/view-transition-head-and-suspense.test.js @@ -21,6 +21,10 @@ */ import { enableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 0)); async function settle() { for (let i = 0; i < 6; i++) await tick(); } @@ -46,6 +50,7 @@ function setViewTransitionMeta(on) { suite('Client router: page-scoped reconciliation on soft nav (#1046)', () => { let container, origFetch, origSVT; function setup() { + navGuard = installNavGuard(); enableClientRouter(); container = document.createElement('div'); document.body.appendChild(container); @@ -53,6 +58,7 @@ suite('Client router: page-scoped reconciliation on soft nav (#1046)', () origSVT = document.startViewTransition; } function teardown() { + navGuard.remove(); window.fetch = origFetch; document.startViewTransition = origSVT; setViewTransitionMeta(false); @@ -194,6 +200,7 @@ suite('Client router: Suspense streaming resolves under view transitions (#1048) ''; function setup() { + navGuard = installNavGuard(); enableClientRouter(); container = document.createElement('div'); document.body.appendChild(container); @@ -201,6 +208,7 @@ suite('Client router: Suspense streaming resolves under view transitions (#1048) origSVT = document.startViewTransition; } function teardown() { + navGuard.remove(); window.fetch = origFetch; document.startViewTransition = origSVT; setViewTransitionMeta(false); diff --git a/website/test/components/browser/docs-drawer.test.js b/website/test/components/browser/docs-drawer.test.js index 9c98bd3e9..a079c8529 100644 --- a/website/test/components/browser/docs-drawer.test.js +++ b/website/test/components/browser/docs-drawer.test.js @@ -55,6 +55,13 @@ suite('docs drawer', () => { // anchors. Left alone they navigate the runner page and abort the suite, // so navigation is cancelled in the CAPTURE phase: the default action never // happens, while the listeners under test still see the event. + // + // Deliberately NOT the shared `test/browser-nav-guard.js` (#1135), whose + // whole point is the opposite phase. Capture sets `defaultPrevented` before + // the router's document-level listener runs, so the router bows out, and + // that is exactly what this suite wants: it tests the drawer, not + // navigation, and a live router here would issue real page fetches. Do not + // "unify" this onto the shared helper. blockNav = (e) => { if (e.target.closest?.('a')) e.preventDefault(); }; document.addEventListener('click', blockNav, true); diff --git a/website/test/components/browser/site-nav-menu.test.js b/website/test/components/browser/site-nav-menu.test.js index d109d6fd8..44e103599 100644 --- a/website/test/components/browser/site-nav-menu.test.js +++ b/website/test/components/browser/site-nav-menu.test.js @@ -23,6 +23,13 @@ suite('site nav menu', () => { let blockNav; setup(async () => { + // Cancel real anchor navigation in the CAPTURE phase, deliberately NOT via + // the shared `test/browser-nav-guard.js` (#1135), whose whole point is the + // opposite phase. Capture sets `defaultPrevented` before the router's + // document-level listener runs, so the router bows out, and that is exactly + // what this suite wants: it tests the menu, not navigation, and a live + // router here would issue real page fetches. Do not "unify" this onto the + // shared helper. blockNav = (e) => { if (e.target.closest?.('a')) e.preventDefault(); }; document.addEventListener('click', blockNav, true); From 96211c86886cd8006db853152006ebfa8e2a62fe Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 5 Aug 2026 15:46:52 +0530 Subject: [PATCH 3/4] docs: state the browser-test nav guard rule and its phase reason --- .agents/skills/webjs/references/testing.md | 12 ++++++++++++ packages/core/AGENTS.md | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/.agents/skills/webjs/references/testing.md b/.agents/skills/webjs/references/testing.md index fda287285..a1d666d58 100644 --- a/.agents/skills/webjs/references/testing.md +++ b/.agents/skills/webjs/references/testing.md @@ -24,6 +24,18 @@ Feature folders are primary, and the test kind is a subfolder inside the feature Assert only on what the layer needs. A block that inspects only the HTTP response, the SSR HTML string, headers, or the importmap does NOT need a browser. Keep in the browser suite only blocks that genuinely need a DOM (live state via `page.evaluate`, hydration, client-router nav, slots, view transitions, streaming into the DOM, custom-element upgrade). +### A browser test that clicks a real link or submits a real form MUST cancel the default + +web-test-runner aborts the whole SESSION, not one file, when the page navigates. So a test that clicks a real `` or submits a real `` is a single point of failure for every browser test file you have: whenever the client router loses the race to intercept, the browser performs the real navigation and the run dies reporting `0 failed` and then exiting non-zero, which reads as an infrastructure blip rather than a test problem. Cancel the default so an interception gap fails ONE test on its own assertion. + +Register the canceling listener on `window` in the BUBBLE phase. That is the last step of the propagation path, so it runs after the router's own document-level listeners, and `preventDefault()` still cancels the default action because that action runs only once dispatch completes. + +**Never use the capture phase.** Capture sets `defaultPrevented` before the router ever sees the event, and the router returns immediately on that flag (the same guard that lets a component's `@click` opt out). Every guarded router test then passes while testing nothing. Capture is correct only when suppressing the router is the actual goal, for a test that exercises a menu or a drawer rather than navigation; say so in a comment when you do it, because it looks identical to the mistake. Do not reach for `stopPropagation` either, which hides the click from the router and turns the assertion into a tautology. + +Cancel a form on its `submit` event, not on the submit control's `click`. The form's default action fires on submit, so canceling the click stops the form from ever submitting and the router never sees it. + +One thing this cannot cover: the router assigns `location.href` when it degrades a soft navigation, and `preventDefault` does not cancel a script assignment. Listen for `webjs:navigation-fallback` on `document` and assert none fired; its `cause` is the diagnosis. In the framework repo this all lives in one shared module, `test/browser-nav-guard.js`, whose `installNavGuard()` returns `{ fallbacks, remove }`. + ## App runners (`webjs test`) ```sh diff --git a/packages/core/AGENTS.md b/packages/core/AGENTS.md index 105fda90c..f4f3d2ed5 100644 --- a/packages/core/AGENTS.md +++ b/packages/core/AGENTS.md @@ -236,6 +236,17 @@ organised by feature: `signals/`, `rendering/`, `directives/`, `browser/` subfolder when there are real-browser tests for it (run on Chromium, Firefox, and WebKit via web-test-runner). +A browser test that clicks a real `` or submits a real `` +MUST install the shared guard from `test/browser-nav-guard.js` +(`installNavGuard()`, returning `{ fallbacks, remove }`). web-test-runner +aborts the whole session when the page navigates, so an escaped click +takes down every browser file rather than failing one test. The guard +listens on `window` in the BUBBLE phase, which is load-bearing: capture +would set `defaultPrevented` before the router's document-level listener +runs, and `onClick` returns on that flag, so every guarded router test +would pass while testing nothing. Full rule in +[`references/testing.md`](../../.agents/skills/webjs/references/testing.md). + Cross-package tests that exercise core through the SSR pipeline or scaffolds live at the repo root in `test/ssr/`, `test/scaffolds/`, etc. See [`references/testing.md`](../../.agents/skills/webjs/references/testing.md). From 3fba490c065813e44241302fc37c4599e8c588c3 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 5 Aug 2026 15:59:11 +0530 Subject: [PATCH 4/4] fix(test): resolve the guard's anchor from the composed path, and cover two missed suites Three real gaps from review. The guard resolved the anchor with e.target.closest('a[href]'). The listener is on window, so a click inside an open shadow root arrives retargeted to the host and closest() walks only light-tree ancestors, never finding the link. The guard failed open for exactly the case the router itself handles via composedPath, so it was narrower than the thing it backstops. It now walks the composed path the same way, with a test that navigates the runner page away when reverted. The first sweep for suites needing the guard missed files because the shell glob did not recurse. view-transitions-permanent drives eight real anchor clicks and stream-action calls requestSubmit on a real form; both are now guarded. A pure-fragment href needs no guard, so the ui-a11y suite is correctly left alone. The guard is opt-in per suite, so the comment claiming every browser suite gets it was wrong and now says a new suite has to install it. --- .agents/skills/webjs/references/testing.md | 19 +++++++++++++- .../browser/form-action-submit.test.js | 7 ++--- .../test/routing/browser/nav-guard.test.js | 19 ++++++++++++++ .../routing/browser/stream-action.test.js | 10 +++++++ .../view-transitions-permanent.test.js | 8 ++++++ test/browser-nav-guard.js | 26 ++++++++++++++----- website/app/docs/testing/page.ts | 17 ++++++++++++ 7 files changed, 95 insertions(+), 11 deletions(-) diff --git a/.agents/skills/webjs/references/testing.md b/.agents/skills/webjs/references/testing.md index a1d666d58..a40bfff07 100644 --- a/.agents/skills/webjs/references/testing.md +++ b/.agents/skills/webjs/references/testing.md @@ -34,7 +34,24 @@ Register the canceling listener on `window` in the BUBBLE phase. That is the las Cancel a form on its `submit` event, not on the submit control's `click`. The form's default action fires on submit, so canceling the click stops the form from ever submitting and the router never sees it. -One thing this cannot cover: the router assigns `location.href` when it degrades a soft navigation, and `preventDefault` does not cancel a script assignment. Listen for `webjs:navigation-fallback` on `document` and assert none fired; its `cause` is the diagnosis. In the framework repo this all lives in one shared module, `test/browser-nav-guard.js`, whose `installNavGuard()` returns `{ fallbacks, remove }`. +Resolve the anchor from `e.composedPath()`, not `e.target.closest('a[href]')`. The listener is on `window`, so a click inside a shadow root (a `static shadow = true` component) arrives retargeted to the host, and `closest()` walks only light-tree ancestors and never finds the link, which fails open exactly where the router itself handles the case. A pure-fragment `href="#x"` link needs no guard at all, since it never navigates the page away. + +One thing this cannot cover: the router assigns `location.href` when it degrades a soft navigation, and `preventDefault` does not cancel a script assignment. Listen for `webjs:navigation-fallback` on `document` and assert none fired; its `cause` is the diagnosis. + +It is a few lines, so write it in your suite's setup and detach it in teardown: + +```js +const onClick = (e) => { + for (const el of e.composedPath()) { + if (el instanceof HTMLAnchorElement && el.hasAttribute('href')) { e.preventDefault(); return; } + } +}; +const onSubmit = (e) => e.preventDefault(); +window.addEventListener('click', onClick); // bubble, never capture +window.addEventListener('submit', onSubmit); +``` + +(The WebJs framework repo keeps its own copy of exactly this in one shared module, `test/browser-nav-guard.js`, whose `installNavGuard()` returns `{ fallbacks, remove }`. That module is framework-repo infrastructure and is not part of a scaffolded app.) ## App runners (`webjs test`) diff --git a/packages/core/test/routing/browser/form-action-submit.test.js b/packages/core/test/routing/browser/form-action-submit.test.js index b099c9af6..1b47628be 100644 --- a/packages/core/test/routing/browser/form-action-submit.test.js +++ b/packages/core/test/routing/browser/form-action-submit.test.js @@ -29,9 +29,10 @@ const tick = () => new Promise((r) => setTimeout(r, 20)); suite('Client router: bound form submissions (#1155)', () => { // The navigation backstop this suite used to declare inline now lives in the // shared guard (#1135), which is the same window-bubble listener with the - // same reasoning, so every browser suite gets it rather than the few that - // hand-rolled a copy. See `test/browser-nav-guard.js` for why the phase is - // window bubble and never capture. + // same reasoning. It is installed per suite, not globally, so any NEW suite + // that clicks a real link or submits a real form has to opt in. See + // `test/browser-nav-guard.js` for why the phase is window bubble and never + // capture. let navGuard; let container, origFetch, calls; diff --git a/packages/core/test/routing/browser/nav-guard.test.js b/packages/core/test/routing/browser/nav-guard.test.js index 401136fd4..c3a72c216 100644 --- a/packages/core/test/routing/browser/nav-guard.test.js +++ b/packages/core/test/routing/browser/nav-guard.test.js @@ -118,6 +118,25 @@ suite('Browser-test nav guard (#1135)', () => { } finally { teardown(); } }); + test('blocks a link inside a shadow root, which retargets away from the anchor', async () => { + setup(); + const before = location.pathname; + try { + // A `static shadow = true` component rendering a link. The listener is on + // `window`, so `e.target` here is the HOST, not the anchor, and a + // `target.closest('a[href]')` lookup finds nothing and fails open. The + // guard walks the composed path instead, like the router does. + const host = document.createElement('div'); + container.appendChild(host); + host.attachShadow({ mode: 'open' }).innerHTML = + 'go'; + host.shadowRoot.querySelector('a').click(); + await tick(); + assert.equal(location.pathname, before, + 'the guard must block an anchor inside a shadow root'); + } finally { teardown(); } + }); + test('does NOT suppress the router on a plain link (capture-phase regression)', async () => { setup(); try { diff --git a/packages/core/test/routing/browser/stream-action.test.js b/packages/core/test/routing/browser/stream-action.test.js index da4857b60..b008aaf1e 100644 --- a/packages/core/test/routing/browser/stream-action.test.js +++ b/packages/core/test/routing/browser/stream-action.test.js @@ -19,16 +19,22 @@ import { enableClientRouter } from '../../../src/router-client.js'; import { connectWS } from '../../../src/websocket-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 0)); async function settle() { for (let i = 0; i < 4; i++) await tick(); } suite(' applier (#248)', () => { let host; function setup() { + navGuard = installNavGuard(); host = document.createElement('div'); document.body.appendChild(host); } function teardown() { + navGuard.remove(); host.remove(); // Clean up any stray stream elements a failing case left behind. document.querySelectorAll('webjs-stream').forEach((e) => e.remove()); @@ -153,6 +159,7 @@ suite(' applier (#248)', () => { suite('Client router: content-negotiated stream-action form response (#248)', () => { let host, origFetch, calls; function setup() { + navGuard = installNavGuard(); enableClientRouter(); // idempotent host = document.createElement('div'); document.body.appendChild(host); @@ -160,6 +167,7 @@ suite('Client router: content-negotiated stream-action form response (#248)', () calls = []; } function teardown() { + navGuard.remove(); window.fetch = origFetch; host.remove(); document.querySelectorAll('webjs-stream').forEach((e) => e.remove()); @@ -192,6 +200,7 @@ suite('Client router: content-negotiated stream-action form response (#248)', () suite('Live channel: connectWS message applied by the same applier (#248)', () => { let host, OrigWS, sockets; function setup() { + navGuard = installNavGuard(); host = document.createElement('div'); document.body.appendChild(host); OrigWS = window.WebSocket; @@ -207,6 +216,7 @@ suite('Live channel: connectWS message applied by the same applier (#248)', () = window.WebSocket = FakeWS; } function teardown() { + navGuard.remove(); window.WebSocket = OrigWS; host.remove(); document.querySelectorAll('webjs-stream').forEach((e) => e.remove()); diff --git a/packages/core/test/routing/browser/view-transitions-permanent.test.js b/packages/core/test/routing/browser/view-transitions-permanent.test.js index 5af303770..cce84e677 100644 --- a/packages/core/test/routing/browser/view-transitions-permanent.test.js +++ b/packages/core/test/routing/browser/view-transitions-permanent.test.js @@ -26,6 +26,10 @@ import { enableClientRouter } from '../../../src/router-client.js'; import { assert } from '../../../../../test/browser-assert.js'; +import { installNavGuard } from '../../../../../test/browser-nav-guard.js'; + +/** Shared across the suites below; installed per test in setup(). */ +let navGuard; const tick = () => new Promise((r) => setTimeout(r, 0)); async function settle() { await tick(); await tick(); await tick(); await tick(); } @@ -52,6 +56,7 @@ suite('Client router: View Transitions on partial swaps (#250)', () => { let container, origFetch, origSVT, calls; function setup() { + navGuard = installNavGuard(); enableClientRouter(); container = document.createElement('div'); document.body.appendChild(container); @@ -61,6 +66,7 @@ suite('Client router: View Transitions on partial swaps (#250)', () => { calls = []; } function teardown() { + navGuard.remove(); window.fetch = origFetch; restoreSVT(); setViewTransitionMeta(false); @@ -197,6 +203,7 @@ suite('Client router: data-webjs-permanent persistence (#250)', () => { let container, sibling, origFetch; function setup() { + navGuard = installNavGuard(); enableClientRouter(); container = document.createElement('div'); sibling = document.createElement('div'); @@ -207,6 +214,7 @@ suite('Client router: data-webjs-permanent persistence (#250)', () => { origFetch = window.fetch; } function teardown() { + navGuard.remove(); window.fetch = origFetch; container.remove(); const s = document.getElementById('perm-sibling'); diff --git a/test/browser-nav-guard.js b/test/browser-nav-guard.js index cfa67b2be..85ecba8f1 100644 --- a/test/browser-nav-guard.js +++ b/test/browser-nav-guard.js @@ -11,9 +11,13 @@ * navigation and the run dies with `0 failed` plus exit 1, which reads as an * infrastructure blip rather than a test problem. * - * This makes the browser's default activation structurally impossible while - * leaving the router fully exercised, so an interception gap FAILS one test on - * its own assertion instead of taking down the run. + * This cancels the browser's default activation while leaving the router fully + * exercised, so an interception gap FAILS one test on its own assertion + * instead of taking down the run. + * + * It is opt-in PER SUITE, not global, so a new suite that clicks a real link or + * submits a real form has to install it. A pure-fragment `href="#x"` link needs + * no guard, since it never navigates the page away. * * ## The phase is load-bearing: `window`, BUBBLE, never capture * @@ -50,10 +54,18 @@ export function installNavGuard() { const fallbacks = []; const onClick = (e) => { - // `closest` is guarded because the target can be a text node or the - // document itself, neither of which has it. - const anchor = e.target && e.target.closest && e.target.closest('a[href]'); - if (anchor) e.preventDefault(); + // Walk the COMPOSED path, exactly as the router's `findAnchorInPath` does, + // rather than `e.target.closest('a[href]')`. This listener is on `window`, + // so a click originating inside an open shadow root arrives retargeted to + // the shadow HOST, and `closest()` walks only light-tree ancestors and + // never finds the anchor. The guard would then fail open for a + // `static shadow = true` component rendering a link, which is precisely a + // case the router itself handles, so the backstop must not be narrower + // than the thing it backstops. + const path = typeof e.composedPath === 'function' ? e.composedPath() : []; + for (const el of path) { + if (el instanceof HTMLAnchorElement && el.hasAttribute('href')) { e.preventDefault(); return; } + } }; // Forms need the `submit` event, NOT the click on the submit control. The diff --git a/website/app/docs/testing/page.ts b/website/app/docs/testing/page.ts index 219ecae12..98d36e9d2 100644 --- a/website/app/docs/testing/page.ts +++ b/website/app/docs/testing/page.ts @@ -216,6 +216,23 @@ suite('Example browser tests', () => { }); }); +

Clicking a real link or submitting a real form

+

web-test-runner aborts the whole test session, not one file, when the page navigates. So a browser test that clicks a real <a href> or submits a real <form> is a single point of failure for every browser test you have: if the client router loses the race to intercept, the browser performs the real navigation and the run dies reporting zero failures and then exiting non-zero, which reads as an infrastructure blip rather than a test problem. Cancel the default so an interception gap fails one test on its own assertion.

+ const onClick = (e) => { + // Walk the COMPOSED path: this listener is on window, so a click inside a + // shadow root is retargeted to the host and closest() would never find the + // link. + for (const el of e.composedPath()) { + if (el instanceof HTMLAnchorElement && el.hasAttribute('href')) { e.preventDefault(); return; } + } +}; +const onSubmit = (e) => e.preventDefault(); // forms cancel on submit, not on the button's click + +window.addEventListener('click', onClick); // window BUBBLE phase, never capture +window.addEventListener('submit', onSubmit); +

Register on window in the bubble phase. That is the last step of the propagation path, so it runs after the router's own listeners, and preventDefault() still cancels the default action. Never use the capture phase: it sets defaultPrevented before the router sees the event, and the router bows out on that flag, so every such test would pass while testing nothing. A pure-fragment href="#x" link needs no guard, since it never navigates the page away.

+

One case this cannot cover: the router assigns location.href when it degrades a soft navigation, and preventDefault does not cancel a script assignment. Listen for webjs:navigation-fallback on document and assert none fired; its cause names the reason.

+

Convention Validation

webjs check validates your app for correctness issues:

# Run the correctness checks