From 428168ac91fdba55794632da8df273780f2988dc Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 23 Jun 2026 07:56:10 +0800 Subject: [PATCH] fix(reader): show the centred section's chapter title in scrolled mode (#4739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In scrolled mode the header chapter title was wrong while transitioning between sections, while paginated mode was correct (#4436). foliate-js #getVisibleRange returned the first overlapping view (topmost in scroll order), so when the tail of one section was a thin sliver at the top of the viewport and the next section occupied the centre and most of the screen, the relocate event reported the sliver's section — and its title lagged behind what the reader was reading. Bump foliate-js to prefer the view covering the viewport centre (readest/foliate-js#37) and add a browser-lane regression test that scrolls a sliver of section K to the top with K+1 across the centre and asserts the relocate index is K+1. Fixes #4436 Co-authored-by: Claude Opus 4.8 (1M context) --- .../paginator-scrolled.browser.test.ts | 121 ++++++++++++++++++ packages/foliate-js | 2 +- 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/apps/readest-app/src/__tests__/document/paginator-scrolled.browser.test.ts b/apps/readest-app/src/__tests__/document/paginator-scrolled.browser.test.ts index cda43113..64a669ef 100644 --- a/apps/readest-app/src/__tests__/document/paginator-scrolled.browser.test.ts +++ b/apps/readest-app/src/__tests__/document/paginator-scrolled.browser.test.ts @@ -370,4 +370,125 @@ describe('Paginator scrolled mode (browser)', () => { } expect(paginator.getContents().map((c) => c.index)).toContain(firstBefore - 1); }); + + // Regression for issue #4436: in scrolled mode the header chapter title is + // wrong while transitioning between sections. When the tail of section K is + // a thin sliver at the very top of the viewport but section K+1 occupies the + // centre (and the majority) of the screen, the relocate event must report + // K+1 — the section the reader is actually reading — not K. The reported + // `index` is what `view.js` feeds to TOCProgress to resolve the header + // title, so reporting K here is exactly the mismatch users see versus + // paginated mode (where each page belongs to a single section). + it('should report the section occupying the viewport centre, not the sliver at 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'; + // Two adjacent tall linear sections: K leaves a text-bearing sliver at the + // top while K+1 is tall enough to span the centre and most of the screen. + let K = -1; + for (let i = 0; i < sections.length - 1; i++) { + if ( + isLinear(i) && + isLinear(i + 1) && + (sections[i]!.size ?? 0) > 8000 && + (sections[i + 1]!.size ?? 0) > 8000 + ) { + K = i; + break; + } + } + expect(K).toBeGreaterThanOrEqual(0); + const next = K + 1; + + const stabilized = waitForStabilized(paginator); + await paginator.goTo({ index: K, anchor: 0 }); + await stabilized; + await waitForFillComplete(paginator); + + // Both sections must be loaded to exercise the boundary. + if (!paginator.getContents().some((c) => c.index === next)) return; + + // Freeze the loaded set so preload/scroll-compensation can no longer shift + // view offsets out from under the absolute scrollTop we set below. + paginator.setAttribute('no-preload', ''); + + const container = paginator.shadowRoot!.getElementById('container')!; + + // Map each loaded view element to its section index and its offset within + // the scroll content (independent of the current scrollTop). + const viewBoxes = () => { + const contents = paginator.getContents(); + const cTop = container.getBoundingClientRect().top; + const scrollTop = container.scrollTop; + const boxes: { index: number; top: number; bottom: number; height: number }[] = []; + for (const child of Array.from(container.children) as HTMLElement[]) { + const iframe = child.querySelector('iframe'); + if (!iframe) continue; + const match = contents.find((x) => x.doc === iframe.contentDocument); + if (!match) continue; + const r = child.getBoundingClientRect(); + const top = r.top - cTop + scrollTop; + boxes.push({ index: match.index as number, top, bottom: top + r.height, height: r.height }); + } + return boxes; + }; + + const boxK = viewBoxes().find((b) => b.index === K); + const boxNext = viewBoxes().find((b) => b.index === next); + expect(boxK).toBeDefined(); + expect(boxNext).toBeDefined(); + + const size = paginator.size; + // Section K must be tall enough to leave a clear (text-bearing) sliver at + // the top while the viewport centre lands inside K+1. + if (boxK!.height <= 0.4 * size) return; + + // Position the boundary in the top ~35% of the viewport: a sliver of K at + // the top, K+1 across the centre and most of the screen. + const target = boxNext!.top - 0.35 * size; + expect(target).toBeGreaterThan(boxK!.top); + + // Sanity: with this scroll position the top-of-viewport section is K — the + // exact regime where the old "first overlapping view" heuristic mislabels + // the header. + const topIndexAt = (scrollTop: number) => { + const center = scrollTop + size / 2; + let topIdx: number | null = null; + for (const b of viewBoxes()) { + if (b.bottom <= scrollTop + 1 || b.top >= scrollTop + size) continue; + if (topIdx === null) topIdx = b.index; + if (center >= b.top && center < b.bottom) return { topIdx, centerIdx: b.index }; + } + return { topIdx, centerIdx: topIdx }; + }; + + const relocates: { reason: string; index: number }[] = []; + const onReloc = (e: Event) => { + const d = (e as CustomEvent).detail as { reason: string; index: number }; + if (d.reason === 'scroll') relocates.push(d); + }; + paginator.addEventListener('relocate', onReloc as EventListener); + + // The first debounced scroll after a navigation just clears #justAnchored; + // nudge a few times (all inside the target regime) so a genuine + // afterScroll('scroll') fires and emits a relocate we can read. + const nudges = [target, target - 2, target]; + for (const top of nudges) { + container.scrollTop = top; + container.dispatchEvent(new Event('scroll')); + await new Promise((r) => setTimeout(r, 350)); + } + paginator.removeEventListener('relocate', onReloc as EventListener); + + const { topIdx, centerIdx } = topIndexAt(container.scrollTop); + expect(topIdx).toBe(K); // sliver of K at the top + expect(centerIdx).toBe(next); // K+1 owns the centre + + const last = relocates.at(-1); + expect(last).toBeDefined(); + expect(last!.index).toBe(next); + }); }); diff --git a/packages/foliate-js b/packages/foliate-js index e366bdb7..20ab3ec1 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit e366bdb79c7258afe4942a932be7f20d2e9aaa89 +Subproject commit 20ab3ec1fd6ca48d72d803f7331b2eeae3335a74