fix(threads): keep thread history painted after scroll corrections - #493
Conversation
Mac WebKit can leave a scroller unpainted when native momentum overlaps an instant programmatic scroll correction (WebKit 262287 class). The thread view hit this when older replies were prepended mid-flick: the anchor correction landed, DOM coverage was complete, and the viewport stayed blank for most of a second. Apply the thread's two automatic corrections through a small helper that interrupts momentum the same way the Virtua patch does for ChannelTimeline: on Mac WebKit only, hide vertical overflow for one task around the correction, then restore the prior declaration. The thread scroller keeps a stable scrollbar gutter so the toggle cannot re-wrap rows. Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
One public-material change needed:
- PR description → Evidence: Remove the internal live-relay channel name. “A real 47-reply thread with media” preserves the useful test context without exposing a workspace identifier in this public PR.
No functional changes requested in the four-file diff; the helper, both automatic-correction callers, and stable gutter follow the existing momentum-interruption contract.
Star Lord’s automated source review via Wes’s account (wesbillman), head d8072e8c0571629a2367e8677a48253402bca337, base 5e24150216316a0ee6df3c112685eb7127e0ce77. No code, tests, or app executed. Hosted required CI/DCO passed; Windows validation was skipped. Reported native trials are author evidence, not independently verified signed-package or human acceptance.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No material correctness or security blockers found. Reviewed head d8072e8c0571629a2367e8677a48253402bca337 against target base 5e24150216316a0ee6df3c112685eb7127e0ce77 (diff merge base bc542f284710fbbdfc25b6a7de4951894fb17306), including an independent helper/CSS review.
- Traced both correction callers through anchor capture, reader input, layout and scroll-state updates. The change preserves those contracts and stays within the stated scope.
- Existing required CI passed. A focused actual-ThreadPanel fixture probe in Mac WebKit and Chromium verified prepend visibility, unchanged content width, overflow restoration and continued scrolling on a fresh wheel gesture. The Mac WebKit helper branch executed; Chromium remained untouched. A merge-base control reproduced the same resulting geometry.
- This does not independently verify native momentum/compositor painting or signed-package acceptance. The reported native trials remain author evidence; physical trackpad acceptance should assess the documented braking tradeoff.
Comment-only review, not approval.
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Thanks for this. No blocking findings at d8072e8c0571629a2367e8677a48253402bca337.
correctScrollTop mirrors the Virtua patch's interruptMomentum: it captures value and priority, sets overflow-y: hidden !important for one task, restores only if the declaration is still the one it wrote, and an overlapping correction restores its predecessor before capturing again. The WeakMap keeps scrollers independent, and an original hidden (including hidden !important) comes back with its priority. The platform check isn't textually identical to the patch (!maxTouchPoints instead of excluding Virtua's iOS detector), but both exclude iOS/iPadOS, Chromium, Firefox, WebKitGTK and WebView2. Both relative scrollTop += corrections in ThreadPanel now go through the helper, and scrollbar-gutter: stable lands on .threadHistory, the only scroller the helper toggles, same requirement as .feed.
Beyond the source read: at this head, scroll-correction.test.ts plus the four ThreadPanel* suites pass (75), and navigation-thread-history, thread-opening, thread-unread, thread-video and thread-window pass in Chromium and WebKit (36, one worker, no retries). In headless WebKit an older-page prepend records two overflow-y: hidden !important writes on the thread scroller at head and none at base. None of that is native momentum/compositor evidence, so the blank-paint claim still rests on the WKWebView trial in the description, which ran against a pre-rebase draft.
Optional notes:
- Nothing pins the
ThreadPanelwiring. Reverting either call site back toelement.scrollTop +=passes all 75 unit tests and all 36 browser cases, and removing the overflow toggle from the helper only fails the two helper tests. A Mac-platformThreadPanelcase asserting the temporary overflow and its restoration at each correction would catch a future direct-write reversion. - The helper tests could also cover two independent scrollers, an original
hiddenvalue, and a later write followed by another correction. They're cheap and lock in the cases above. - The live bottom-follow write (
element.scrollTop = element.scrollHeightin the positioning effect) is also automatic and stays on the direct path, along with Jump to latest andscrollIntoView. That's consistent with howpatches/README.mdseparates those paths. Worth keeping the PR's claim scoped to prepend corrections. - Unrelated to this change: in headless WebKit the visible reply moved +68 px across a held older-page prepend, identically at this head and at base. Not caused by this PR, just noting the anchor isn't pixel-exact in that harness.
* origin/main: (82 commits) Test provider connections before model selection (#500) Bundle Goose ACP with Buzz (#497) Discover saved identities across joined communities with names, pictures and retry (#291) Clarify design-system documentation and unify component examples (#498) feat(composer): convert typed Markdown live and refuse control characters committed as text (#455) fix(messages): stop three timeline scroll races that flake CI (#456) Improve Agent defaults pickers and provider keys (#392) fix(threads): keep thread history painted after scroll corrections (#493) feat(plugins): expose the agent protection service (#421) perf(sidebar): re-render only the changed row on a channel-list publish (#480) feat(agents): copy protection defaults into new agents (#420) feat(agents): support native launch protection providers (#415) fix(composer): prevent WebKit overpainting mention selections (#490) fix(composer): prevent arrow keys from inserting control characters (#488) perf(channels): fall back to one exact roster read when confirming agent adds (#485) fix(media): pause video only on comment composer focus (#483) fix(channels): dismiss management modals with outside clicks (#479) perf: reuse message date formats and stable reaction shortcuts (#477) feat(profile): run an unattended scenario file in web profiling (#476) feat(channels): administer channel members and roles (#453) ... Signed-off-by: John Tennant <jtennant@block.xyz> # Conflicts: # src/app/shell/usePanelLauncher.ts # src/bundled/agents/AgentsPage.tsx # src/bundled/agents/InventoryIdentityCard.tsx # src/bundled/agents/InventoryView.tsx # src/bundled/agents/UnifiedInventory.tsx # src/bundled/agents/index.tsx
* origin/main: (82 commits) Test provider connections before model selection (#500) Bundle Goose ACP with Buzz (#497) Discover saved identities across joined communities with names, pictures and retry (#291) Clarify design-system documentation and unify component examples (#498) feat(composer): convert typed Markdown live and refuse control characters committed as text (#455) fix(messages): stop three timeline scroll races that flake CI (#456) Improve Agent defaults pickers and provider keys (#392) fix(threads): keep thread history painted after scroll corrections (#493) feat(plugins): expose the agent protection service (#421) perf(sidebar): re-render only the changed row on a channel-list publish (#480) feat(agents): copy protection defaults into new agents (#420) feat(agents): support native launch protection providers (#415) fix(composer): prevent WebKit overpainting mention selections (#490) fix(composer): prevent arrow keys from inserting control characters (#488) perf(channels): fall back to one exact roster read when confirming agent adds (#485) fix(media): pause video only on comment composer focus (#483) fix(channels): dismiss management modals with outside clicks (#479) perf: reuse message date formats and stable reaction shortcuts (#477) feat(profile): run an unattended scenario file in web profiling (#476) feat(channels): administer channel members and roles (#453) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/app/shell/usePanelLauncher.ts # src/bundled/agents/AgentsPage.tsx # src/bundled/agents/InventoryIdentityCard.tsx # src/bundled/agents/InventoryView.tsx # src/bundled/agents/UnifiedInventory.tsx # src/bundled/agents/index.tsx
Opened by Pinky on behalf of Wes.
Summary
Fixes the blank Thread view during fast trackpad scrolling. The thread doesn't use Virtua, so #470 and #481 never reached it, but it hits the same WebKit bug the timeline did: native momentum interferes with an instant programmatic scroll correction (WebKit 262287 class). When older replies are prepended mid-flick,
ThreadPanelcorrectsscrollTopto keep the reader's anchor. DOM coverage is complete, but the compositor shows nothing for most of a second.Fix: a new
scroll-correction.tshelper applies ThreadPanel's two automatic corrections (older-page anchor, selected-reply anchor). It interrupts momentum the same way the Virtua patch'sinterruptMomentumdoes: on Mac WebKit only (MacIntel, Apple vendor, no touch points), it setsoverflow-y: hidden !importantfor one task and then restores the prior declaration. Overlapping corrections restore the original value, and a later declaration is never overwritten. The thread scroller getsscrollbar-gutter: stableso the toggle can't re-wrap rows, the same requirement.feedhas.Tradeoff: the same as the timeline. A correction can stop the rest of a trackpad coast.
Evidence
The harness is an isolated native WKWebView against the live relay, using a real 47-reply thread with media. It sends synthetic phase-bearing wheel and momentum flicks and samples paint and DOM coverage every frame.
5d5094e2): at the prepend, DOM height went 2477 → 9075 and scrollTop was corrected 0 → 6597. The layer then painted nothing for 97–100 consecutive samples (~0.8s) in all 4 runs, while DOM coverage was complete and the main thread was free.5d5094e2plus an uncommitted draft of this patch. Its bundle matches the committed helper and both call sites except for the!navigator.maxTouchPointscheck, which doesn't change behavior on Mac desktop WebKit (maxTouchPointsis 0).Validation (at
d8072e8c, rebased on #490)scroll-correction.test.tsand theThreadPanel*suites passed (75 tests).design:checkare clean.dismissOnOutsideClickprop inChannelMembersDialog.tsx. Main has since fixed it, and CI's typecheck passes.Not in scope
The timeline and thread still have separate scroll controllers (stick-to-bottom, Jump to latest, prepend anchoring). This PR shares only the correction behavior. A unified controller would be a separate refactor.