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