diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx
index df9af09e67..082af46130 100644
--- a/desktop/src/features/messages/ui/MessageTimeline.tsx
+++ b/desktop/src/features/messages/ui/MessageTimeline.tsx
@@ -570,12 +570,17 @@ const MessageTimelineBase = React.forwardRef<
// Indexed find navigation can legitimately land near the current history
// boundary. Do not mistake that programmatic jump for scrollback intent and
// prepend underneath the active match.
- // A settle-gate hold means the reader is still parked at the OLD
- // boundary — don't stack more page fetches behind the held commit.
+ // A landed page that has not painted yet — deferred snapshot still behind
+ // the live cache, or held by the settle gate — means the reader is still
+ // parked at the OLD boundary. The scroll events WebKit keeps emitting
+ // there would otherwise start the next page in the gap between
+ // `isFetchingOlder` clearing and the prepend committing, cascading one
+ // gesture into several pages. Same predicate the spinner uses below.
if (
searchActiveMessageId ||
!fetchOlder ||
isFetchingOlder ||
+ isRenderedTimelineBehindHistoryPrepend(deferredMessages, messages) ||
isHoldingPrepend ||
showTimelineSkeleton ||
!hasOlderMessages
@@ -585,10 +590,12 @@ const MessageTimelineBase = React.forwardRef<
void fetchOlder();
return true;
}, [
+ deferredMessages,
fetchOlder,
hasOlderMessages,
isFetchingOlder,
isHoldingPrepend,
+ messages,
searchActiveMessageId,
showTimelineSkeleton,
]);
diff --git a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs
index b134b832af..12bc9332b1 100644
--- a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs
+++ b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs
@@ -86,3 +86,122 @@ test("passes when the previous snapshot was empty (initial load)", () => {
"pass",
);
});
+
+// Hook-level: the hold is released by the scroller's observed motion, never by
+// an assumed one. Frames and the clock are both driven by hand.
+async function mountHold({ motionBeforeHoldAt = null } = {}) {
+ const { JSDOM } = await import("jsdom");
+ const React = await import("react");
+ const { act } = React;
+ const { createRoot } = await import("react-dom/client");
+ const { useSettleGatedPrependMessages } = await import(
+ "./useSettleGatedPrependMessages.ts"
+ );
+ const dom = new JSDOM(
+ "",
+ );
+ const frames = [];
+ let now = 0;
+ const saved = {
+ document: globalThis.document,
+ window: globalThis.window,
+ IS_REACT_ACT_ENVIRONMENT: globalThis.IS_REACT_ACT_ENVIRONMENT,
+ requestAnimationFrame: globalThis.requestAnimationFrame,
+ cancelAnimationFrame: globalThis.cancelAnimationFrame,
+ performanceNow: performance.now,
+ };
+ Object.assign(globalThis, {
+ document: dom.window.document,
+ window: dom.window,
+ IS_REACT_ACT_ENVIRONMENT: true,
+ requestAnimationFrame(callback) {
+ frames.push(callback);
+ return frames.length;
+ },
+ cancelAnimationFrame() {},
+ });
+ performance.now = () => now;
+
+ const scroller = dom.window.document.getElementById("scroller");
+ scroller.scrollTop = 400;
+ if (motionBeforeHoldAt !== null) {
+ now = motionBeforeHoldAt;
+ }
+ let output;
+ function Harness({ messages }) {
+ output = useSettleGatedPrependMessages({
+ channelId: "c",
+ messages,
+ meta: null,
+ scrollElementRef: { current: scroller },
+ });
+ return null;
+ }
+ const root = createRoot(dom.window.document.getElementById("root"));
+ await act(async () =>
+ root.render(React.createElement(Harness, { messages: rows("a", "b") })),
+ );
+ if (motionBeforeHoldAt !== null) {
+ scroller.dispatchEvent(new dom.window.Event("scroll"));
+ }
+ now = motionBeforeHoldAt === null ? 1_000 : motionBeforeHoldAt + 50;
+ await act(async () =>
+ root.render(
+ React.createElement(Harness, { messages: rows("older", "a", "b") }),
+ ),
+ );
+ const tick = async (advanceMs) => {
+ now += advanceMs;
+ const pending = frames.splice(0);
+ await act(async () => {
+ for (const frame of pending) frame();
+ });
+ };
+ const cleanup = async () => {
+ await act(async () => root.unmount());
+ dom.window.close();
+ performance.now = saved.performanceNow;
+ for (const key of [
+ "document",
+ "window",
+ "IS_REACT_ACT_ENVIRONMENT",
+ "requestAnimationFrame",
+ "cancelAnimationFrame",
+ ]) {
+ if (saved[key] === undefined) delete globalThis[key];
+ else globalThis[key] = saved[key];
+ }
+ };
+ return { output: () => output, tick, cleanup };
+}
+
+test("a scroller at rest admits after the stable-frame count with no assumed quiet window", async () => {
+ const hold = await mountHold();
+ assert.equal(hold.output().isHoldingPrepend, true);
+ // Three frames, clock barely moving: nothing has reported motion, so the
+ // only thing the gate may wait for is frame-over-frame stability.
+ await hold.tick(1);
+ await hold.tick(1);
+ assert.equal(hold.output().isHoldingPrepend, true);
+ await hold.tick(1);
+ assert.equal(hold.output().isHoldingPrepend, false);
+ assert.deepEqual(
+ hold.output().messages.map(({ id }) => id),
+ ["older", "a", "b"],
+ );
+ await hold.cleanup();
+});
+
+test("motion observed before the hold opened still charges the quiet window", async () => {
+ const hold = await mountHold({ motionBeforeHoldAt: 10_000 });
+ // Scroll event at t=10000, hold opened at t=10050. Stable frames alone must
+ // not admit until 100ms have passed since that event.
+ await hold.tick(10);
+ await hold.tick(10);
+ await hold.tick(10);
+ await hold.tick(10);
+ assert.equal(hold.output().isHoldingPrepend, true);
+ await hold.tick(20);
+ assert.equal(hold.output().isHoldingPrepend, false);
+ await hold.cleanup();
+});
diff --git a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts
index bad8d0f04a..0f42eae443 100644
--- a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts
+++ b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts
@@ -27,13 +27,15 @@ import * as React from "react";
export const SETTLE_MOTION_WINDOW_MS = 100;
export const SETTLE_FRAME_COUNT = 3;
/**
- * Upper bound on how long a fetched page may be withheld. Trackpad momentum
- * decays in well under a second; a reader actively driving the scroller for
- * this long has moved on, and admitting under continuous REAL input is safe —
- * the dropped-write hazard is specific to the inertial momentum phase, which
- * cannot outlive this deadline.
+ * Upper bound on how long a fetched page may be withheld. The dropped-write
+ * hazard is specific to the inertial momentum phase, and trackpad momentum
+ * decays in well under a second — so a scroller still reporting motion past
+ * this deadline is under continuous REAL input, where admitting is safe.
+ * Every millisecond above the momentum bound is pure wait for a reader who
+ * keeps two fingers on the pad while history loads (#1698 used 4s; a 450ms
+ * page then painted seconds after it arrived).
*/
-export const SETTLE_HOLD_DEADLINE_MS = 4_000;
+export const SETTLE_HOLD_DEADLINE_MS = 1_500;
export type SettleGateDecision =
| { kind: "pass" }
@@ -126,6 +128,28 @@ export function useSettleGatedPrependMessages({
const latestMetaRef = React.useRef(meta);
latestMetaRef.current = meta;
+ // Track motion for the scroller's whole life, not just while holding, so a
+ // hold that begins on a scroller at rest knows it is at rest. #1698 assumed
+ // motion at hold start instead, which charged every page a full quiet
+ // window (~100ms) even when the reader had stopped long before it landed.
+ // A live fling keeps this fresh on its own: its scroll events precede the
+ // hold, so a hold opened mid-fling still waits out the quiet window even if
+ // WebKit starves the first events after it opens.
+ const lastMotionTsRef = React.useRef(Number.NEGATIVE_INFINITY);
+ React.useEffect(() => {
+ const scroller = scrollElementRef.current;
+ if (!scroller) return;
+ const markMotion = () => {
+ lastMotionTsRef.current = performance.now();
+ };
+ scroller.addEventListener("scroll", markMotion, { passive: true });
+ scroller.addEventListener("wheel", markMotion, { passive: true });
+ return () => {
+ scroller.removeEventListener("scroll", markMotion);
+ scroller.removeEventListener("wheel", markMotion);
+ };
+ }, [scrollElementRef]);
+
React.useEffect(() => {
if (!isHoldingPrepend) return;
const scroller = scrollElementRef.current;
@@ -138,24 +162,15 @@ export function useSettleGatedPrependMessages({
}
let frame: number | null = null;
const deadline = performance.now() + SETTLE_HOLD_DEADLINE_MS;
- // Assume motion at hold start: worst case this costs one quiet window
- // (~100ms) behind the fetching-older spinner when the reader was already
- // at rest; the alternative admits mid-fling if WebKit starves the first
- // scroll events.
- let lastMotionTs = performance.now();
let previousScrollTop = scroller.scrollTop;
let settledFrames = 0;
- const markMotion = () => {
- lastMotionTs = performance.now();
- };
- scroller.addEventListener("scroll", markMotion, { passive: true });
- scroller.addEventListener("wheel", markMotion, { passive: true });
const watch = () => {
const scrollTop = scroller.scrollTop;
settledFrames =
Math.abs(scrollTop - previousScrollTop) < 0.5 ? settledFrames + 1 : 0;
previousScrollTop = scrollTop;
- const quiet = performance.now() - lastMotionTs >= SETTLE_MOTION_WINDOW_MS;
+ const quiet =
+ performance.now() - lastMotionTsRef.current >= SETTLE_MOTION_WINDOW_MS;
if (
(quiet && settledFrames >= SETTLE_FRAME_COUNT) ||
performance.now() >= deadline
@@ -170,8 +185,6 @@ export function useSettleGatedPrependMessages({
};
frame = requestAnimationFrame(watch);
return () => {
- scroller.removeEventListener("scroll", markMotion);
- scroller.removeEventListener("wheel", markMotion);
if (frame !== null) cancelAnimationFrame(frame);
};
}, [isHoldingPrepend, scrollElementRef]);
diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts
index 0e1d4cfed6..6c6492f780 100644
--- a/desktop/tests/e2e/scroll-history.spec.ts
+++ b/desktop/tests/e2e/scroll-history.spec.ts
@@ -1976,16 +1976,30 @@ test("one scroll-up gesture pages older history once, not to the channel top", a
// Let any cascade run unimpeded for a generous window. With the bug, the
// observer re-arms after each page and digs through all ~1200 older roots in
// this window; with the fix it pages once and waits for the next gesture.
- await page.waitForTimeout(2_500);
+ //
+ // WKWebView keeps emitting `scroll` events while the reader rubber-bands at
+ // the boundary; each one re-enters the load-older trigger. Chromium emits
+ // none at rest, which hid the resolve→commit re-fire gap from this test.
+ // Emulate WebKit: nudge the offset 0↔1 every frame while parked in the
+ // trigger band, until the prepend commit carries the reader out of it.
+ await timeline.evaluate(async (element) => {
+ const deadline = performance.now() + 2_500;
+ while (performance.now() < deadline) {
+ if (element.scrollTop <= 200) {
+ element.scrollTop = element.scrollTop === 0 ? 1 : 0;
+ }
+ await new Promise((resolve) => requestAnimationFrame(resolve));
+ }
+ });
const pagesFetched = await fetchCount();
const deepest = await oldestRenderedIndex();
- // One gesture should yield a small, bounded number of pages — not dozens.
- // pageOlderMessagesUntilRowFloor may fetch up to MAX_BATCHES_PER_FETCH (3)
- // relay pages to satisfy one visible row floor, so allow that ceiling plus a
- // little slack; a cascade blows far past it.
- expect(pagesFetched).toBeLessThanOrEqual(4);
+ // One gesture, one page. `pageOlderMessagesUntilRowFloor` fetches exactly
+ // one server-defined window per call (#1500), and the trigger refuses to
+ // re-fire while a landed page is still unpainted, so anything above one is a
+ // cascade off the resolve→commit gap.
+ expect(pagesFetched).toBeLessThanOrEqual(1);
// And it must NOT have reached the oldest seeded root on its own.
expect(deepest ?? Number.POSITIVE_INFINITY).toBeGreaterThan(50);
});
diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts
index f7a44f05c7..2764b603c7 100644
--- a/desktop/tests/e2e/virtualization.spec.ts
+++ b/desktop/tests/e2e/virtualization.spec.ts
@@ -483,19 +483,19 @@ test.describe("list virtualization", () => {
}
});
- test("09 — older-page render commit waits for scroller rest under continued wheel input", async ({
+ test("09 — older-page render commit is bounded under continued wheel input, and holds the anchor", async ({
page,
}) => {
test.setTimeout(60_000);
- // Production shape for the WKWebView dropped-write hazard: heavy
- // variable-height rows, a slow older-page fetch, and wheel input that
- // KEEPS ARRIVING through fetch resolution. Every prepend-compensation
- // mechanism is a scrollTop write, and macOS WebKit can drop those writes
- // while trackpad momentum owns the offset — so the contract under test is
- // that the fetched page's RENDER COMMIT (the scrollHeight jump) is
- // deferred until input quiesces, and that the at-rest commit then holds
- // the anchored row. Chromium cannot reproduce the dropped write itself;
- // it CAN prove the commit-at-rest scheduling that makes it unreachable.
+ // Production shape: heavy variable-height rows, a slow older-page fetch,
+ // and wheel input that KEEPS ARRIVING through fetch resolution. #1698 held
+ // the landed page until input stopped entirely; under a sustained trackpad
+ // gesture that turned a ~450ms page into a multi-second wait behind the
+ // spinner. The contract now is a bounded hold: the page's RENDER COMMIT
+ // (the scrollHeight jump) lands within SETTLE_HOLD_DEADLINE_MS of the
+ // fetch resolving even though input never quiesces, and that commit still
+ // holds the reader's anchored row. Chromium cannot reproduce WebKit's
+ // dropped-write hazard; it CAN prove the scheduling bound and the anchor.
await installMockBridge(page, {
deepHistoryMessageCount: 1_800,
channelWindowDelayMs: 300,
@@ -505,119 +505,143 @@ test.describe("list virtualization", () => {
await expect(timeline.locator("[data-message-id]").first()).toBeVisible();
await page.waitForTimeout(1_000);
- // Mount mid-history rows clear of the load-older sentinel, then trip it.
+ // Mount mid-history rows clear of the load-older trigger, then trip it.
await timeline.evaluate((element) => {
element.scrollTop = 4000;
});
await page.waitForTimeout(300);
- // In-page observer: tracks the last wheel-input timestamp, captures the
- // first at-rest anchor after input stops, and records when the prepend
- // commit (scrollHeight jump) lands relative to the last input.
- const tracePromise = timeline.evaluate(async (scroller) => {
+ // In-page observer: records when the spinner first appears (the fetch is
+ // in flight), when the prepend commit (scrollHeight jump) lands, how long
+ // input had been quiet at that moment, and the anchored row's drift
+ // across the commit. The anchor is re-captured on every frame before the
+ // commit so it reflects the reader's CURRENT position under live input.
+ const WHEEL_TICK_PX = 30;
+ const tracePromise = timeline.evaluate(async (scroller, WHEEL_TICK_PX) => {
const s = scroller as HTMLElement;
const baseHeight = s.scrollHeight;
let lastInputTs = 0;
- let sawInput = false;
- let restAnchor: { id: string; top: number } | null = null;
+ let wheelEvents = 0;
+ // Wheel ticks since the last anchor sample. The commit frame is heavy
+ // (~100ms of prepend render), so ticks land inside it, and each tick's
+ // main-thread event and compositor scroll apply on independent
+ // schedules. The reader's own motion at the commit frame is therefore
+ // some whole number of ticks — up to one more than the events seen.
+ // Only displacement that is NOT a whole number of ticks is drift.
+ let wheelTicksSinceSample = 0;
const onWheel = () => {
lastInputTs = performance.now();
- sawInput = true;
- // Input after a lull invalidates any anchor captured during it —
- // the commit must be measured against the FINAL at-rest position.
- restAnchor = null;
+ wheelEvents += 1;
+ wheelTicksSinceSample += 1;
};
s.addEventListener("wheel", onWheel, { passive: true });
- let commit: { ts: number; gapSinceInput: number } | null = null;
- let sawSpinnerDuringHold = false;
- let anchorDriftAfterCommit: number | null = null;
+ const topRow = () => {
+ const scrollerTop = s.getBoundingClientRect().top;
+ const row = Array.from(
+ s.querySelectorAll("[data-message-id]"),
+ ).find(
+ (candidate) =>
+ candidate.getBoundingClientRect().top - scrollerTop >= 0,
+ );
+ return row?.dataset.messageId
+ ? {
+ id: row.dataset.messageId,
+ top: row.getBoundingClientRect().top - scrollerTop,
+ }
+ : null;
+ };
+ let spinnerAt: number | null = null;
+ let anchor: { id: string; top: number } | null = null;
+ let commit: {
+ ts: number;
+ sinceSpinner: number;
+ gapSinceInput: number;
+ wheelEventsBefore: number;
+ } | null = null;
+ let anchorDriftAtCommit: number | null = null;
const deadline = performance.now() + 8_000;
while (performance.now() < deadline) {
const now = performance.now();
- if (commit === null) {
- if (
- document.querySelector(
- '[data-testid="message-timeline-fetching-older"]',
- ) !== null
- ) {
- sawSpinnerDuringHold = true;
- }
- // First frame at rest (input quiet for 60ms — shorter than the
- // gate's own window, so this reading always precedes admission):
- // capture the row the at-rest commit must hold.
- if (restAnchor === null && sawInput && now - lastInputTs >= 60) {
- const scrollerTop = s.getBoundingClientRect().top;
+ if (
+ spinnerAt === null &&
+ document.querySelector(
+ '[data-testid="message-timeline-fetching-older"]',
+ ) !== null
+ ) {
+ spinnerAt = now;
+ }
+ if (s.scrollHeight > baseHeight + 800) {
+ commit = {
+ ts: now,
+ sinceSpinner: spinnerAt === null ? -1 : now - spinnerAt,
+ gapSinceInput: now - lastInputTs,
+ wheelEventsBefore: wheelEvents,
+ };
+ if (anchor !== null) {
const row = Array.from(
s.querySelectorAll("[data-message-id]"),
- ).find(
- (candidate) =>
- candidate.getBoundingClientRect().top - scrollerTop >= 0,
- );
- if (row?.dataset.messageId) {
- restAnchor = {
- id: row.dataset.messageId,
- top: row.getBoundingClientRect().top - scrollerTop,
- };
+ ).find((candidate) => candidate.dataset.messageId === anchor?.id);
+ if (row) {
+ const top =
+ row.getBoundingClientRect().top - s.getBoundingClientRect().top;
+ anchorDriftAtCommit = Number.POSITIVE_INFINITY;
+ for (let ticks = 0; ticks <= wheelTicksSinceSample + 1; ticks++) {
+ anchorDriftAtCommit = Math.min(
+ anchorDriftAtCommit,
+ Math.abs(top - (anchor.top - ticks * WHEEL_TICK_PX)),
+ );
+ }
+ } else {
+ anchorDriftAtCommit = Number.POSITIVE_INFINITY;
}
}
- if (s.scrollHeight > baseHeight + 800) {
- commit = { ts: now, gapSinceInput: now - lastInputTs };
- }
- } else if (restAnchor !== null) {
- const anchor = restAnchor;
- const scrollerTop = s.getBoundingClientRect().top;
- const row = Array.from(
- s.querySelectorAll("[data-message-id]"),
- ).find((candidate) => candidate.dataset.messageId === anchor.id);
- if (row) {
- anchorDriftAfterCommit = Math.max(
- anchorDriftAfterCommit ?? 0,
- Math.abs(
- row.getBoundingClientRect().top - scrollerTop - anchor.top,
- ),
- );
- }
- // Watch a settle window after the commit, then finish.
- if (now - commit.ts > 700) break;
+ break;
+ }
+ const sampled = topRow();
+ if (sampled) {
+ anchor = sampled;
+ wheelTicksSinceSample = 0;
}
await new Promise((resolve) => requestAnimationFrame(resolve));
}
s.removeEventListener("wheel", onWheel);
- return {
- commit,
- capturedRestAnchor: restAnchor !== null,
- sawSpinnerDuringHold,
- anchorDriftAfterCommit,
- };
- });
+ return { commit, anchorDriftAtCommit, wheelEvents };
+ }, WHEEL_TICK_PX);
// Trip the boundary, then keep real wheel input flowing DOWN (away from
// the boundary) through and well past the 300ms fetch resolution — the
- // mid-gesture window in which the ungated build commits the page.
+ // sustained-gesture window in which #1698 withheld the page.
await timeline.evaluate((element) => {
element.scrollTop = 150;
});
const box = await timeline.boundingBox();
if (!box) throw new Error("timeline has no bounding box");
await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2);
- for (let burst = 0; burst < 30; burst += 1) {
- await page.mouse.wheel(0, 30);
+ for (let burst = 0; burst < 60; burst += 1) {
+ await page.mouse.wheel(0, WHEEL_TICK_PX);
await page.waitForTimeout(40);
}
const trace = await tracePromise;
- // The page must eventually commit — the gate defers, never strands.
+ // The page committed — and while input was still flowing (the gesture
+ // above outlasts the fetch by ~2s), not after the reader gave up.
expect(trace.commit).not.toBeNull();
- // The commit landed only after input quiesced. On the ungated build the
- // deferred snapshot flushes as soon as the fetch resolves — between wheel
- // bursts, a gap far below the quiet window — so this line is the red/green
- // signal for the settle gate.
- expect(trace.commit?.gapSinceInput ?? 0).toBeGreaterThanOrEqual(80);
- // The reader saw the fetching affordance while the page was held.
- expect(trace.sawSpinnerDuringHold).toBe(true);
- // The at-rest commit held the anchored row (writes land at rest).
- expect(trace.capturedRestAnchor).toBe(true);
- expect(trace.anchorDriftAfterCommit ?? 0).toBeLessThan(5);
+ expect(
+ trace.commit?.gapSinceInput ?? Number.POSITIVE_INFINITY,
+ ).toBeLessThan(80);
+ // Bounded hold: the commit landed within the gate's deadline of the fetch
+ // going in flight (spinner) — 300ms network + SETTLE_HOLD_DEADLINE_MS, with
+ // slack for the rAF watcher. #1698's unbounded hold lands ~2.4s here.
+ expect(trace.commit?.sinceSpinner ?? Number.POSITIVE_INFINITY).toBeLessThan(
+ 300 + 1_500 + 250,
+ );
+ // The commit held the reader's anchored row within the 5px contract even
+ // though it landed under live input (modulo the reader's own whole wheel
+ // ticks — see the trace). #2855's lost correction read 452px here.
+ expect(trace.anchorDriftAtCommit).not.toBeNull();
+ expect(trace.anchorDriftAtCommit ?? Number.POSITIVE_INFINITY).toBeLessThan(
+ 5,
+ );
});
});