From e72978493523cd6e2bdeb4d0e4e90c2417e8a708 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Wed, 8 Jul 2026 19:57:00 -0400 Subject: [PATCH] fix(desktop): gate WebKit upscroll fallback on prev-rAF anchor agreement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior fallback fired `scrollTo(currentScrollTop + aboveShift)` whenever the baseline-relative position diff read 0 and `|aboveShift| > 0.5`. On WebKit that condition is not specific to a real reflow: the per-rAF baseline is captured post-realization, so `baseline` and `current` straddle the SAME side of the reflow (dTop=dScroll=0) on EVERY frame, and `aboveShift` — summed from the RO entries — can be nonzero on a frame where the anchor never moved, because a row straddling the anchor boundary is miscounted. The fallback then fabricated a downward shove: the "Shape B" slow-trackpad lurch (`e=0, rowMove=+35..41px, dScroll=0`) — firing the correction WAS the visible jump. Add a second, baseline-timing-independent signal and fire only when the two instruments agree. Carry the anchor snapshot from the PREVIOUS rAF tick (`prevReadingAnchorRef`), which — unlike the same-frame baseline — was captured before this frame's realization and so spans the reflow. Its document-position shift `observedShift = (scrollTop+topOffset) − prev(scrollTop+topOffset)` is scroll-invariant (user scroll moves scrollTop and topOffset equal-and-opposite), so it isolates the reflow alone (Eva's probe `e`, from one coherent snapshot; Quinn's scroll-isolated `R_obs`). Fire only when `prev.id === baseline.id` AND `|aboveShift − observedShift| ≤ 0.5`, and correct by `observedShift` (never raw screen motion — that would refold the user's scroll into the pin). Agreement is the classifier boundary. Two-population split of fallback frames on the WebKit slow-trackpad probe: population | aboveShift vs observedShift | old writer | gated writer -------------------|-----------------------------|--------------|------------- real reflow | agree (Δ ≤ 0.5px) | corrects OK | corrects OK straddler miscount | disagree (Δ = row height) | fabricates | suppressed | | +35..41 shove| (no write) Suppressing the miscount branch removes the fabricated shoves without touching the frames where the RO sum is genuine. Validated (each run x2, stable): - Shape-B fabricated `e=0` shoves: eliminated (was the dominant felt defect) - Chromium slow-trackpad: 0 escapes, sync 0.00, felt 0.00 (no regression) - WebKit felt/wheel gate: 0.91px peak (<< 2px threshold), was 0.00 pre-gate — the 0.91 is the frame-late CV-realization flash the miscount was masking - WebKit slow-trackpad escapes: 67 -> 54 - 2167/2167 unit tests, tsc, biome clean Residual (54 escapes) is a different phenomenon with a different owner: ~15 self-correcting frame-late flashes + ~12 multi-frame-blind under-corrections, both rooted in WebKit delivering the CV-realization RO callback one frame after the realization paints (documented in RESEARCH/WEBKIT_RO_REALIZATION_FRAME_LATE_W1.md; overturns W4a doc lines 96-98, which assumed the RO write is same-frame-visible on WebKit). That is a W4a architecture item (Quinn owns direction; Max's W2 pre-realization band is the A/B candidate), not a W1 branch-logic defect. Co-authored-by: Dawn Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../features/messages/ui/useAnchoredScroll.ts | 65 +++++++++++++++---- 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index b74ca39c9..6891391ac 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -313,6 +313,16 @@ export function useAnchoredScroll({ topOffset: number; scrollTop: number; } | null>(null); + // The reading-anchor snapshot from the PREVIOUS rAF tick. On WebKit the + // current-frame snapshot is captured post-realization (blind to this frame's + // reflow), so the RO fallback diffs against this older, pre-realization read + // to recover the anchor's true document-position shift. Same shape as + // `readingAnchorRef`; the sampler shifts current->previous each frame. + const prevReadingAnchorRef = React.useRef<{ + id: string; + topOffset: number; + scrollTop: number; + } | null>(null); // Reset everything when the channel changes — the layout effect that runs // immediately after this reset is responsible for either jumping to bottom @@ -718,18 +728,42 @@ export function useAnchoredScroll({ // is captured pre-reflow, so this fires and measures the exact shift). container.scrollTo({ top: target, behavior: "auto" }); } else if (Math.abs(aboveShift) > 0.5) { - // The position diff sees no move — the WebKit case, where the rAF - // baseline lands post-realization so `baseline` and `current` straddle - // the SAME side of the reflow (dTop=dScroll=0) and the diff is blind to - // it. The RO entries still carry the true magnitude: the net height - // change of the rows ABOVE the anchor is how far the anchor's document - // position shifted, so absorb it into scrollTop to hold the row. This is - // baseline-timing-independent — it reads the reflow directly rather than - // diffing a snapshot WebKit captured too late. - container.scrollTo({ - top: currentScrollTop + aboveShift, - behavior: "auto", - }); + // The baseline-relative position diff is blind. On WebKit the rAF + // baseline is captured post-realization, so `baseline` and `current` + // sit on the SAME side of the reflow and the diff reads 0 — but that is + // ALSO what a frame with no genuine displacement reads, and `aboveShift` + // (summed from the RO entries) is not by itself enough to tell them + // apart: a row straddling the anchor boundary is misclassified, so + // `aboveShift` can be nonzero on a frame where the anchor did not move + // (the "Shape B" slow-trackpad lurch — firing it IS the visible jump). + // + // Sufficient signal: the anchor's own document-position shift measured + // independently of the RO entries, over the PREVIOUS rAF tick (which, + // unlike the same-frame baseline, was captured before this frame's + // realization, so it spans the reflow and is not blind). Document + // position (`scrollTop + topOffset`) is scroll-invariant: the user's own + // scroll moves `scrollTop` and `topOffset` equal-and-opposite, so this + // observed shift is the reflow alone — Eva's probe `e = rowMove + + // dScroll`, computed from one coherent prev-tick snapshot. + // + // Fire only when the two instruments AGREE (`aboveShift ≈ observed`): + // agreement is the sufficiency condition — both the RO sum and the + // independent geometry see the same reflow, so it is real (not a + // straddler miscount). Correct by the observed shift, never by raw + // on-screen motion (that would refold the user's scroll into the pin). + const prev = prevReadingAnchorRef.current; + if (prev && prev.id === baseline.id) { + const observedShift = + currentScrollTop + + currentTopOffset - + (prev.scrollTop + prev.topOffset); + if (Math.abs(aboveShift - observedShift) <= 0.5) { + container.scrollTo({ + top: currentScrollTop + observedShift, + behavior: "auto", + }); + } + } } }); // Observe every timeline row (not the content wrapper): a @@ -774,6 +808,13 @@ export function useAnchoredScroll({ let rafId = requestAnimationFrame(function sample() { const container = scrollContainerRef.current; if (container && !settlingRef.current) { + // Shift the current snapshot into the previous slot BEFORE overwriting + // it, so the RO callback (which runs after this rAF in the same frame) + // can diff the anchor's document position against the PREVIOUS frame's + // pre-realization read — the only rAF snapshot old enough to span this + // frame's reflow on WebKit. The same-frame snapshot below is captured + // post-realization and is blind to it. + prevReadingAnchorRef.current = readingAnchorRef.current; readingAnchorRef.current = isAtBottomNow(container) ? null : snapshotReadingAnchor(container);