You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Clicking a link that drives a <webjs-frame> scrolls the whole window to the top, even though only one region swapped. A frame swap is explicitly not a navigation of the document, so the reader's place in the page must survive it.
Reproduced live on https://gallery.webjs.dev/features/frames (the filter tabs live inside <webjs-frame id="tasks">): scroll to window.scrollY === 400, click the "Active" tab, and window.scrollY is 0 while everything outside the frame is untouched. On a page where the frame sits below the fold, the region the reader just clicked in is thrown off screen.
Turbo, the prior art <webjs-frame> is modelled on, does not scroll on a frame navigation at all; its FrameController leaves the viewport alone and only scrolls when the frame opts in with autoscroll.
Design / approach
The scroll block in fetchAndApply is gated on recordHistory alone. That flag means "this is a foreground navigation the reader initiated", which is true of a click-driven frame nav too (a frame click DOES advance the URL, deliberately), so a frame swap falls into the page-navigation scroll behaviour by omission rather than by decision.
The fix is to gate the block on recordHistory && !frameId: a frame-scoped response swaps one region and leaves the rest of the document standing, so the router must write no window scroll for it, exactly the reasoning restoreGeneration already uses to exclude frame navs from ending a scroll-restore window (packages/core/src/router-client/scroll.js, and the .agents/skills/webjs/references/client-router-and-streaming.md bullet on it). Two paths in the router currently disagree about whether a frame nav is a page nav; this makes them agree.
That includes the hash branch. A #anchor on a frame link is not a request to move the document viewport any more than the frame swap itself is, and Turbo scrolls in neither case. Skipping the whole block keeps one rule ("a frame swap never moves the window") instead of two.
Non-goal: Turbo's autoscroll opt-in (scroll the frame into view on swap). That is a separate feature; this issue is only about the router not moving the reader unasked. Say so in the docs prose so the absence reads as a decision.
Implementation notes (for the implementing agent)
Where to edit
packages/core/src/router-client/fetch-apply.js, the if (recordHistory) { ... } scroll block at ~L343-361 inside fetchAndApply (signature at L65: fetchAndApply(href, frameId, recordHistory, ...)). frameId is already a parameter in scope, so the change is the guard plus a comment explaining why a frame nav is excluded.
Which call sites this affects (verified by grep, all four fetchAndApply callers)
navigator.js:603, performNavigation -> a click on a link inside a frame or carrying data-webjs-frame="<id>". recordHistory is !isPopState && !refresh, so a foreground frame click hits the bug. THIS is the reported case.
navigator.js:705, submitForm -> a frame-targeted form submission, recordHistory hardcoded true. Same bug, same fix, and it must be covered too: a filter form inside a frame currently jumps the page on submit.
navigator.js:583 (background revalidation) and navigator.js:225 (loadFrame, a <webjs-frame src> self-load) both pass recordHistory: false and are already correct. loadFrame in particular is the existing precedent that a frame fill writes no scroll.
Do NOT touch the cache-miss popstate scrollTo in navigator.js:596. It runs BEFORE fetchAndApply and is the browser-native-restore fallback for a page pop; a frame-history pop is a different question and is out of scope here. Widening the change there risks the dogfood: back-button scroll restores ~763px too low on pages that grow after swap #1310 restore behaviour.
Do NOT reach for currentNavigationToken or restoreGeneration to detect "is this a frame nav". frameId is the direct signal and is already the parameter. scroll.js documents at length why the nav token is the wrong proxy for frame-ness.
The _top breakout and an unresolvable data-webjs-frame id both resolve to frameId === null in resolveTargetFrameId (frames.js), so they stay page navigations and keep scrolling to top. That is correct and a test should pin it, since it is the one way this change could silently over-apply.
The scroll block also calls warnIfSmoothScrollOnHtml(). Skipping the block skips that warning for frame navs, which is right (nothing scrolls, so there is nothing to warn about), but do not leave the warning call outside the guard by accident.
e2e/Bun resolve the BUILT packages/core/dist bundle, so rebuild it before running those layers or the counterfactual passes vacuously.
Invariants to respect
A frame swap changes one region only. Anything outside <webjs-frame id> (including the scroll position) must be observably unchanged, which is what the existing e2e sentinel-outside-the-frame assertions already encode.
Progressive enhancement: with JS off the same link is a full-page navigation and the browser scrolls to top natively. That is unchanged and expected; the two paths differ here because one IS a document navigation and the other is not.
Tests (every applicable layer, per AGENTS.md)
Browser (headline assertion): packages/core/test/routing/browser/frame-targeting.test.js is the natural home (it already builds a <webjs-frame id="content"> fixture with nested, external, and _top links and stubs fetch). Add: scroll the document, click a frame-driving link, assert window.scrollY is unchanged; then click the _top breakout link and assert it DOES scroll to top. The fixture needs enough height to scroll, so give the container a tall spacer.
Browser: the same for a frame-targeted <form> submission (the navigator.js:705 path), since that caller is a separate code path into the same block.
e2e: test/e2e/e2e.test.mjs already drives /frame-demo in examples/blog/app/frame-demo/page.ts with external tab links (see the #252 block around L2942 and the frame-prefetch test at L2626). Add a real-wire assertion: scroll, click a tab, assert window.scrollY held. The fixture page may need a tall block so the page is scrollable at the harness viewport.
Counterfactual: reverting the guard must fail the new browser test (scrollY back to 0).
Bun parity is NOT required: the change is browser-side router code with no runtime-sensitive server surface (no serializer, listener, dispatch, or crypto path). State that in the PR so the require-bun-parity-with-runtime-src gate reasoning is on the record.
Docs surfaces
.agents/skills/webjs/references/client-router-and-streaming.md, the <webjs-frame> section (~L131-146): state that a frame swap never moves the window scroll, and that _top / an unresolvable id are page navigations that do. Mention the autoscroll non-goal.
website/app/docs/client-router/page.ts (the docs site's frame coverage) for the same sentence.
AGENTS.md's client-navigation paragraph mentions frames; add the scroll rule only if it reads naturally there, since that file is deliberately lean.
Run the webjs-doc-sync skill so no surface is missed.
Acceptance criteria
Clicking a link that drives a <webjs-frame> leaves window.scrollY unchanged, on both a nested link and an external data-webjs-frame="<id>" link
A frame-targeted form submission likewise leaves window.scrollY unchanged
A data-webjs-frame="_top" breakout link, an unresolvable frame id, and an ordinary page navigation all still scroll to top
A <webjs-frame src> self-load still writes no scroll (unchanged behaviour, pinned)
Reproduced-on-gallery case is fixed: scroll /features/frames, click a filter tab, the reader stays put
A counterfactual proves the new test actually fires when the guard is reverted
Tests at every layer the change touches (browser + e2e)
Docs updated: the skill reference and the docs site say a frame swap never moves window scroll
Problem
Clicking a link that drives a
<webjs-frame>scrolls the whole window to the top, even though only one region swapped. A frame swap is explicitly not a navigation of the document, so the reader's place in the page must survive it.Reproduced live on https://gallery.webjs.dev/features/frames (the filter tabs live inside
<webjs-frame id="tasks">): scroll towindow.scrollY === 400, click the "Active" tab, andwindow.scrollYis0while everything outside the frame is untouched. On a page where the frame sits below the fold, the region the reader just clicked in is thrown off screen.Turbo, the prior art
<webjs-frame>is modelled on, does not scroll on a frame navigation at all; itsFrameControllerleaves the viewport alone and only scrolls when the frame opts in withautoscroll.Design / approach
The scroll block in
fetchAndApplyis gated onrecordHistoryalone. That flag means "this is a foreground navigation the reader initiated", which is true of a click-driven frame nav too (a frame click DOES advance the URL, deliberately), so a frame swap falls into the page-navigation scroll behaviour by omission rather than by decision.The fix is to gate the block on
recordHistory && !frameId: a frame-scoped response swaps one region and leaves the rest of the document standing, so the router must write no window scroll for it, exactly the reasoningrestoreGenerationalready uses to exclude frame navs from ending a scroll-restore window (packages/core/src/router-client/scroll.js, and the.agents/skills/webjs/references/client-router-and-streaming.mdbullet on it). Two paths in the router currently disagree about whether a frame nav is a page nav; this makes them agree.That includes the hash branch. A
#anchoron a frame link is not a request to move the document viewport any more than the frame swap itself is, and Turbo scrolls in neither case. Skipping the whole block keeps one rule ("a frame swap never moves the window") instead of two.Non-goal: Turbo's
autoscrollopt-in (scroll the frame into view on swap). That is a separate feature; this issue is only about the router not moving the reader unasked. Say so in the docs prose so the absence reads as a decision.Implementation notes (for the implementing agent)
Where to edit
packages/core/src/router-client/fetch-apply.js, theif (recordHistory) { ... }scroll block at ~L343-361 insidefetchAndApply(signature at L65:fetchAndApply(href, frameId, recordHistory, ...)).frameIdis already a parameter in scope, so the change is the guard plus a comment explaining why a frame nav is excluded.fetch-apply.jsis a module under therouter-client.jsbarrel (refactor(framework): overhaul WebJs framework architecture following SOLID, KISS, and DRY principles #1365), so the public export surface is untouched.Which call sites this affects (verified by grep, all four
fetchAndApplycallers)navigator.js:603,performNavigation-> a click on a link inside a frame or carryingdata-webjs-frame="<id>".recordHistoryis!isPopState && !refresh, so a foreground frame click hits the bug. THIS is the reported case.navigator.js:705,submitForm-> a frame-targeted form submission,recordHistoryhardcodedtrue. Same bug, same fix, and it must be covered too: a filter form inside a frame currently jumps the page on submit.navigator.js:583(background revalidation) andnavigator.js:225(loadFrame, a<webjs-frame src>self-load) both passrecordHistory: falseand are already correct.loadFramein particular is the existing precedent that a frame fill writes no scroll.refreshPage(Morph page/layout edits in place in dev instead of a full reload #1398) passesrecordHistory: falseand is unaffected.Landmines / gotchas
scrollToinnavigator.js:596. It runs BEFOREfetchAndApplyand is the browser-native-restore fallback for a page pop; a frame-history pop is a different question and is out of scope here. Widening the change there risks the dogfood: back-button scroll restores ~763px too low on pages that grow after swap #1310 restore behaviour.currentNavigationTokenorrestoreGenerationto detect "is this a frame nav".frameIdis the direct signal and is already the parameter.scroll.jsdocuments at length why the nav token is the wrong proxy for frame-ness._topbreakout and an unresolvabledata-webjs-frameid both resolve toframeId === nullinresolveTargetFrameId(frames.js), so they stay page navigations and keep scrolling to top. That is correct and a test should pin it, since it is the one way this change could silently over-apply.warnIfSmoothScrollOnHtml(). Skipping the block skips that warning for frame navs, which is right (nothing scrolls, so there is nothing to warn about), but do not leave the warning call outside the guard by accident.packages/core/distbundle, so rebuild it before running those layers or the counterfactual passes vacuously.Invariants to respect
<webjs-frame id>(including the scroll position) must be observably unchanged, which is what the existing e2e sentinel-outside-the-frame assertions already encode.Tests (every applicable layer, per AGENTS.md)
packages/core/test/routing/browser/frame-targeting.test.jsis the natural home (it already builds a<webjs-frame id="content">fixture with nested, external, and_toplinks and stubs fetch). Add: scroll the document, click a frame-driving link, assertwindow.scrollYis unchanged; then click the_topbreakout link and assert it DOES scroll to top. The fixture needs enough height to scroll, so give the container a tall spacer.<form>submission (thenavigator.js:705path), since that caller is a separate code path into the same block.test/e2e/e2e.test.mjsalready drives/frame-demoinexamples/blog/app/frame-demo/page.tswith external tab links (see the#252block around L2942 and the frame-prefetch test at L2626). Add a real-wire assertion: scroll, click a tab, assertwindow.scrollYheld. The fixture page may need a tall block so the page is scrollable at the harness viewport.scrollYback to 0).require-bun-parity-with-runtime-srcgate reasoning is on the record.Docs surfaces
.agents/skills/webjs/references/client-router-and-streaming.md, the<webjs-frame>section (~L131-146): state that a frame swap never moves the window scroll, and that_top/ an unresolvable id are page navigations that do. Mention theautoscrollnon-goal.website/app/docs/client-router/page.ts(the docs site's frame coverage) for the same sentence.AGENTS.md's client-navigation paragraph mentions frames; add the scroll rule only if it reads naturally there, since that file is deliberately lean.webjs-doc-syncskill so no surface is missed.Acceptance criteria
<webjs-frame>leaveswindow.scrollYunchanged, on both a nested link and an externaldata-webjs-frame="<id>"linkwindow.scrollYunchangeddata-webjs-frame="_top"breakout link, an unresolvable frame id, and an ordinary page navigation all still scroll to top<webjs-frame src>self-load still writes no scroll (unchanged behaviour, pinned)/features/frames, click a filter tab, the reader stays put