Harden Navigation API interception in the content loader

[philipwalton]

Problem

The navigate event handler in src/javascript/content-loader.ts (init()) lacked the standard Navigation API guards:

• No event.canIntercept check — calling intercept() on an uninterceptable navigation throws.
• No event.hashChange check — same-page fragment navigations (e.g. clicking a footnote or heading anchor) were intercepted and re-fetched the entire page partial pointlessly.
• No event.downloadRequest check — download-triggering links shouldn't be intercepted.
• No event.formData check — no forms exist today, but the guard is standard and free.
• The resource check url.pathname.match(/\.(png|svg|webp)$/) was a fragile blacklist that missed .jpg, .gif, .pdf, .xml, /site.webmanifest, etc.

Root cause

The handler was written before/without the standard guard checklist for navigate interception, and the resource check enumerated file extensions instead of positively identifying SPA-navigable pages.

What changed

• src/javascript/content-loader.ts: early-return when !event.canIntercept, when event.hashChange, when event.downloadRequest !== null, and when event.formData is truthy. Replaced the extension blacklist with a positive check: only intercept when url.pathname.endsWith('/'), since every SPA-navigable page on this site has a directory-style path (the partial loader appends index.prtl). Anything else falls back to a normal full-page navigation, which is always safe. Verified against dist/ that every built page is a directory-style index.html (the only exceptions are 404.html and the __reset/__blank test helpers, none of which are SPA-navigation targets).
• src/javascript/types.d.ts: added canIntercept, hashChange, downloadRequest, and formData to the hand-rolled NavigateEvent interface with spec-correct types.
• test/e2e/content-loading.ts: new test "should not re-fetch content for same-page fragment navigations" — SPA-navigates to an article, clicks a heading anchor, and asserts (via resource-timing entries) that no additional index.prtl fetch was issued and no full page load occurred. This test fails on main (the fragment click re-fetches the partial) and passes with this change.

Verification

• npm run lint — pass (0 errors)
• npm run types:check — pass
• npm run test:unit — pass (16 files, 67 tests)
• npm run build — pass
• npm test (full e2e) — results:
  • code-highlighting, worker, and log specs: all pass.
  • homepage.ts "should contain working links to all published articles": known pre-existing failure (atom.xml sort-order bug, fixed separately in fix/atom-feed-order). Tolerated per instructions.
  • content-loading.ts "should not attempt to load non-HTML content": fails, but this is pre-existing and unrelated — it fails identically (3/3 runs) on unmodified origin/main in the same environment. The test picks articles[28] from dist/atom.xml; the same sort-order bug shifts the index to an article with no figure img. I verified that once the feed order is fixed, articles[28] resolves to /articles/the-ga-setup-i-use-on-every-site-i-build/ (which has 8 figure images), and I confirmed by temporarily pointing the test at that article that it passes 3/3 with this change — i.e. clicking an image link to a .webp URL correctly falls through to a full-page navigation under the new trailing-slash check.
  • content-loading.ts "should show an error if the content cannot be loaded": pre-existing intermittent flake (observed failing 1/3 on unmodified origin/main and passing 3/3 with this change in other runs); its automatic retry in full-suite runs is often cancelled because bail: 1 triggers once the known homepage failure exhausts its retries.

Codex review

Verdict: REQUEST_CHANGES, with one finding I'm overruling as non-blocking (recorded here per process):

• GET form submissions: event.formData is only non-null for POST form submissions, so a same-origin GET form targeting a /-terminated action would still be intercepted (and its query string dropped by the pathname-based partial fetch). This is technically correct per spec, but the site has no forms (the only <form> in the repo is inside an article's code sample), this matches the standard documented guard pattern, and query strings being ignored by the partial loader is pre-existing behavior that equally affects plain links. If forms are ever added, a sourceElement-based check would be worth revisiting.
• Non-blocking: the getPartialFetchCount helper matches any resource name containing index.prtl (could parse the URL and match same-origin /index.prtl paths); no GET-form regression test (moot while no forms exist).

Please scrutinize

• The trailing-slash rule: this change assumes any URL intended to be SPA-navigable ends in /. If a page is ever added with a non-directory path, it will silently get full-page navigations instead of SPA loads (safe, but not SPA). Please confirm this invariant is one you're happy to commit to.
• Fragment navigations no longer re-fetch partials: same-page hash clicks are now handled natively by the browser (scroll + history entry) instead of being intercepted. Verify you're happy with the native scroll behavior for heading anchors/footnotes.
• The now-redundant url.origin !== location.origin check was kept (canIntercept already excludes cross-origin) to keep the diff minimal.

🤖 Generated with Claude Code