From d0071a6bcba5436e452da52d110098954e871a13 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Sun, 31 May 2026 03:02:52 +0800 Subject: [PATCH] fix(sync,reader): discard malformed sync CFIs; fix swipe background flash (#4370) sync: empty-start/end range CFIs left by the cfi-inert skip-link bug (e.g. epubcfi(/6/24!/4,,/20/1:58)) resolve to a section-spanning range and navigate to the wrong end of the section. Add isMalformedLocationCfi and discard such locations on the cloud-sync receive path (useProgressSync) and the kosync push path (useKOSync) so they can't move the reader or propagate to other devices. foliate 569cc06 stops generating them but does not repair already-synced values. reader: bump foliate-js to 167757a to fix the white<->black background flash when swiping between differently-colored pages; add a regression test for the sliding per-view background segments. Co-authored-by: Claude Opus 4.8 (1M context) --- apps/readest-app/.claude/memory/MEMORY.md | 2 + .../.claude/memory/empty-start-cfi-sync.md | 53 +++++++++++++ .../memory/paginator-swipe-bg-flash.md | 52 +++++++++++++ .../paginator-background-segments.test.ts | 77 +++++++++++++++++++ .../__tests__/hooks/useProgressSync.test.tsx | 44 +++++++++-- .../src/__tests__/utils/cfi.test.ts | 24 +++++- .../src/__tests__/utils/epubcfi-inert.test.ts | 58 ++++++++++++++ .../src/app/reader/hooks/useKOSync.ts | 47 ++++++----- .../src/app/reader/hooks/useProgressSync.ts | 11 ++- apps/readest-app/src/utils/cfi.ts | 23 ++++++ packages/foliate-js | 2 +- 11 files changed, 365 insertions(+), 28 deletions(-) create mode 100644 apps/readest-app/.claude/memory/empty-start-cfi-sync.md create mode 100644 apps/readest-app/.claude/memory/paginator-swipe-bg-flash.md create mode 100644 apps/readest-app/src/__tests__/document/paginator-background-segments.test.ts diff --git a/apps/readest-app/.claude/memory/MEMORY.md b/apps/readest-app/.claude/memory/MEMORY.md index 75bc522a..a8b3bdbc 100644 --- a/apps/readest-app/.claude/memory/MEMORY.md +++ b/apps/readest-app/.claude/memory/MEMORY.md @@ -12,6 +12,7 @@ - [Issue #4112 scroll-anchoring](issue-4112-scroll-anchoring.md) — RESOLVED (PR #4349). Scroll-anchoring suppressed at scrollTop 0 when prepending a section in scrolled mode; fix patterns (prepend compensation, eager backward preload, no-blank nav) + test & dev-server gotchas - [Reading ruler line/column-aware](reading-ruler-line-aware.md) — ruler snaps to real lines; multi-column band spans one column; Range.getClientRects() returns tall block boxes that must be dropped; iframe frame-offset mapping; synthetic-key throttling - [TOC expand + auto-scroll](toc-expand-and-autoscroll.md) — #4059 collapse-by-default policy in `tocTree.ts`; pinned-sidebar mounts before progress → dynamic expansion breaks scroll-to-current via (1) spurious onScroll clearing pending and (2) Virtuoso scrollToIndex landing short after row growth (re-assert on rAF) +- [Swipe page-turn bg flash](paginator-swipe-bg-flash.md) — white↔black flash on swipe+animation only; `#background` was static screen-space and didn't track content during drag/snap; fix = sliding per-view full-bleed segments (`computeBackgroundSegments`) rebuilt on scroll + per-rAF synced to the view transform during snap ## Critical Files (Most Bug-Prone) - `src/utils/style.ts` - Central EPUB CSS transformation hub (14+ bug fixes) @@ -23,6 +24,7 @@ ## Sync Notes - [KOSync CFI spine resolution](kosync-cfi-spine-resolution.md) — convert via the CFI's own spine (`getXPointerFromCFI`/`getCFIFromXPointer`), never `new XCFI(primaryDoc, primaryIndex)`; primaryIndex lags during scroll → spine-mismatch throw +- [Empty-start CFI sync bug](empty-start-cfi-sync.md) — `epubcfi(/6/24!/4,,/20/1:58)` (empty-start range) from the cfi-inert skip-link transitional window; jumps to wrong section end; `isMalformedLocationCfi` → discard the synced value in `useProgressSync` (NOT the local open path); foliate fix doesn't repair already-synced values ## Feature Notes - [Manage Cache + iOS container layout](manage-cache-ios-layout.md) — `'Cache'` base = `Library/Caches/` only (not all of Caches); iOS `Documents/Inbox` cleared too; WebKit cache + tmp out of reach; never touch App Support diff --git a/apps/readest-app/.claude/memory/empty-start-cfi-sync.md b/apps/readest-app/.claude/memory/empty-start-cfi-sync.md new file mode 100644 index 00000000..55a8c718 --- /dev/null +++ b/apps/readest-app/.claude/memory/empty-start-cfi-sync.md @@ -0,0 +1,53 @@ +--- +name: empty-start-cfi-sync +description: "Invalid synced-progress CFIs like epubcfi(/6/24!/4,,/20/1:58) — the empty-start range bug from the cfi-inert skip-link, and the read-side normalizeLocationCfi sanitizer" +metadata: + node_type: memory + type: project + originSessionId: ffa4a291-55fa-4cd5-8e35-0ac2852ff5c9 +--- + +Synced progress CFIs of the form `epubcfi(/6/24!/4,,/20/1:58)` (a range with an +**empty start** component — the `,,`) are invalid: the start collapses to the +section beginning `(body, 0)` while the end reaches the section's last block, so +a receiving device navigates to the **wrong end** of the section. + +**Root cause** — the cfi-inert a11y skip-link (`a11y.ts` prepends a 1×1 +`position:absolute` `
` as body's first child). There was a +~2.5-month transitional window (foliate `c558766` 2026-03-11 → `569cc06` +2026-05-30) where `epubcfi.js getChildNodes` already skipped `cfi-inert` but +`paginator.js getVisibleRange` did **not** yet reject it. The relocate range's +START could anchor on the skip-link; `fromRange`→`nodeToParts` asks for its +index, `getChildNodes` filters it out, `findIndex` returns -1, the +`.filter(x => x.index !== -1)` drops the step, and the start collapses to the +body boundary → empty start. (Symmetric empty-END form from the next-section +skip-link on a section's last page.) + +**Generation is fixed** by `569cc06` (live on `dev` via `c23c21d37`) — but that +does NOT repair CFIs already stored on the sync server. Those keep being served. + +**Fix (this work):** `isMalformedLocationCfi(cfi)` predicate in `src/utils/cfi.ts` +— true for a degenerate range (empty `parts.start` or `parts.end` via +`CFI.parse`). Chose **discard over repair** (user call): don't derive a position +from a corrupt CFI; drop it and let a known-good fallback win. +- Applied ONLY at `useProgressSync.ts` `applyRemoteProgress`: a malformed + `syncedConfig.location` is set to `undefined` so it can't drive goTo, can't win + the `CFI.compare` gate, and is filtered out of the persisted config (local + location kept; stops re-propagation). A valid `xpointer` still recovers the + real position via `getCFIFromXPointer`. +- Applied at `useKOSync.ts` `generateKOProgress` (push side): if local + `progress.location` is malformed, skip the CFI→XPointer conversion and reuse + the last known-good `config.xpointer`. Critical because once a bad CFI is + pushed as an XPointer the "malformed" signal is lost — other devices pull a + plain XPointer pointing at the wrong section end and can't discard it. The + kosync RECEIVE path needs no guard: `getCFIFromXPointer` builds point CFIs from + point XPointers, which can't take the empty-start form. +- Deliberately NOT applied to `FoliateViewer.tsx` open path — that uses the + user's OWN local `config.location`; discarding it would dump them at book start + (`goToFraction(0)`). Left untouched per user preference; a legacy local bad + value self-heals on the next page-turn save. + +Tests: predicate in `__tests__/utils/cfi.test.ts`; repro + flag in +`__tests__/utils/epubcfi-inert.test.ts`; discard behavior (no goTo, not +persisted) in `__tests__/hooks/useProgressSync.test.tsx`. +Related: [[kosync-cfi-spine-resolution]]. diff --git a/apps/readest-app/.claude/memory/paginator-swipe-bg-flash.md b/apps/readest-app/.claude/memory/paginator-swipe-bg-flash.md new file mode 100644 index 00000000..5252b685 --- /dev/null +++ b/apps/readest-app/.claude/memory/paginator-swipe-bg-flash.md @@ -0,0 +1,52 @@ +--- +name: paginator-swipe-bg-flash +description: Swipe page-turn background flash — +metadata: + node_type: memory + type: project + originSessionId: 374255ae-fbfd-4933-bc47-555e541fa115 +--- + +Swipe page-turn flash (white↔black pages) in the multiview paginator. Only on +**swipe + animation** (not arrow keys, not animation-off). Repro: mobile +emulator single column, swipe between a transparent page (colour comes from the +host behind, e.g. a white cover) and a CSS-coloured page (e.g. `body{background:#000!important}`). +`page2→page1` (black→white, backward) is 100%. Tell-tale: **slow swipe = big +flash, quick swipe = small flash on the trailing side** (flash width == drag distance). + +**Root cause.** `#background` (foliate-js `paginator.js`) was a static +screen-space layer (a `repeat(cc,1fr)` grid coloured by each column's midpoint). +Doc bodies are set transparent (`doc.body.style.background='none'`), so a page's +full-bleed colour is painted by `#background`, NOT the iframe — except pages that +force their colour with `!important`, which keep it in-iframe. During a swipe the +content moves but `#background` did not: (1) the **drag** scrolls via `scrollBy` +with no `#replaceBackground` call (the debounced scroll handler doesn't fire +mid-drag), so the incoming page rendered over the *outgoing* page's stale colour; +(2) the **snap** pre-set the background to the *destination* (`#replaceBackground(offset)`) +then slid only the content for 300ms, so the outgoing page lost its colour +instantly and flashed across the area it still covered. Arrow keys don't flash +because they start from rest (content already aligned with the pre-set destination). + +**Fix** (both phases needed — drag-only fix leaves the snap flash): +- `computeBackgroundSegments(views, scrollPos, bgSize, inset, containerSize)` — + exported pure helper. One full-bleed segment per rendered view at + `inset + viewOffset - scrollPos`; transparent (`bg===''`) views get no segment + (host/theme shows through); segments meeting a container edge stretch into the + full-bleed gutter. `#background` is now `position:relative; overflow:hidden` + with absolutely-positioned segment divs (not a grid). +- Drag: rebuild every scroll in the container `scroll` listener, gated on + `!this.scrolled && !this.#isAnimating`. +- Snap (`#scrollTo`): build at the **current** position, then a per-rAF + `syncBackground` loop reads the animated view's transform + (`new DOMMatrix(getComputedStyle(child).transform).m41`, m42 for vertical) and + calls `#replaceBackground(startPosition - tx)` so segments track the content + every frame. Per-frame rebuild (not a CSS translate) because the incoming + page's segment must *grow* as it slides in. + +Key insight: `expand()` makes a view element exactly `contentPages*columnSize` +wide, so per-view segments == the old per-column grid at rest, but they can slide. + +Test: `src/__tests__/document/paginator-background-segments.test.ts` (pure helper). +Visual repro = synthetic `Touch` events via Chrome MCP + an rAF timeline sampling +`#background` segment `{left,width,bg}` + the view transform through drag AND snap. +foliate-js is a submodule — commit there + bump the pointer. See [[issue-4112-scroll-anchoring]]. diff --git a/apps/readest-app/src/__tests__/document/paginator-background-segments.test.ts b/apps/readest-app/src/__tests__/document/paginator-background-segments.test.ts new file mode 100644 index 00000000..4694a256 --- /dev/null +++ b/apps/readest-app/src/__tests__/document/paginator-background-segments.test.ts @@ -0,0 +1,77 @@ +// Regression test for the swipe page-turn background flash. +// +// In paginated mode each rendered view paints a full-bleed background segment +// positioned so it tracks its content on screen. The paginator rebuilds these +// on every scroll, so while the user drags a swipe the backgrounds stay glued +// to the content. The previous implementation painted evenly-spaced screen +// columns coloured by their midpoint, so a single colour spanned the whole +// viewport — mid-swipe, when two sections with different backgrounds were both +// visible, the incoming page rendered over the outgoing page's colour until the +// swipe snapped (e.g. dragging from a black page back to a white page showed the +// white page coming in black, then snapping white on release). +// +// See readest/readest swipe background flash. +import { describe, expect, it } from 'vitest'; + +import { computeBackgroundSegments } from 'foliate-js/paginator.js'; + +describe('computeBackgroundSegments', () => { + it('splits the background at the content seam mid-swipe (incoming page keeps its own colour)', () => { + // Single column, container coextensive with the full-bleed background. + // view0 is a transparent page (white via the host), view1 is a black page. + // Dragging from view1 back toward view0: scrolled 300 of 430 px. + const views = [ + { size: 430, bg: '' }, // page 1 — transparent, no segment + { size: 430, bg: 'rgb(0, 0, 0)' }, // page 2 — black + ]; + const segments = computeBackgroundSegments(views, 300, 430, 0, 430); + + // Only the black page produces a segment, and it starts at the content + // seam (430 - 300 = 130) — it must NOT bleed left over the incoming + // transparent page (which would render the white page black mid-swipe). + expect(segments).toEqual([{ start: 130, size: 300, bg: 'rgb(0, 0, 0)' }]); + expect(segments.every((s) => s.start >= 130)).toBe(true); + }); + + it('fills the whole screen at rest for a single colored page (full-bleed)', () => { + const views = [ + { size: 430, bg: 'rgb(0, 0, 0)' }, + { size: 430, bg: '' }, + ]; + // At rest on the black page (scrollPos === its offset 0). + const segments = computeBackgroundSegments(views, 0, 430, 0, 430); + expect(segments).toEqual([{ start: 0, size: 430, bg: 'rgb(0, 0, 0)' }]); + }); + + it('stretches a centered page into the full-bleed gutters (inset > 0)', () => { + // Desktop-style: container is inset 50px inside a 430px background. + const views = [{ size: 330, bg: 'rgb(0, 0, 0)' }]; + const segments = computeBackgroundSegments(views, 0, 430, 50, 330); + // Extends out to both background edges so the page is full-bleed. + expect(segments).toEqual([{ start: 0, size: 430, bg: 'rgb(0, 0, 0)' }]); + }); + + it('gives each section its own full-bleed half in a two-up spread', () => { + const views = [ + { size: 215, bg: 'rgb(255, 0, 0)' }, + { size: 215, bg: 'rgb(0, 0, 255)' }, + ]; + // bg 500 wide, container 430 inset 35 → seam at 35 + 215 = 250. + const segments = computeBackgroundSegments(views, 0, 500, 35, 430); + expect(segments).toEqual([ + { start: 0, size: 250, bg: 'rgb(255, 0, 0)' }, // left page bleeds into left gutter + { start: 250, size: 250, bg: 'rgb(0, 0, 255)' }, // right page bleeds into right gutter + ]); + }); + + it('skips views scrolled far off screen', () => { + const views = [ + { size: 430, bg: 'rgb(0, 0, 0)' }, // offset 0 — far left, fully gone + { size: 430, bg: 'rgb(1, 1, 1)' }, // offset 430 + { size: 430, bg: 'rgb(2, 2, 2)' }, // offset 860 — far right, beyond headroom + ]; + // Scrolled to the middle view. + const segments = computeBackgroundSegments(views, 430, 430, 0, 430); + expect(segments).toEqual([{ start: 0, size: 430, bg: 'rgb(1, 1, 1)' }]); + }); +}); diff --git a/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx b/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx index 445b36e8..7cb78404 100644 --- a/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx +++ b/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx @@ -31,6 +31,9 @@ const h = vi.hoisted(() => { user: { id: 'u1' }, syncConfigsMock: vi.fn(async () => {}), syncBooksMock: vi.fn(async () => {}), + setConfigMock: vi.fn(), + cfiCompareMock: vi.fn((_a: string, _b: string) => 0), + view: { renderer: { getContents: () => [], primaryIndex: 0 }, goTo: vi.fn() }, state: { syncedConfigs: [] as unknown[] | null, progress: { location: 'cfi-loc' } as { location: string } | null, @@ -58,17 +61,14 @@ vi.mock('@/hooks/useTranslation', () => ({ vi.mock('@/store/bookDataStore', () => ({ useBookDataStore: h.makeStore({ getConfig: () => h.config, - setConfig: vi.fn(), + setConfig: h.setConfigMock, getBookData: () => ({ book: h.book }), }), })); vi.mock('@/store/readerStore', () => ({ useReaderStore: h.makeStore({ - getView: () => ({ - renderer: { getContents: () => [], primaryIndex: 0 }, - goTo: vi.fn(), - }), + getView: () => h.view, getProgress: () => h.state.progress, setHoveredBookKey: vi.fn(), getViewState: () => ({ previewMode: false }), @@ -93,7 +93,7 @@ vi.mock('@/utils/xcfi', () => ({ })); vi.mock('@/libs/document', () => ({ - CFI: { compare: () => 0 }, + CFI: { compare: (a: string, b: string) => h.cfiCompareMock(a, b) }, })); vi.mock('@/utils/event', () => ({ @@ -129,6 +129,10 @@ beforeEach(() => { vi.useFakeTimers(); h.syncConfigsMock.mockClear(); h.syncBooksMock.mockClear(); + h.setConfigMock.mockClear(); + h.view.goTo.mockClear(); + h.cfiCompareMock.mockReset(); + h.cfiCompareMock.mockReturnValue(0); h.state.syncedConfigs = []; h.state.progress = { location: 'cfi-loc' }; h.eventListeners.clear(); @@ -220,6 +224,34 @@ describe('useProgressSync', () => { expect(pullCallCount()).toBe(initialPulls); }); + test('discards a malformed synced location instead of navigating to it', async () => { + // An empty-start range CFI left by the cfi-inert skip-link bug. compare() + // returns -1 so it would "win" and drive a goTo if it were not discarded. + h.cfiCompareMock.mockReturnValue(-1); + h.state.syncedConfigs = [ + { bookHash: 'h1', metaHash: 'm1', location: 'epubcfi(/6/24!/4,,/20/1:58)', updatedAt: 3000 }, + ]; + renderHook(() => useProgressSync('h1-view1')); + await advance(0); + + // Not navigated to, and not persisted into the local config (the local + // 'cfi-loc' is kept instead of the malformed remote value). + expect(h.view.goTo).not.toHaveBeenCalled(); + const persisted = h.setConfigMock.mock.calls.at(-1)?.[1] as { location?: string } | undefined; + expect(persisted?.location).toBe('cfi-loc'); + }); + + test('navigates to a well-formed newer synced location', async () => { + h.cfiCompareMock.mockReturnValue(-1); + h.state.syncedConfigs = [ + { bookHash: 'h1', metaHash: 'm1', location: 'epubcfi(/6/24!/4/20/1:58)', updatedAt: 3000 }, + ]; + renderHook(() => useProgressSync('h1-view1')); + await advance(0); + + expect(h.view.goTo).toHaveBeenCalledWith('epubcfi(/6/24!/4/20/1:58)'); + }); + test('sync-book-progress event resets and re-runs the pull chain', async () => { h.state.syncedConfigs = null; renderHook(() => useProgressSync('h1-view1')); diff --git a/apps/readest-app/src/__tests__/utils/cfi.test.ts b/apps/readest-app/src/__tests__/utils/cfi.test.ts index 69596114..d0fc37c8 100644 --- a/apps/readest-app/src/__tests__/utils/cfi.test.ts +++ b/apps/readest-app/src/__tests__/utils/cfi.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { isCfiInLocation, findNearestCfi } from '@/utils/cfi'; +import { isCfiInLocation, findNearestCfi, isMalformedLocationCfi } from '@/utils/cfi'; describe('isCfiInLocation', () => { it('should return true when cfi path starts with location path', () => { @@ -65,3 +65,25 @@ describe('findNearestCfi', () => { expect(findNearestCfi(sortedCfis, undefined)).toBeNull(); }); }); + +describe('isMalformedLocationCfi', () => { + it('flags an empty-start range CFI', () => { + // Produced by the cfi-inert skip-link bug: the visible-range start anchored + // on the injected a11y skip-link, foliate dropped that inert step, and the + // range start went empty (the `,,`). Resolving it spans the whole section + // and jumps to the wrong end, so the location must be discarded. + expect(isMalformedLocationCfi('epubcfi(/6/24!/4,,/20/1:58)')).toBe(true); + }); + + it('flags an empty-end range CFI', () => { + expect(isMalformedLocationCfi('epubcfi(/6/24!/4,/18/1:0,)')).toBe(true); + }); + + it('does not flag a well-formed range CFI', () => { + expect(isMalformedLocationCfi('epubcfi(/6/6!/4/4/54,/1:4,/1:15)')).toBe(false); + }); + + it('does not flag a point CFI', () => { + expect(isMalformedLocationCfi('epubcfi(/6/24!/4/20/1:58)')).toBe(false); + }); +}); diff --git a/apps/readest-app/src/__tests__/utils/epubcfi-inert.test.ts b/apps/readest-app/src/__tests__/utils/epubcfi-inert.test.ts index 2921daa2..4457fd9b 100644 --- a/apps/readest-app/src/__tests__/utils/epubcfi-inert.test.ts +++ b/apps/readest-app/src/__tests__/utils/epubcfi-inert.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; import * as CFI from 'foliate-js/epubcfi.js'; +import { isMalformedLocationCfi } from '@/utils/cfi'; const XHTML = (str: string) => new DOMParser().parseFromString(str, 'application/xhtml+xml'); @@ -783,4 +784,61 @@ describe('epubcfi cfi-inert element filtering', () => { expect(() => CFI.fromRange(range)).not.toThrow(); }); }); + + // The invalid synced-progress CFI `epubcfi(/6/24!/4,,/20/1:58)` (an empty-start + // range) comes from the same skip-link. During the window where epubcfi.js + // skipped cfi-inert (c558766) but the paginator's visible-range walker did not + // yet (before 569cc06), the relocate range's START could anchor on the + // skip-link. fromRange asks for the skip-link's index, getChildNodes filters it + // out, findIndex returns -1, the `.filter(x => x.index !== -1)` drops the step, + // and the start collapses to the body boundary -> empty start `/4,,`. + describe('empty-start range CFI from a skip-link-anchored visible range', () => { + // body: [skip-link(cfi-inert), p×9, last]; with cfi-inert filtered the 10 + // real element children sit at /2../20, so /20 == the section's last block. + const page = () => + XHTML(` + t + + +

First

Second

Third

Fourth

+

xxxyyy0123456789

+

Sixth

Seventh

+ an image +

Ninth

+

0123456789012345678901234567890123456789012345678901234567890123456789

+ + `); + + it('generates an empty-start (`,,`) CFI when the range start anchors on the skip-link', () => { + const doc = page(); + const range = doc.createRange(); + range.setStart(doc.getElementById('skip-link')!, 0); + range.setEnd(doc.getElementById('last')!.firstChild!, 58); + expect(CFI.fromRange(range)).toBe('epubcfi(/4,,/20[last]/1:58)'); + }); + + it('resolves the empty-start CFI to a range from the section start to the last element', () => { + const doc = page(); + const range = CFI.toRange(doc, CFI.parse('/4,,/20[last]/1:58')); + expect(range).not.toBeNull(); + // Start collapses to the body boundary (section start); the end reaches the + // last element (section end) -> a reader jumps to the wrong end of the section. + expect(range!.startContainer).toBe(doc.body); + expect(range!.startOffset).toBe(0); + expect(range!.endContainer).toBe(doc.getElementById('last')!.firstChild); + expect(range!.endOffset).toBe(58); + }); + + it('the FIX (foliate 569cc06): a range anchored on the first real element has a non-empty start', () => { + const doc = page(); + const range = doc.createRange(); + range.setStart(doc.querySelector('p')!.firstChild!, 0); + range.setEnd(doc.getElementById('last')!.firstChild!, 58); + expect(CFI.fromRange(range)).not.toContain(',,'); + }); + + it('isMalformedLocationCfi flags the already-synced empty-start CFI so it is discarded', () => { + expect(isMalformedLocationCfi('epubcfi(/6/24!/4,,/20/1:58)')).toBe(true); + }); + }); }); diff --git a/apps/readest-app/src/app/reader/hooks/useKOSync.ts b/apps/readest-app/src/app/reader/hooks/useKOSync.ts index 5f8fb859..e3f3c341 100644 --- a/apps/readest-app/src/app/reader/hooks/useKOSync.ts +++ b/apps/readest-app/src/app/reader/hooks/useKOSync.ts @@ -10,6 +10,7 @@ import { BookDoc } from '@/libs/document'; import { debounce } from '@/utils/debounce'; import { eventDispatcher } from '@/utils/event'; import { getCFIFromXPointer, getXPointerFromCFI } from '@/utils/xcfi'; +import { isMalformedLocationCfi } from '@/utils/cfi'; import { formatProgressPercentage, getLocalProgressPreview, @@ -74,26 +75,34 @@ export const useKOSync = (bookKey: string) => { const config = getConfig(bookKey); const cfi = progress.location; if (!view || !cfi) return null; - try { - const koContents = view.renderer.getContents(); - const koPrimaryIdx = view.renderer.primaryIndex; - const content = koContents.find((x) => x.index === koPrimaryIdx) ?? koContents[0]; - // progress.location may be a CFI in a different spine section than the - // currently-rendered primary view (#primaryIndex can lag behind the - // viewport while scrolling). Resolve against the CFI's own section - // rather than forcing the primary view's document, which throws on a - // spine-index mismatch. - const xpointerResult = await getXPointerFromCFI( - cfi, - content?.doc, - content?.index, - bookData.bookDoc ?? undefined, - ); - koProgress = xpointerResult.xpointer; - setConfig(bookKey, { xpointer: koProgress }); - } catch (error) { - console.error('Failed to convert CFI to XPointer', error); + if (isMalformedLocationCfi(cfi)) { + // A malformed empty-start/end CFI (cfi-inert skip-link bug) resolves to + // the wrong end of the section. Don't derive an XPointer from it — once + // pushed as an XPointer the "malformed" signal is lost and other devices + // can't discard it. Reuse the last known-good XPointer instead. if (config?.xpointer) koProgress = config.xpointer; + } else { + try { + const koContents = view.renderer.getContents(); + const koPrimaryIdx = view.renderer.primaryIndex; + const content = koContents.find((x) => x.index === koPrimaryIdx) ?? koContents[0]; + // progress.location may be a CFI in a different spine section than the + // currently-rendered primary view (#primaryIndex can lag behind the + // viewport while scrolling). Resolve against the CFI's own section + // rather than forcing the primary view's document, which throws on a + // spine-index mismatch. + const xpointerResult = await getXPointerFromCFI( + cfi, + content?.doc, + content?.index, + bookData.bookDoc ?? undefined, + ); + koProgress = xpointerResult.xpointer; + setConfig(bookKey, { xpointer: koProgress }); + } catch (error) { + console.error('Failed to convert CFI to XPointer', error); + if (config?.xpointer) koProgress = config.xpointer; + } } const page = progress.pageinfo?.current ?? 0; diff --git a/apps/readest-app/src/app/reader/hooks/useProgressSync.ts b/apps/readest-app/src/app/reader/hooks/useProgressSync.ts index c19b3ac7..e98d999e 100644 --- a/apps/readest-app/src/app/reader/hooks/useProgressSync.ts +++ b/apps/readest-app/src/app/reader/hooks/useProgressSync.ts @@ -12,6 +12,7 @@ import { debounce } from '@/utils/debounce'; import { eventDispatcher } from '@/utils/event'; import { DEFAULT_BOOK_SEARCH_CONFIG, SYNC_PROGRESS_INTERVAL_SEC } from '@/services/constants'; import { getCFIFromXPointer, getXPointerFromCFI } from '@/utils/xcfi'; +import { isMalformedLocationCfi } from '@/utils/cfi'; // Backoff schedule for the first-pull retry on book open. After these // attempts the gate releases unconditionally so the user's progress can @@ -187,10 +188,18 @@ export const useProgressSync = (bookKey: string) => { const bookHash = bookKey.split('-')[0]!; const metaHash = book.metaHash; - const syncedConfig = syncedConfigs.filter( + let syncedConfig = syncedConfigs.filter( (c) => c.bookHash === bookHash || c.metaHash === metaHash, )[0]; if (syncedConfig) { + // Discard a malformed synced location (an empty-start/end range CFI left by + // the cfi-inert skip-link bug, e.g. `epubcfi(/6/24!/4,,/20/1:58)`) so it + // can't move the reader or be persisted — it resolves to a section-spanning + // range and jumps to the wrong end of the section. A valid xpointer below + // can still recover the real position. + if (syncedConfig.location && isMalformedLocationCfi(syncedConfig.location)) { + syncedConfig = { ...syncedConfig, location: undefined }; + } const configCFI = config?.location; let remoteCFILocation = syncedConfig.location; const xpointer = syncedConfig.xpointer; diff --git a/apps/readest-app/src/utils/cfi.ts b/apps/readest-app/src/utils/cfi.ts index 0681983a..2e1c794b 100644 --- a/apps/readest-app/src/utils/cfi.ts +++ b/apps/readest-app/src/utils/cfi.ts @@ -51,6 +51,29 @@ export function findNearestCfi( return sortedCfis[lo - 1]!; } +/** + * Detect a degenerate range CFI whose start or end path is empty — e.g. + * `epubcfi(/6/24!/4,,/20/1:58)`. These were produced by the cfi-inert skip-link + * bug (fixed in foliate 569cc06): the visible-range start/end anchored on an + * injected a11y skip-link, foliate dropped that inert step, and the range + * silently collapsed to the section boundary. Such a CFI resolves to a + * section-spanning range that navigates to the wrong end of the section, so a + * synced/saved location matching this shape should be discarded rather than + * trusted. Well-formed point and range CFIs return false. + */ +export function isMalformedLocationCfi(cfi: string): boolean { + try { + const parts = CFI.parse(cfi); + if (!parts.parent) return false; + const isEmptyPath = (segment: unknown): boolean => + Array.isArray(segment) && + segment.every((group) => Array.isArray(group) && group.length === 0); + return isEmptyPath(parts.start) || isEmptyPath(parts.end); + } catch { + return false; + } +} + export function getIndexFromCfi(cfi: string): number | null { try { const parts = CFI.parse(cfi); diff --git a/packages/foliate-js b/packages/foliate-js index 569cc060..167757a1 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit 569cc06076165f057239332484d158fd1bb79313 +Subproject commit 167757a1020a77ef0cbd8f7b291a47e5ccc83186