fix(router): a click the page already handled is not a navigation

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/router/src/client.tsmodified+16-1

Changes to packages/router/src/client.ts

		⋯1,354 linesShow the 1,354 lines above this hunk@@ -1355,6 +1355,21 @@ else {
1355	1355	// mode where every page is part of the same SPA shell.
1356	1356	document.addEventListener('click',function(e){
1357	1357	if(e.metaKey||e.ctrlKey||e.shiftKey||e.altKey||e.button!==0)return;
	1358	+ // A handler on the page has already decided this click is not a
	1359	+ // navigation, so the router must not perform one - the same contract the
	1360	+ // browser honours for a plain anchor.
	1361	+ //
	1362	+ // This listener used to run in the CAPTURE phase, which broke that twice
	1363	+ // over: it saw the click before the anchor did, so defaultPrevented was
	1364	+ // necessarily false by the time it was read here, and the stopPropagation
	1365	+ // below then stopped the event ever reaching the element - so a page's own
	1366	+ // click handler on an intercepted link never ran AT ALL. Progressive
	1367	+ // enhancement on an anchor was impossible; the only way out was to swap it
	1368	+ // for a button in script (stacksjs/stacks#2393).
	1369	+ //
	1370	+ // Bubbling instead puts the page's handlers first, which is where a
	1371	+ // decision about the page's own markup belongs.
	1372	+ if(e.defaultPrevented)return;
1358	1373	if(!e.target||!e.target.closest)return;
1359	1374	var link=e.target.closest('[data-stx-link]');
1360	1375	// A marked link still has to clear the opt-out set. It used to skip it
		⋯11 linesShow the 11 lines above this hunk@@ -1372,7 +1387,7 @@ else {
1372	1387	e.stopPropagation();
1373	1388	log('[router] navigating to:',href);
1374	1389	navigate(withCurrentLocale(href));
1375		- },true);
	1390	+ });
1376	1391
1377	1392	// ── Form submission ──
1378	1393	// A submit IS a navigation, and the router only ever watched clicks. So any

packages/router/test/link-preventdefault.test.tsadded+161-0

