fix(web): pane Back holds the restored scroll position until the pane settles, so a late-loading section no longer leaves the reader short (BUG-3250)

#1604 (BUG-3240) made the body render as soon as the replay is applied, so
PaneHost's restore now lands about 100ms after onReady, before other parts of
the pane settle: the attachment strip's loading placeholder disappears and the
timeline's loader becomes its content. Scroll anchoring and clamping then
moved scrollTop, and a reader returning near the end of a long item landed
70-90px short. bug-2182's Back leg at 8 workers: 26/100 failed on main, 0/100
on the parent.

After the first apply the restore now holds its target until
RESTORE_CAP_MS: every resize re-applies it. The hold ends at the cap or at the
first reader move: a wheel, touch or key gesture, or a scroll away from our
last assignment while scrollHeight held still (a scrollbar drag). A clamp or
an anchoring shift changes the height; our own assignment lands on the value
we set.

Legs pin each rule with a deterministic 200px shift: re-applied during the
hold; a drag honoured; a gesture that scrolls nothing ends it; nothing after
the cap. Main's PaneHost fails only the first; mutants of each rule fail only
their own. Gate: the Back leg 300/300 at 8 workers.

RESTORE_CAP_MS stays 2500 (TASK-3248's receipt): with a ~300-frame op-log
the first apply reaches 1693ms and 9/100 never applied by 2500 (the parent:
6/100), so lowering it would cut into a tail that predates BUG-3240.

Claude-Session: https://claude.ai/code/session_01NidgPePY7UvHcEJCMkD2Qz
This commit is contained in:
xarmian
2026-09-27 09:57:51 +00:00
parent 2d51308d39
commit ec58ea1f51
2 changed files with 93 additions and 13 deletions
+50
View File
@@ -103,3 +103,53 @@ test('BUG-2182: a reader gesture during the restore wait wins', async ({ page, f
await deleteCollection(fixture, request, ctx.coll.slug); await deleteCollection(fixture, request, ctx.coll.slug);
} }
}); });
// BUG-3250: once applied, the restore HOLDS its target until the cap, because
// parts of the pane settle after the body and move scrollTop (measured: the
// attachment strip's placeholder and the timeline's loader, 70-90px). A 200px
// spacer prepended inside the pane's content stands in for that late shift,
// deterministically.
async function backAndRestore(page: import('@playwright/test').Page, ctx: Awaited<ReturnType<typeof setup>>) {
await page.locator('.pane-back-btn').click();
await expect(ctx.pane.locator('a', { hasText: ctx.titleB }).last()).toBeAttached({ timeout: 10_000 });
await expect.poll(ctx.scrollTop, { timeout: 3000, message: 'premise: the restore landed' }).toBeGreaterThan(ctx.before - 40);
return ctx.scrollTop();
}
const shiftContent = (pane: import('@playwright/test').Locator) =>
pane.evaluate((el) => {
const s = document.createElement('div');
s.style.height = '200px';
s.dataset.bug3250 = 'spacer';
el.querySelector('.item-page')!.prepend(s);
});
for (const leg of [
{ name: 'a layout shift during the hold is re-applied', before: async () => {}, reapplied: true },
{ name: 'a scrollbar-style drag during the hold is honoured', before: async (ctx: any) => { await ctx.pane.evaluate((el: HTMLElement) => { el.scrollTop -= 600; }); }, reapplied: false },
// A gesture that scrolls nothing: only the gesture listener can end the hold
// here, since a scrolling wheel is also caught by the drag rule above.
{ name: 'a wheel gesture during the hold ends it, even one that scrolls nothing', before: async (ctx: any, page: any) => { const b = (await ctx.pane.boundingBox())!; await page.mouse.move(b.x + b.width / 2, b.y + b.height / 2); await page.mouse.wheel(0, 0); }, reapplied: false },
{ name: 'the hold ends at the cap, leaving no listener behind', before: async (_ctx: any, page: any) => { await page.waitForTimeout(3000); }, reapplied: false },
]) {
test(`BUG-3250: ${leg.name}`, async ({ page, fixture, request }, testInfo) => {
test.skip(testInfo.project.name !== 'desktop-chromium', 'the split pane is a desktop layout');
test.setTimeout(60_000);
const ctx = await setup(page, fixture, request);
try {
const restored = await backAndRestore(page, ctx);
await leg.before(ctx, page);
await page.waitForTimeout(150);
const beforeShift = await ctx.scrollTop();
await shiftContent(ctx.pane);
await page.waitForTimeout(400);
const after = await ctx.scrollTop();
if (leg.reapplied) {
expect(Math.abs(after - restored), `the shift was not re-applied (restored ${restored}, now ${after})`).toBeLessThanOrEqual(3);
} else {
expect(Math.abs(after - restored), `the restore overrode the reader or outlived its window (restored ${restored}, before the shift ${beforeShift}, now ${after})`).toBeGreaterThan(50);
}
} finally {
await deleteCollection(fixture, request, ctx.coll.slug);
}
});
}
@@ -128,9 +128,18 @@
// applied once the pane can scroll that far — waited for with a // applied once the pane can scroll that far — waited for with a
// ResizeObserver on the pane's content, capped. The reader wins: any // ResizeObserver on the pane's content, capped. The reader wins: any
// scroll during the wait that is not our own, or a wheel/touch/key // scroll during the wait that is not our own, or a wheel/touch/key
// gesture, cancels it. Our own assignment happens only at the moment it // gesture, cancels it.
// succeeds, so the scroll event it raises is never mistaken for the //
// reader's. // Once applied, the target is HELD until the cap (BUG-3250). Other parts
// of the pane settle after the body (the attachment strip's loading
// placeholder goes away, the timeline's loader becomes its content), the
// browser's scroll anchoring or clamping then moves scrollTop, and a
// one-shot restore left the reader 70-90px short of where they were. So
// every resize re-applies it. During the hold a scroll is the reader's only
// when it moves away from our last assignment while scrollHeight held
// still: a clamp or an anchoring shift comes with a height change, and our
// own assignment lands on the value we set. That rule is what honours a
// scrollbar drag, which raises no gesture event.
// //
// THE CAP MUST CLEAR THE COLLAB SYNC GRACE (BUG-3228). The body renders // THE CAP MUST CLEAR THE COLLAB SYNC GRACE (BUG-3228). The body renders
// only once the collab provider reports `synced`. On a connection that gets // only once the collab provider reports `synced`. On a connection that gets
@@ -145,15 +154,23 @@
// is a restore that lands late for a reader who has not moved, and any move // is a restore that lands late for a reader who has not moved, and any move
// cancels it. If the grace comes down, this can come down with it. // cancels it. If the grace comes down, this can come down with it.
// //
// The grace is gone (BUG-3240): the body now waits for the replay itself, // The grace is gone (BUG-3240), and since BUG-3250 the cap is also the
// which measured p50 8-241 ms (to 303 replay frames) and up to 740 ms for // window in which the restore HOLDS its target. Re-measured for TASK-3248
// about 2,000 frames under 8 workers. Lowering this cap owes its own // on that build, the pane Back spec at 8 workers, ms from onReady: with a
// measurement of the pane Back spec on that build; it is unchanged here. // small op-log (x300), first apply p50 75 / p99 281 / max 435, and the
// last re-apply after a late layout shift max 320. With a ~300-frame
// op-log on the item (x100), first apply p99 and max 1693, and 9/100 runs
// had not applied by 2500. That tail predates BUG-3240: the parent build
// failed the same variant 6/100 (3 of them at the cap). So the cap stays at
// 2500: lowering it would cut into that tail, not into slack.
const RESTORE_CAP_MS = 2500; const RESTORE_CAP_MS = 2500;
let cancelRestore: (() => void) | null = null; let cancelRestore: (() => void) | null = null;
function restorePaneScroll(el: HTMLElement, target: number): () => void { function restorePaneScroll(el: HTMLElement, target: number): () => void {
let done = false; let done = false;
let applied = false;
let lastSet = -1;
let lastHeight = el.scrollHeight;
const finish = () => { const finish = () => {
if (done) return; if (done) return;
done = true; done = true;
@@ -164,14 +181,27 @@
}; };
const apply = () => { const apply = () => {
if (done) return; if (done) return;
if (el.scrollHeight - el.clientHeight < target) return; lastHeight = el.scrollHeight;
el.scrollTop = target; const max = el.scrollHeight - el.clientHeight;
finish(); // Before the first apply, wait until the pane can reach the target.
if (!applied && max < target) return;
applied = true;
const want = Math.min(target, max);
if (el.scrollTop !== want) el.scrollTop = want;
lastSet = el.scrollTop;
}; };
// Before the restore lands, the pane sits at 0 while content grows, which
// raises no scroll event; a scroll now can only be the reader's.
const onScroll = () => { const onScroll = () => {
if (!done && el.scrollTop > 2) finish(); if (done) return;
// Before the restore lands, the pane sits at 0 while content grows,
// which raises no scroll event; a scroll now can only be the reader's.
if (!applied) {
if (el.scrollTop > 2) finish();
return;
}
const height = el.scrollHeight;
const readerMoved = Math.abs(el.scrollTop - lastSet) > 2 && height === lastHeight;
lastHeight = height;
if (readerMoved) finish();
}; };
const ro = new ResizeObserver(apply); const ro = new ResizeObserver(apply);
for (const child of Array.from(el.children)) ro.observe(child); for (const child of Array.from(el.children)) ro.observe(child);