fix: a <webjs-frame> swap no longer scrolls the page to top - #1429
fix: a <webjs-frame> swap no longer scrolls the page to top#1429vivek7405 wants to merge 3 commits into
Conversation
The router's scroll block was gated on `recordHistory` alone, which means "a foreground navigation the reader initiated". A click on a frame-driving link is exactly that, since a frame click advances the URL deliberately, so a frame swap fell into the page-navigation scroll by omission rather than by decision. On gallery.webjs.dev/features/frames, clicking a filter tab from 400px threw the reader back to the top with the panel they had just clicked in off screen. A frame swaps ONE region and leaves the rest of the document standing, the scroll offset included, so the router now writes no scroll for it. That is the same rule `restoreGeneration` already applies when deciding what ends a scroll-restore window, so the two paths agree on what a frame nav is. Turbo, which <webjs-frame> is modelled on, likewise never scrolls on a frame navigation. The hash branch is excluded too: a #anchor on a frame link is no more a request to move the document viewport than the swap itself is. `_top` and an unresolvable data-webjs-frame id both resolve to a null frame id, so they stay page navigations and still scroll to top.
A page swap merges the incoming head and removes any live head element the response does not carry, so the empty <head> in the page-navigation fixture stripped web-test-runner's own session scripts out of the page. The file itself still passed, having already loaded; the damage showed up as unrelated later files failing to start a test page on Firefox, which reads as an infrastructure blip rather than a fixture bug.
Its last statement is the precondition assertion, so a failure there left the fetch stub and the nav guard installed for the rest of the RUN rather than the rest of the file. That damage is invisible where it happens and surfaces as unrelated later files misbehaving, which is the same shape the head handling in this fixture already produced once. Teardown now guards each step so it can clean up a partially-installed setup.
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Read this one cold, looking for the ways a guard like this over-applies.
The change itself is the right shape and the smallest one available: frameId was already a parameter of fetchAndApply, so no plumbing, and the rule it introduces is one scroll.js already states for the restore window. What I wanted to be sure of is that a non-null frameId can never end up applying a PAGE swap, because then skipping the scroll would strand a reader mid-document on a page they have never seen. It cannot: applySwap's frame branch either swaps the subtree or returns 'none' with a cancelable webjs:frame-missing, and it deliberately refuses to fall through to a full-body replace. The build-mismatch hard reload at the top of the same function is likewise gated on !frameId. So a frame nav has exactly two outcomes, and neither one wants a scroll.
The one thing I did fix is in my own fixture, and it is the second time this file bit in the same way. Worth stating plainly since it is a fixture-authoring trap the next person will hit: a page-swap response with an empty <head> makes the router's head merge strip every live head element the response does not carry, web-test-runner's own session scripts included, so the file passes while a later unrelated file fails to start a test page on Firefox. The fixture now echoes the live head back. Same class of problem in the setup ordering, flagged inline.
Two behaviour notes I made deliberately and would want challenged if anyone disagrees. The hash branch is skipped for frames too, so a #anchor on a frame link no longer scrolls, which matches Turbo and keeps this to one rule instead of two. And Turbo's autoscroll has no equivalent here; the docs now say so, since an unexplained absence reads as an oversight.
1 participant
Closes #1427
A
<webjs-frame>swap scrolled the whole window to top, so filtering a panel below the fold threw the reader back to the top of the page with the panel they had just clicked in off screen. Reproduced on https://gallery.webjs.dev/features/frames: scroll to 400, click a filter tab, land at 0.What changed
The scroll block in
fetchAndApplywas gated onrecordHistoryalone, which means "a foreground navigation the reader initiated". A click on a frame-driving link is exactly that, since a frame click advances the URL deliberately, so a frame swap fell into the page-navigation scroll by omission rather than by decision. It is now gated onrecordHistory && !frameId.A frame swaps ONE region and leaves the rest of the document standing, the scroll offset included, so the router writes no scroll for it. That is the same rule
restoreGenerationinscroll.jsalready applies when deciding what ends a scroll-restore window, so the two paths now agree on what a frame nav is. Turbo, which<webjs-frame>is modelled on, likewise never scrolls on a frame navigation.The hash branch is excluded too. A
#anchoron a frame link is no more a request to move the document viewport than the swap itself is, and one rule ("a frame swap never moves the window") beats two.Both callers that could reach the block with a frame id are covered: a link click through
performNavigation, and a frame-targeted form submission throughsubmitForm, which hardcodesrecordHistory: true.loadFrame(a<webjs-frame src>self-load) and the background revalidation already passedrecordHistory: false.Deliberately not in scope
Turbo's
autoscrollopt-in, which scrolls the frame itself into view on swap. This is only about the router not moving the reader unasked; the docs now say the absence is a decision.Test plan
packages/core/test/routing/browser/frame-swap-scroll.test.js, 5 cases across Chromium, Firefox, and WebKit: a nested frame link, an externaldata-webjs-framelink, and a frame-targeted form submission all hold the offset; a_topbreakout and an unresolvable frame id are page navigations and still scroll to top.test/e2e/e2e.test.mjs, over the real wire against the blog's/frame-demo.scrollYto 0 and look exactly like the defect), the starting offset is asserted before the click, and the page-nav case asserts the destination is still tall enough to hold the old offset.Docs
.agents/skills/webjs/references/client-router-and-streaming.md, the<webjs-frame>section.website/app/docs/client-router/page.ts, a new subsection under the frame docs.Note for whoever merges second
fix/ios-back-swipe-snapshot-abis editing this sameif (recordHistory)block infetch-apply.js(a temporary #1428 diagnostic that extracts it into a thunk). Whichever lands second gets a small textual conflict there.