fix(reader): scrolled-mode prev-section preloading and nav drift (#4112) (#4349)

Bump foliate-js: compensate the scroll position when a previous section is prepended above the viewport, so the browser's scroll-anchoring suppression at scrollTop 0 no longer drifts the view into chapter n-1; preload the previous section on backward navigation and eagerly while scrolling toward the top; and stop the blank-screen flash on adjacent navigation in continuous scrolled mode. Split paginator-multiview.browser.test.ts into paginator-scrolled and paginator-paginated, adding regression tests for the drift, previous-section preloading, the eager backward buffer, and the flash-free transition. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-05-29 13:16:25 +08:00
parent ce0ab5cc61
commit 3c14d5a4b9
5 changed files with 399 additions and 71 deletions
@@ -8,6 +8,9 @@
 - [Platform Compat Fixes](platform-compat-fixes.md) - Android, iOS, Linux, macOS platform-specific bugs
 - [Annotator & Reader Fixes](annotator-reader-fixes.md) - Highlight, selection, accessibility bugs

+## Active Investigations
+- [Issue #4112 scroll-anchoring](issue-4112-scroll-anchoring.md) — scrolled-mode backward-nav drifts to n-1; scroll-anchoring suppressed at scrollTop 0 when prepending a section
+
 ## Critical Files (Most Bug-Prone)
 - `src/utils/style.ts` - Central EPUB CSS transformation hub (14+ bug fixes)
 - `packages/foliate-js/paginator.js` - Page layout, image sizing, backgrounds
@@ -0,0 +1,34 @@
+---
+name: issue-4112-scroll-anchoring
+description: "Root cause for readest#4112 scrolled-mode backward-nav bugs — scroll-anchoring suppressed at scrollTop 0 when prepending a section"
+metadata: 
+  node_type: memory
+  type: project
+  originSessionId: e0b11058-53ee-4554-a518-134f788823ee
+---
+
+readest/readest#4112 — two scrolled-mode bugs, ONE root cause.
+
+**Root cause:** Browser scroll-anchoring (`overflow-anchor: auto`, paginator.js container) is **suppressed when scrollTop === 0**. The multiview paginator preloads the *previous* section by inserting its View element **above** the current one (`#loadAdjacentSection`, sorted insertion in `#createView`). When that prepend happens while `scrollTop === 0`, the inserted section pushes the current content down with no scroll compensation, so the viewport ends up showing the previous section.
+
+**Bug 1 (TOC backward jump → lands on n-1):** Reproduce by navigating *one section back* (target N-1 is an already-loaded adjacent view → `#goTo` "view already loaded" branch, NOT `#display`). `scrollToAnchor(0)` lands at scrollTop 0 with target as topmost view; ~250ms later the **debounced** backward-preload (paginator.js ~line 977) inserts N-2... wait, inserts the section before the target, at scrollTop 0 → suppression → viewport drifts to target-1. Intermittent (~1/3–2/3) because it races `#fillVisibleArea`'s reanchor. `primaryIndex` stays = target but the *visible* top section = target-1.
+
+**Bug 2 (can't scroll up / jumps to beginning of prev section):** same suppression — prev section inserted above at scrollTop≈0 shifts viewport to the *beginning* of prev instead of staying put. Backward-preload is debounced-only (forward preload is eager/immediate) → asymmetry adds lag.
+
+**FIX (landed on foliate-js branch `fix/scrolled-prev-prepend-anchor`, 2 changes in `#loadAdjacentSection` + `#goTo`):**
+1. Manual scroll compensation at the single prepend choke point `#loadAdjacentSection`: when prepending in scrolled mode (`index < sortedViews[0]`), after `await view.load()`, set `#renderedStart` to `startBefore + addedSize`. `containerPosition += (#vertical ? -1 : 1) * correction`; no-op (correction≈0) when the browser already anchored at scrollTop>0. Fixes drift (Bug 1) + scroll-up-shows-beginning (Bug 2b).
+2. The already-loaded `#goTo` branch only preloaded prev for *short* sections (`contentPages < columnCount`); changed to `needsPrev || this.scrolled` (+ `#isSameDirection` guard), mirroring `#display`. Fixes can't-scroll-up (Bug 2a) — the debounced backward-preload bails while `#stabilizing` after nav, so nav must preload prev itself.
+
+**UX follow-ups (same branch, same file):**
+3. **No blank flash on adjacent nav**: the already-loaded `#goTo` branch faded the container `opacity 0→1`; in continuous scrolled mode that flashed (worse after change 2 put the prev-load inside the blank window). Now `blank = !this.scrolled || this.noContinuousScroll` — continuous scrolled scrolls straight to the (already-rendered) target. `loadPrev` helper: paginated loads prev BEFORE the scroll (fill leading columns), scrolled loads it AFTER (instant transition; compensation keeps position).
+4. **Eager backward preload**: removed the debounced, one-viewport-gated backward preload; added an eager one in the immediate scroll listener mirroring forward (`pagesBehind < minPages`, scrolled-gated). Fixes "scroll up dead-ends at top until you nudge down". Safe now because change 1 compensation handles position stability (the old "debounced to avoid cascade" reason is obsolete).
+
+**Verified live + tests.** Scrolled regression tests live in `paginator-scrolled.browser.test.ts` (split out of the old `paginator-multiview.browser.test.ts`, which was renamed to `paginator-paginated.browser.test.ts` for the default/paginated + CFI tests). The 4 #4112 tests: drift / prev-preload-after-nav / no-blank / eager-backward-within-a-few-viewports (+ the moved 'columnCount=1 in scrolled mode' and the #3987 toggle-off test). `pnpm test` (4921) + `pnpm lint` + `pnpm format:check` + 57 paginator browser tests green (4 files: scrolled, paginated, expand, stabilization).
+
+**GOTCHA for live verification:** programmatic `el.scrollTop = N` does NOT fire 'scroll' events in the claude-in-chrome context (real wheel/touch does). To test scroll-driven preloading via the JS console, set scrollTop AND `el.dispatchEvent(new Event('scroll'))`. Also: the Next.js 16 dev server bundles foliate-js (transpilePackages); editing paginator.js hot-reloads, but verify the served chunk has your edit (fetch `_next/static/chunks/packages_foliate-js_paginator_*.js` and grep) — recompile can lag.
+
+readest-app test change is uncommitted on `dev`; foliate-js fix on branch `fix/scrolled-prev-prepend-anchor` (uncommitted) → needs a PR to readest/foliate-js then a submodule bump.
+
+**Verification harness:** localhost:3000/reader/<id> book "凡人修仙传" (2470 sections). Expose `__pg`/`__fv`, tag view elements with section index via `iframe.contentDocument` identity, measure `visibleTopSec()` vs target after settle. jsdom CANNOT reproduce (no real layout); use Chrome.
+
+Key file: `packages/foliate-js/paginator.js` (submodule, fork readest/foliate-js).
@@ -465,18 +465,6 @@ describe('Paginator multi-view architecture (browser)', () => {
      expect(header?.children.length).toBe(2);
      expect(footer?.children.length).toBe(2);
    });
-
-    it('should use columnCount=1 in scrolled mode regardless of width', async () => {
-      paginator = createPaginator();
-      paginator.open(book);
-      paginator.setAttribute('flow', 'scrolled');
-      const idx = book.sections!.findIndex((s) => s.linear !== 'no');
-      const stabilized = waitForStabilized(paginator);
-      await paginator.goTo({ index: idx });
-      await stabilized;
-      // Scrolled mode always uses 1 column
-      expect(paginator.columnCount).toBe(1);
-    });
  });

  describe('Adjacent index skipping non-linear sections', () => {
@@ -504,64 +492,6 @@ describe('Paginator multi-view architecture (browser)', () => {
      expect(true).toBe(true);
    });
  });
-
-  describe('Scrolled mode', () => {
-    it('should support flow=scrolled attribute', async () => {
-      paginator = createPaginator();
-      paginator.open(book);
-      paginator.setAttribute('flow', 'scrolled');
-      const idx = book.sections!.findIndex((s) => s.linear !== 'no');
-      const stabilized = waitForStabilized(paginator);
-      await paginator.goTo({ index: idx });
-      await stabilized;
-      expect(paginator.scrolled).toBe(true);
-      expect(paginator.primaryIndex).toBe(idx);
-    });
-
-    // Regression for issue #3987: toggling scrolled mode off shortly after
-    // scrolling into the next section reverted the position to the previous
-    // section because the debounced scroll handler had not yet updated
-    // #primaryIndex / #anchor.
-    it('should not revert to previous section when toggling scrolled mode off mid-scroll', async () => {
-      paginator = createPaginator();
-      paginator.open(book);
-      paginator.setAttribute('flow', 'scrolled');
-
-      const linearIndices = book
-        .sections!.map((s, i) => (s.linear !== 'no' ? i : -1))
-        .filter((i) => i >= 0);
-      if (linearIndices.length < 2) return;
-
-      const startIdx = linearIndices[0]!;
-      const nextIdx = linearIndices[1]!;
-
-      const stabilized = waitForStabilized(paginator);
-      await paginator.goTo({ index: startIdx });
-      await stabilized;
-      await waitForFillComplete(paginator);
-
-      const contents = paginator.getContents();
-      if (!contents.some((c) => c.index === nextIdx)) return;
-
-      const container = paginator.shadowRoot!.getElementById('container')!;
-      const viewElements = Array.from(container.children).filter((c): c is HTMLElement =>
-        Boolean((c as HTMLElement).querySelector?.('iframe')),
-      );
-      expect(viewElements.length).toBeGreaterThanOrEqual(2);
-      const firstViewHeight = viewElements[0]!.getBoundingClientRect().height;
-      expect(firstViewHeight).toBeGreaterThan(0);
-
-      // Scroll past the first section into the second, then immediately
-      // toggle scrolled mode off without giving the 250 ms debounce a chance
-      // to fire.
-      container.scrollTop = firstViewHeight + 100;
-      const stabilized2 = waitForStabilized(paginator);
-      paginator.setAttribute('flow', 'paginated');
-      await stabilized2;
-
-      expect(paginator.primaryIndex).toBe(nextIdx);
-    });
-  });
 });

 describe('FoliateView CFI navigation (browser)', () => {
@@ -0,0 +1,361 @@
+import { describe, it, expect, beforeAll, afterEach } from 'vitest';
+import { DocumentLoader } from '@/libs/document';
+import type { BookDoc } from '@/libs/document';
+import type { Renderer } from '@/types/view';
+
+// Vite serves fixture files; fetch the EPUB at runtime in the browser.
+const EPUB_URL = new URL('../fixtures/data/sample-alice.epub', import.meta.url).href;
+
+let book: BookDoc;
+
+const loadEPUB = async () => {
+  const resp = await fetch(EPUB_URL);
+  const buffer = await resp.arrayBuffer();
+  const file = new File([buffer], 'sample-alice.epub', { type: 'application/epub+zip' });
+  const loader = new DocumentLoader(file);
+  const { book } = await loader.open();
+  return book;
+};
+
+/**
+ * Wait for the paginator to emit 'stabilized'.
+ * MUST be called BEFORE the action that triggers stabilization (e.g. goTo),
+ * because #display dispatches 'stabilized' synchronously before returning.
+ */
+const waitForStabilized = (el: HTMLElement, timeout = 10000) =>
+  new Promise<void>((resolve, reject) => {
+    const timer = setTimeout(() => reject(new Error('stabilized timeout')), timeout);
+    el.addEventListener(
+      'stabilized',
+      () => {
+        clearTimeout(timer);
+        resolve();
+      },
+      { once: true },
+    );
+  });
+
+/** Wait for fill to complete by polling until getContents count stabilizes. */
+const waitForFillComplete = async (el: Renderer, timeout = 10000) => {
+  const start = Date.now();
+  let lastCount = -1;
+  let stableFor = 0;
+  while (Date.now() - start < timeout) {
+    const count = el.getContents().length;
+    if (count === lastCount) {
+      stableFor += 100;
+      if (stableFor >= 500) return;
+    } else {
+      stableFor = 0;
+      lastCount = count;
+    }
+    await new Promise((r) => setTimeout(r, 100));
+  }
+};
+
+describe('Paginator scrolled mode (browser)', () => {
+  let paginator: Renderer;
+
+  beforeAll(async () => {
+    book = await loadEPUB();
+    await import('foliate-js/paginator.js');
+  }, 30000);
+
+  const createPaginator = () => {
+    const el = document.createElement('foliate-paginator') as Renderer;
+    // The paginator needs non-zero dimensions for layout calculations
+    Object.assign(el.style, {
+      width: '800px',
+      height: '600px',
+      position: 'absolute',
+      left: '0',
+      top: '0',
+    });
+    document.body.appendChild(el);
+    return el;
+  };
+
+  afterEach(() => {
+    if (paginator) {
+      try {
+        paginator.destroy();
+      } catch {
+        /* iframe body may already be torn down */
+      }
+      paginator.remove();
+    }
+  });
+
+  it('should support flow=scrolled attribute', async () => {
+    paginator = createPaginator();
+    paginator.open(book);
+    paginator.setAttribute('flow', 'scrolled');
+    const idx = book.sections!.findIndex((s) => s.linear !== 'no');
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: idx });
+    await stabilized;
+    expect(paginator.scrolled).toBe(true);
+    expect(paginator.primaryIndex).toBe(idx);
+  });
+
+  it('should use columnCount=1 in scrolled mode regardless of width', async () => {
+    paginator = createPaginator();
+    paginator.open(book);
+    paginator.setAttribute('flow', 'scrolled');
+    const idx = book.sections!.findIndex((s) => s.linear !== 'no');
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: idx });
+    await stabilized;
+    // Scrolled mode always uses 1 column
+    expect(paginator.columnCount).toBe(1);
+  });
+
+  // Regression for issue #3987: toggling scrolled mode off shortly after
+  // scrolling into the next section reverted the position to the previous
+  // section because the debounced scroll handler had not yet updated
+  // #primaryIndex / #anchor.
+  it('should not revert to previous section when toggling scrolled mode off mid-scroll', async () => {
+    paginator = createPaginator();
+    paginator.open(book);
+    paginator.setAttribute('flow', 'scrolled');
+
+    const linearIndices = book
+      .sections!.map((s, i) => (s.linear !== 'no' ? i : -1))
+      .filter((i) => i >= 0);
+    if (linearIndices.length < 2) return;
+
+    const startIdx = linearIndices[0]!;
+    const nextIdx = linearIndices[1]!;
+
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: startIdx });
+    await stabilized;
+    await waitForFillComplete(paginator);
+
+    const contents = paginator.getContents();
+    if (!contents.some((c) => c.index === nextIdx)) return;
+
+    const container = paginator.shadowRoot!.getElementById('container')!;
+    const viewElements = Array.from(container.children).filter((c): c is HTMLElement =>
+      Boolean((c as HTMLElement).querySelector?.('iframe')),
+    );
+    expect(viewElements.length).toBeGreaterThanOrEqual(2);
+    const firstViewHeight = viewElements[0]!.getBoundingClientRect().height;
+    expect(firstViewHeight).toBeGreaterThan(0);
+
+    // Scroll past the first section into the second, then immediately
+    // toggle scrolled mode off without giving the 250 ms debounce a chance
+    // to fire.
+    container.scrollTop = firstViewHeight + 100;
+    const stabilized2 = waitForStabilized(paginator);
+    paginator.setAttribute('flow', 'paginated');
+    await stabilized2;
+
+    expect(paginator.primaryIndex).toBe(nextIdx);
+  });
+
+  // Regression for issue #4112: in scrolled mode, navigating one section
+  // back lands the target as the top-most view at scrollTop 0. The debounced
+  // backward-preload then inserts the *previous* section above it. Browser
+  // scroll anchoring is suppressed at scrollTop 0, so the inserted section
+  // pushes the target down and the viewport drifts to the previous section
+  // (the reader appears to "jump" to chapter n-1).
+  it('should not drift to the previous section when it preloads above at scrollTop 0', async () => {
+    paginator = createPaginator();
+    paginator.open(book);
+    paginator.setAttribute('flow', 'scrolled');
+
+    const sections = book.sections!;
+    const isLinear = (i: number) => i >= 0 && i < sections.length && sections[i]!.linear !== 'no';
+    // F is a tall section that will become the top-most view at scrollTop 0.
+    // It needs a linear neighbour on each side: F-1 (prepended above) and
+    // F+1 (navigated to first, which preloads F as its previous section).
+    let F = -1;
+    for (let i = 1; i < sections.length - 1; i++) {
+      if (isLinear(i) && isLinear(i - 1) && isLinear(i + 1) && (sections[i]!.size ?? 0) > 8000) {
+        F = i;
+        break;
+      }
+    }
+    expect(F).toBeGreaterThan(0);
+    const M = F + 1;
+
+    // Which loaded section currently sits at the top of the viewport.
+    const visibleTopIndex = (): number | null => {
+      const c = paginator.shadowRoot!.getElementById('container')!;
+      const cTop = c.getBoundingClientRect().top;
+      const contents = paginator.getContents();
+      for (const child of Array.from(c.children) as HTMLElement[]) {
+        const iframe = child.querySelector('iframe');
+        if (!iframe) continue;
+        const r = child.getBoundingClientRect();
+        if (r.top - cTop <= 1 && r.bottom - cTop > 1) {
+          const match = contents.find((x) => x.doc === iframe.contentDocument);
+          return match ? (match.index as number) : null;
+        }
+      }
+      return null;
+    };
+
+    // 1) Navigate to M so #display preloads F above it.
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: M, anchor: 0 });
+    await stabilized;
+    await waitForFillComplete(paginator);
+
+    // F is loaded as the top view; F-1 has not been pulled in yet.
+    const loadedIndices = () => paginator.getContents().map((c) => c.index);
+    expect(loadedIndices()).toContain(F);
+    expect(loadedIndices()).not.toContain(F - 1);
+
+    // 2) Arm the measurement: capture the section shown at the top of the
+    //    viewport the instant F-1 is prepended above the target.
+    const driftAtPrepend = new Promise<number | null>((resolve, reject) => {
+      const timer = setTimeout(() => reject(new Error('F-1 was never preloaded')), 8000);
+      const onCreate = (e: Event) => {
+        if ((e as CustomEvent).detail.index !== F - 1) return;
+        paginator.removeEventListener('create-overlayer', onCreate as EventListener);
+        clearTimeout(timer);
+        requestAnimationFrame(() => resolve(visibleTopIndex()));
+      };
+      paginator.addEventListener('create-overlayer', onCreate as EventListener);
+    });
+
+    // 3) Navigate one section back to F. It is already loaded, so the
+    //    paginator re-anchors to scrollTop 0 with F as the top view. The
+    //    debounced backward-preload then inserts F-1 above it.
+    await paginator.goTo({ index: F, anchor: 0 });
+
+    const visibleWhenPrepended = await driftAtPrepend;
+    expect(visibleWhenPrepended).toBe(F);
+  });
+
+  // Regression for issue #4112 (second symptom): after navigating one
+  // section back in scrolled mode the target sits at the very top, so the
+  // user must be able to scroll up into the previous section. The paginator
+  // must therefore pre-load that previous section above the target.
+  it('should preload the previous section after navigating one section back in scrolled mode', async () => {
+    paginator = createPaginator();
+    paginator.open(book);
+    paginator.setAttribute('flow', 'scrolled');
+
+    const sections = book.sections!;
+    const isLinear = (i: number) => i >= 0 && i < sections.length && sections[i]!.linear !== 'no';
+    let F = -1;
+    for (let i = 1; i < sections.length - 1; i++) {
+      if (isLinear(i) && isLinear(i - 1) && isLinear(i + 1) && (sections[i]!.size ?? 0) > 8000) {
+        F = i;
+        break;
+      }
+    }
+    expect(F).toBeGreaterThan(0);
+    const M = F + 1;
+
+    // Navigate to M (preloads F above it); F-1 is not loaded yet.
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: M, anchor: 0 });
+    await stabilized;
+    await waitForFillComplete(paginator);
+    expect(paginator.getContents().map((c) => c.index)).not.toContain(F - 1);
+
+    // Navigate one section back to F. The previous section (F-1) must be
+    // pre-loaded so the user can immediately scroll up into it.
+    await paginator.goTo({ index: F, anchor: 0 });
+    expect(paginator.primaryIndex).toBe(F);
+    expect(paginator.getContents().map((c) => c.index)).toContain(F - 1);
+  });
+
+  // Regression for issue #4112 (UX follow-up): navigating between adjacent
+  // already-loaded sections in continuous scrolled mode must not fade the
+  // container to opacity 0 — that produced a hard blank-screen flash. The
+  // target view is already rendered, so we scroll straight to it.
+  it('should not blank the container when navigating to an adjacent section in scrolled mode', async () => {
+    paginator = createPaginator();
+    paginator.open(book);
+    paginator.setAttribute('flow', 'scrolled');
+
+    const linear = book.sections!.map((s, i) => (s.linear !== 'no' ? i : -1)).filter((i) => i >= 0);
+    expect(linear.length).toBeGreaterThan(4);
+    const K = linear[3]!;
+
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: K, anchor: 0 });
+    await stabilized;
+    await waitForFillComplete(paginator);
+
+    // Pick an adjacent section that is already loaded.
+    const loaded = paginator.getContents().map((c) => c.index);
+    const adjacent = [K + 1, K - 1].find((i) => loaded.includes(i));
+    expect(adjacent).toBeDefined();
+
+    // Record every value the container's inline opacity takes during the
+    // navigation (via the old value carried on each style mutation).
+    const container = paginator.shadowRoot!.getElementById('container')!;
+    const styleHistory: string[] = [];
+    const obs = new MutationObserver((records) => {
+      for (const r of records) if (r.oldValue != null) styleHistory.push(r.oldValue);
+      styleHistory.push(container.getAttribute('style') ?? '');
+    });
+    obs.observe(container, {
+      attributes: true,
+      attributeFilter: ['style'],
+      attributeOldValue: true,
+    });
+
+    await paginator.goTo({ index: adjacent!, anchor: 0 });
+    await new Promise((r) => setTimeout(r, 100));
+    obs.disconnect();
+
+    const blanked = styleHistory.some((s) => /opacity:\s*0(?![.\d])/.test(s));
+    expect(blanked).toBe(false);
+    expect(paginator.primaryIndex).toBe(adjacent);
+  });
+
+  // Regression for issue #4112 (UX follow-up): the backward preload used to
+  // fire only within one viewport of the top (and only after the debounce),
+  // so scrolling up could dead-end at the top-most loaded section until the
+  // user nudged back down. It must mirror the eager forward buffer and load
+  // the previous section while still a few viewports away from the top.
+  it('should preload the previous section when scrolled within a few viewports of the top', async () => {
+    paginator = createPaginator();
+    paginator.open(book);
+    paginator.setAttribute('flow', 'scrolled');
+
+    const sections = book.sections!;
+    const isLinear = (i: number) => i >= 0 && i < sections.length && sections[i]!.linear !== 'no';
+    const firstLinear = sections.findIndex((s) => s.linear !== 'no');
+    // Deepest section with two linear neighbours before it, so there is
+    // room to preload backward without immediately hitting the book start.
+    let K = -1;
+    for (let i = sections.length - 1; i >= 2; i--) {
+      if (isLinear(i) && isLinear(i - 1) && isLinear(i - 2)) {
+        K = i;
+        break;
+      }
+    }
+    expect(K).toBeGreaterThan(firstLinear + 1);
+
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: K, anchor: 0 });
+    await stabilized;
+    await waitForFillComplete(paginator);
+
+    const loadedMin = () => Math.min(...paginator.getContents().map((c) => c.index as number));
+    const firstBefore = loadedMin();
+    if (firstBefore <= firstLinear) return; // already at the book start
+
+    // Position ~2 viewports below the top of loaded content — past the old
+    // one-viewport gate, but inside the forward-mirrored backward buffer.
+    const container = paginator.shadowRoot!.getElementById('container')!;
+    container.scrollTop = 2 * paginator.size;
+
+    const start = Date.now();
+    while (
+      Date.now() - start < 3000 &&
+      !paginator.getContents().some((c) => c.index === firstBefore - 1)
+    ) {
+      await new Promise((r) => setTimeout(r, 100));
+    }
+    expect(paginator.getContents().map((c) => c.index)).toContain(firstBefore - 1);
+  });
+});