Changes to packages/router/test/link-preventdefault.test.ts

		@@ -0,0 +1,161 @@
	1	+/**
	2	+ * The router must not navigate a click the page already handled
	3	+ * (stacksjs/stacks#2393).
	4	+ *
	5	+ * The click listener ran in the CAPTURE phase, which broke the contract twice
	6	+ * over. It saw the click before the anchor did, so `e.defaultPrevented` was
	7	+ * necessarily false by the time the router read it - and it read it only to
	8	+ * log it, never to branch on it. Worse, the `stopPropagation()` it then called
	9	+ * stopped the event ever reaching the element, so a page's own click handler
	10	+ * on an intercepted link never ran at all: not a late handler, not a losing
	11	+ * handler, no handler.
	12	+ *
	13	+ * The visible symptom was an anchor that navigated away instead of opening the
	14	+ * inline form its handler was there to render. The only workaround was to
	15	+ * replace the anchor with a <button> in script, which costs the no-JS reader
	16	+ * the link.
	17	+ *
	18	+ * Each case is paired with a control that must STILL be intercepted. A probe
	19	+ * reporting "not intercepted" for both is measuring nothing.
	20	+ */
	21	+import { afterEach, describe, expect, it } from 'bun:test'
	22	+import { Window } from 'very-happy-dom'
	23	+import { getRouterScript } from '../src/client'
	24	+
	25	+const originalGlobals = {
	26	+ window: globalThis.window,
	27	+ document: globalThis.document,
	28	+ location: globalThis.location,
	29	+ history: globalThis.history,
	30	+ fetch: globalThis.fetch,
	31	+ CustomEvent: globalThis.CustomEvent,
	32	+ Event: globalThis.Event,
	33	+ MouseEvent: globalThis.MouseEvent,
	34	+ DOMParser: globalThis.DOMParser,
	35	+}
	36	+
	37	+afterEach(() => {
	38	+ Object.assign(globalThis, originalGlobals)
	39	+})
	40	+
	41	+interface Harness {
	42	+ window: any
	43	+ /** URLs the router actually fetched, i.e. navigations it claimed. */
	44	+ navigations: string[]
	45	+}
	46	+
	47	+function installRouter(body: string, config: Record<string, unknown> = {}): Harness {
	48	+ const navigations: string[] = []
	49	+ const window = new Window({ url: 'http://localhost/' })
	50	+ window.document.write(`<html><head></head><body><main>Home</main>${body}</body></html>`)
	51	+ ;(window as any).stx = {}
	52	+ ;(window as any).__stxRouterConfig = {
	53	+ cache: false,
	54	+ prefetch: false,
	55	+ progress: false,
	56	+ viewTransitions: false,
	57	+ ...config,
	58	+ }
	59	+
	60	+ Object.assign(globalThis, {
	61	+ window,
	62	+ document: window.document,
	63	+ location: window.location,
	64	+ history: window.history,
	65	+ CustomEvent: window.CustomEvent,
	66	+ Event: window.Event,
	67	+ MouseEvent: window.MouseEvent,
	68	+ DOMParser: window.DOMParser,
	69	+ fetch: async (url: string) => {
	70	+ navigations.push(String(url))
	71	+ return new Response('<section>next</section>', {
	72	+ status: 200,
	73	+ headers: { 'Content-Type': 'text/html' },
	74	+ })
	75	+ },
	76	+ })
	77	+
	78	+ new Function(getRouterScript())()
	79	+
	80	+ return { window, navigations }
	81	+}
	82	+
	83	+function click(harness: Harness, selector: string): void {
	84	+ const element = harness.window.document.querySelector(selector)
	85	+ if (!element)
	86	+ throw new Error(`no element matched ${selector}`)
	87	+ element.dispatchEvent(new harness.window.MouseEvent('click', { bubbles: true, cancelable: true }))
	88	+}
	89	+
	90	+/** Binds the progressive-enhancement handler the issue describes. */
	91	+function enhance(harness: Harness, selector: string, ran: { value: boolean }): void {
	92	+ harness.window.document.querySelector(selector).addEventListener('click', (event: any) => {
	93	+ ran.value = true
	94	+ event.preventDefault()
	95	+ })
	96	+}
	97	+
	98	+describe('a page handler that cancels the click (#2393)', () => {
	99	+ describe('a marked [data-stx-link] anchor', () => {
	100	+ it('control: without a handler the router still claims it', () => {
	101	+ const harness = installRouter(`<a id="t" data-stx-link href="/partner">Go</a>`)
	102	+ click(harness, '#t')
	103	+ expect(harness.navigations).toEqual(['/partner'])
	104	+ })
	105	+
	106	+ it('runs the page handler at all', () => {
	107	+ // Under capture + stopPropagation the event never reached the anchor,
	108	+ // so this was false: the handler did not lose the race, it never ran.
	109	+ const harness = installRouter(`<a id="t" data-stx-link href="/partner">Go</a>`)
	110	+ const ran = { value: false }
	111	+ enhance(harness, '#t', ran)
	112	+ click(harness, '#t')
	113	+ expect(ran.value).toBe(true)
	114	+ })
	115	+
	116	+ it('does not navigate once that handler has cancelled it', () => {
	117	+ const harness = installRouter(`<a id="t" data-stx-link href="/partner">Go</a>`)
	118	+ enhance(harness, '#t', { value: false })
	119	+ click(harness, '#t')
	120	+ expect(harness.navigations).toEqual([])
	121	+ })
	122	+ })
	123	+
	124	+ describe('an ordinary anchor under interceptAllLinks', () => {
	125	+ const staticSite = { interceptAllLinks: true }
	126	+
	127	+ it('control: without a handler the router still claims it', () => {
	128	+ const harness = installRouter(`<a id="t" href="/partner">Go</a>`, staticSite)
	129	+ click(harness, '#t')
	130	+ expect(harness.navigations).toEqual(['/partner'])
	131	+ })
	132	+
	133	+ it('runs the page handler at all', () => {
	134	+ const harness = installRouter(`<a id="t" href="/partner">Go</a>`, staticSite)
	135	+ const ran = { value: false }
	136	+ enhance(harness, '#t', ran)
	137	+ click(harness, '#t')
	138	+ expect(ran.value).toBe(true)
	139	+ })
	140	+
	141	+ it('does not navigate once that handler has cancelled it', () => {
	142	+ const harness = installRouter(`<a id="t" href="/partner">Go</a>`, staticSite)
	143	+ enhance(harness, '#t', { value: false })
	144	+ click(harness, '#t')
	145	+ expect(harness.navigations).toEqual([])
	146	+ })
	147	+
	148	+ it('leaves an uncancelled handler on a sibling link alone', () => {
	149	+ // The guard reads the event, not the element, so one enhanced link must
	150	+ // not stop the next plain one from routing.
	151	+ const harness = installRouter(
	152	+ `<a id="a" href="/partner">Claim</a><a id="b" href="/browse">Browse</a>`,
	153	+ staticSite,
	154	+ )
	155	+ enhance(harness, '#a', { value: false })
	156	+ click(harness, '#a')
	157	+ click(harness, '#b')
	158	+ expect(harness.navigations).toEqual(['/browse'])
	159	+ })
	160	+ })
	161	+})