Repository navigation
fix(core): a scrolled list keeps its offset when blur moves the root - #7190
Conversation
Chrome resets a scrolled container's offset when the root leaves or enters the document, so the first blurred frame of a fresh page or render worker drew the list at offset 0. Offsets are read at move time and put back before the re-seek, so a container that became scrollable after the scan is covered too. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…nown bug The scroll offset now survives the root's move, so the three engine differential cases and the multi-worker render case expect blur on to equal blur off, and the measured constants for the old symptom go. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 1 important · 0 nits
Fix is correct and ordered right: offsets are read at move time, restored after the move and before the re-seek, so timeline-driven scroll still wins. CI is green except the non-required Comments ratchet, which this PR turns red ↓ inline.
Scope
Read all 5 files. Checked: both moveRootThenResync callers (wrap/unwrap) get the restore; root itself is covered; negative (RTL) scrollLeft passes the !== 0 filter; scroll writes sit outside ignoreMutations but emit no mutation records; no runtime scroll listeners react to the extra event; light-DOM-only walk matches discover.ts:194. The removed boxFromMarkup has no other users. Did not run the browser suites locally, relied on CI success for the head.
— Genesis
| // Chrome resets a scrolled container's offset when its subtree is removed from the document. | ||
| const scrolled = [root, ...root.querySelectorAll<HTMLElement>("*")] | ||
| .filter((el) => el.scrollLeft !== 0 || el.scrollTop !== 0) | ||
| .map((el) => ({ el, left: el.scrollLeft, top: el.scrollTop })); | ||
| (host.ignoreMutations ?? ((write: () => void) => write()))(move); | ||
| for (const { el, left, top } of scrolled) { | ||
| el.scrollLeft = left; | ||
| el.scrollTop = top; | ||
| } |
There was a problem hiding this comment.
important: the new comment line is what turns Comments red (comment-ratchet.mjs: index.ts 8.4% 69/817 vs base 68/809, comment share can only go down). Moving the reason into the name keeps it and clears the check:
| // Chrome resets a scrolled container's offset when its subtree is removed from the document. | |
| const scrolled = [root, ...root.querySelectorAll<HTMLElement>("*")] | |
| .filter((el) => el.scrollLeft !== 0 || el.scrollTop !== 0) | |
| .map((el) => ({ el, left: el.scrollLeft, top: el.scrollTop })); | |
| (host.ignoreMutations ?? ((write: () => void) => write()))(move); | |
| for (const { el, left, top } of scrolled) { | |
| el.scrollLeft = left; | |
| el.scrollTop = top; | |
| } | |
| const offsetsChromeDropsOnDetach = [root, ...root.querySelectorAll<HTMLElement>("*")] | |
| .filter((el) => el.scrollLeft !== 0 || el.scrollTop !== 0) | |
| .map((el) => ({ el, left: el.scrollLeft, top: el.scrollTop })); | |
| (host.ignoreMutations ?? ((write: () => void) => write()))(move); | |
| for (const { el, left, top } of offsetsChromeDropsOnDetach) { | |
| el.scrollLeft = left; | |
| el.scrollTop = top; | |
| } |
…t may only fall Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
There was a problem hiding this comment.
Verdict: APPROVE · 0 open · 8c11c55e → ec6f9002
The delta is the one removed comment line in captureMotionBlur/index.ts. The Comments ratchet is green now. All CI is green; regression shard-6 timed out building its Docker image and passed on rerun.
Merge-order heads-up: this PR conflicts with #7191 in the three pin files, since each removes the last KNOWN_REAL_DIFFERENCES entry. Whichever lands second needs a rebase that empties both maps. require_last_push_approval means it then needs a fresh approval.
— Genesis (Claude)
…eview Adopts heygen-com#7199 (delivery-PCM duration normalization), heygen-com#7194 (browser install retries while Windows holds a file), the Studio marquee and timeline range-selection series, waapi initial-state, lint media and CLI check/contrast fixes, and skills docs. Not adopted with ledger reasons: heygen-com#7203, heygen-com#7207, heygen-com#7202, heygen-com#7197; limitDeliveryPeaks and the neverStyledStarts cache are left out because the fork lacks their prerequisites. Watermark advances to aa34cff / PR heygen-com#7190. Co-authored-by: WaterrrForever <[email protected]> Co-authored-by: Miguel Ángel <[email protected]> Co-authored-by: kevin <[email protected]> Co-authored-by: Jason <[email protected]> Co-authored-by: cooleryu <[email protected]> Co-authored-by: Vincent Yip <[email protected]> Co-authored-by: Vance Ingalls <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
What
A scrolled list lost its scroll offset on the first blurred frame of a fresh page or a fresh render worker: Chrome resets a scroll container's offset when its subtree is removed from the document, which is what moving the root into the capture canvas does.
moveRootThenResyncnow records every non-zero scroll offset under the root before the move and restores it after.Found by the pixel invariants in #7108 (
scroll-listfixture, outside the trail blur-on != blur-off at 538 px). This PR flips those pins: the engineoutside:scroll-list@1 / 1.2 / 1.5cases and the producer scroll-list render case are now ordinary "outside the trail, blur on equals blur off" cases with zero bad tiles.Verification
index.test.ts: a scroll container with an offset keeps it across a wrap and an unwrap; fails without the change.scrollLeft/scrollTopof every element under the root once per wrap or unwrap (a render with plain frames does this per frame). Measured in Chrome 153 with a dirty layout each time: 0.8 ms at 500 nodes, 1.5 ms at 3000, 4.6 ms at 10000.Supersedes the scroll-offset part of the draft #7038, which only found the containers once after the scan.
🤖 Generated with Claude Code