From c23c21d37da613e0b99ea741049775ddf1108475 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Sun, 31 May 2026 00:07:29 +0800 Subject: [PATCH] fix(kosync): reflowable conflict comparison via local CFI; scrolled-mode + library fixes (#4367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(kosync): compare reflowable conflicts via locally-resolved CFI percentage KOReader reports progress as a percentage from its own pagination, which isn't directly comparable to Readest's progress. For reflowable books, resolve the remote XPointer to a local CFI and compute the equivalent fraction (getRemoteLocalFraction), comparing that against the local percentage and falling back to the reported percentage only when it can't be resolved locally (non-XPointer progress or a missing section). The resolved fraction also drives the conflict-dialog remote preview so the shown value matches what was compared. Loosen the conflict threshold to 0.01 when the remote progress was last pushed from this same device (remote.device_id === local deviceId), so sub-page drift between a push and the next pull doesn't prompt. Render sync percentages with 2 decimals via formatProgressPercentage. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(reader): correct scrolled-mode reopen drift over background-image sections Bump the foliate-js submodule to include the scrolled-mode reopen drift fix for sections with background images, and add a browser regression test plus its EPUB fixture. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(library): redirect to login on pull-to-refresh when signed out Guard the pull-to-refresh handlers so an unauthenticated user is sent to the login screen instead of attempting a library pull and OPDS subscription check. Co-Authored-By: Claude Opus 4.8 (1M context) * docs(memory): add kosync conflict + toc/scrolled-restore notes Co-Authored-By: Claude Opus 4.8 (1M context) * fix(reader): prevent CFI crash on inert-only section bodies Reopening/​paginating across a background-image or otherwise content-less section could crash with "Cannot destructure property 'nodeType' of 'param' as it is undefined" in foliate's fromRange, aborting the relocate so the reading position was never saved. Bumps the foliate-js submodule to 569cc06 (visible-range walker skips cfi-inert skip-links; isTextNode/isElementNode are null-safe) and adds a regression test reproducing the exact crash. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(kosync): keep auto-push working when a pull finds no real conflict In the 'prompt' strategy, pullProgress set syncState to 'conflict' unconditionally on every pull that returned remote progress, even when promptedSync found no actual difference. Since auto-push only runs while 'synced', and a pull fires on every book-open and window re-activation, progress stopped being pushed. promptedSync now returns whether a real conflict was surfaced, and pullProgress only stays in 'conflict' for genuine conflicts (otherwise 'synced'). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(settings): show most recent sync time and reorder settings tabs Library settings menu now reports the latest of the book/config/note sync timestamps as "Synced …" instead of only the books timestamp. Reorder the settings tabs so Integrations precedes TTS. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- apps/readest-app/.claude/memory/MEMORY.md | 4 + .../memory/kosync-cfi-spine-resolution.md | 20 ++ .../memory/toc-expand-and-autoscroll.md | 17 ++ ...paginator-scrolled-restore.browser.test.ts | 243 ++++++++++++++++++ .../fixtures/data/repro-bg-restore.epub | Bin 0 -> 2344 bytes .../src/__tests__/hooks/kosyncPreview.test.ts | 8 +- .../hooks/kosyncRemoteFraction.test.ts | 67 +++++ .../src/__tests__/utils/epubcfi-inert.test.ts | 44 ++++ .../app/library/components/SettingsMenu.tsx | 10 +- apps/readest-app/src/app/library/page.tsx | 10 +- .../src/app/reader/hooks/kosyncPreview.ts | 5 +- .../src/app/reader/hooks/kosyncProgress.ts | 34 +++ .../src/app/reader/hooks/useKOSync.ts | 44 +++- .../components/settings/SettingsDialog.tsx | 12 +- packages/foliate-js | 2 +- 15 files changed, 495 insertions(+), 25 deletions(-) create mode 100644 apps/readest-app/.claude/memory/kosync-cfi-spine-resolution.md create mode 100644 apps/readest-app/.claude/memory/toc-expand-and-autoscroll.md create mode 100644 apps/readest-app/src/__tests__/document/paginator-scrolled-restore.browser.test.ts create mode 100644 apps/readest-app/src/__tests__/fixtures/data/repro-bg-restore.epub create mode 100644 apps/readest-app/src/__tests__/hooks/kosyncRemoteFraction.test.ts diff --git a/apps/readest-app/.claude/memory/MEMORY.md b/apps/readest-app/.claude/memory/MEMORY.md index 16167d52..75bc522a 100644 --- a/apps/readest-app/.claude/memory/MEMORY.md +++ b/apps/readest-app/.claude/memory/MEMORY.md @@ -11,6 +11,7 @@ ## Paginator Scroll Knowledge - [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) ## Critical Files (Most Bug-Prone) - `src/utils/style.ts` - Central EPUB CSS transformation hub (14+ bug fixes) @@ -20,6 +21,9 @@ - `src/app/reader/components/FoliateViewer.tsx` - Reader view orchestration - `src/app/reader/components/annotator/Annotator.tsx` - Annotation lifecycle +## 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 + ## 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 - [D-pad Navigation](dpad-navigation.md) — Android TV remote / keyboard arrow navigation design, key files, and pitfalls diff --git a/apps/readest-app/.claude/memory/kosync-cfi-spine-resolution.md b/apps/readest-app/.claude/memory/kosync-cfi-spine-resolution.md new file mode 100644 index 00000000..bbd3a5f6 --- /dev/null +++ b/apps/readest-app/.claude/memory/kosync-cfi-spine-resolution.md @@ -0,0 +1,20 @@ +--- +name: kosync-cfi-spine-resolution +description: "KOSync CFI↔XPointer conversion must resolve via the CFI's own spine index, not the paginator's primaryIndex (which lags during scroll)" +metadata: + node_type: memory + type: project + originSessionId: 41b15d60-969d-4173-9db7-d1515721e527 +--- + +KOSync progress push/pull converts between foliate CFI and KOReader XPointer via `src/utils/xcfi.ts`. `XCFI` is constructed for ONE spine section (`spineItemIndex`) and `cfiToXPointer`/`xPointerToCFI` throw `CFI spine index N does not match converter spine index M` if asked to convert a CFI from a different section. + +**Pitfall:** `view.renderer.primaryIndex` lags behind the viewport during scrolling (see `paginator.js` `#primaryIndex` comment). So `progress.location` can be a CFI in a different spine section than the currently-rendered primary view. Building the converter from the primary view's `doc`/`index` then converting `progress.location` throws on mismatch. + +**Fix (PR branch fix/kosync-spine-index):** always go through the exported helpers `getXPointerFromCFI(cfi, doc?, index?, bookDoc)` / `getCFIFromXPointer(...)` in `xcfi.ts`. They call `XCFI.extractSpineIndex(cfi)` and, when the passed `index` ≠ the CFI's spine, load the correct section's document via `bookDoc.sections[xSpineIndex].createDocument()`. `generateKOProgress` in `src/app/reader/hooks/useKOSync.ts` had hand-rolled `new XCFI(primaryDoc, primaryIndex)` instead of using the helper — that was the bug. + +**Why:** the helper already handles cross-section resolution correctly; never reconstruct the converter inline against `primaryIndex`. + +**How to apply:** for any CFI→XPointer or XPointer→CFI in KOSync code, use the `getXPointerFromCFI`/`getCFIFromXPointer` helpers and pass `bookDoc` so off-screen sections can be loaded. Related: [[issue-4112-scroll-anchoring]] (same primaryIndex/scroll-lag family). The companion foliate `fromRange` crash (`Cannot destructure property 'nodeType'`) during relocate is separate scroll-lag noise, not the sync blocker. + +**Conflict-detection comparison (`promptedSync` in `useKOSync.ts`):** KOReader's reported `remote.percentage` comes from CREngine pagination and is NOT comparable to Readest's progress. For reflowable books, convert the remote XPointer → local CFI → `view.getCFIProgress(cfi).fraction` (helper `getRemoteLocalFraction` in `kosyncProgress.ts`) and compare THAT to the local percentage; fall back to `remote.percentage` only when it can't be resolved locally (non-XPointer/Kavita progress or missing section). The resolved fraction also drives the "Approximately X%" remote preview so the dialog shows what was actually compared. Fixed-layout still compares page-derived `remotePercentage`. Threshold: `0.0001` cross-device, but loosened to `0.01` when `remote.device_id === settings.kosync.deviceId` (same device = our own earlier push; don't prompt on sub-page drift). Percentages render via `formatProgressPercentage` (2 decimals, `kosyncPreview.ts`). diff --git a/apps/readest-app/.claude/memory/toc-expand-and-autoscroll.md b/apps/readest-app/.claude/memory/toc-expand-and-autoscroll.md new file mode 100644 index 00000000..1bb33791 --- /dev/null +++ b/apps/readest-app/.claude/memory/toc-expand-and-autoscroll.md @@ -0,0 +1,17 @@ +--- +name: toc-expand-and-autoscroll +description: TOC sidebar auto-expansion policy + the Virtuoso scrollToIndex-after-expansion race that breaks the initial scroll-to-current-chapter +metadata: + node_type: memory + type: project + originSessionId: 4d4eefcb-324a-4342-9472-9dae7cc57b41 +--- + +Issue #4059: the TOC sidebar used to auto-expand *every* top-level container (`computeExpandedSet` added all `toc.filter(item => item.subitems?.length)`), so multi-volume books ("文学必读合集20册") opened fully expanded. Fix: expand only the current location's ancestor path (`findParentPath`), with a single-root fallback (expand `toc[0]` only when `toc.length === 1`) so a one-root-wrapper TOC isn't reduced to one row. Pure logic lives in `src/app/reader/components/sidebar/tocTree.ts` (extracted from TOCView so it's unit-testable — TOCView itself can't be imported in jsdom: react-virtuoso + OverlayScrollbars CSS). + +Collapsing-by-default introduced a **regression**: the current chapter no longer scrolled into view on fresh load. Root cause is two-fold, both stemming from the pinned sidebar mounting *before* the first relocate (progress is null at mount, so `initialScrollTarget.index` is 0 and Virtuoso's `initialTopMostItemIndex` can't anchor): + +1. When progress arrives, the current volume expands and the virtualized list grows by dozens of rows in one commit. Virtuoso emits a synthetic `onScroll`; the old handler treated *any* scroll while a scroll was queued as "user took over" and cleared `pendingScrollRef`. Fix: ignore a queued-state scroll unless a real gesture preceded it — track `userInputRef` via capture-phase `wheel`/`touchstart`/`pointerdown`/`keydown` listeners on the scroller; `if (pendingScrollRef.current && !userInputRef.current) return;`. +2. Even with pending preserved, a single `scrollToIndex(idx, {align:'center'})` fired in the same commit as the 28→90 row growth **lands short** — Virtuoso scrolls before measuring the new rows. Fix: re-assert the scroll on the next `requestAnimationFrame` (only for the instant `behavior:'auto'` case). The toggle-sidebar (remount) path always worked because remounting feeds `initialTopMostItemIndex` before first render. + +Verified live: 红楼梦 book at `/reader/217e3f1e...` — only 下卷 expanded, current chapter centered (0px from viewport center), user scroll no longer snaps back. Related: [[issue-4112-scroll-anchoring]], [[reading-ruler-line-aware]], [[virtuoso_overlayscrollbars]]. diff --git a/apps/readest-app/src/__tests__/document/paginator-scrolled-restore.browser.test.ts b/apps/readest-app/src/__tests__/document/paginator-scrolled-restore.browser.test.ts new file mode 100644 index 00000000..bffc645b --- /dev/null +++ b/apps/readest-app/src/__tests__/document/paginator-scrolled-restore.browser.test.ts @@ -0,0 +1,243 @@ +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'; + +// A two-section book: section 0 is a full-page illustration whose has a +// background image; section 1 is the text chapter the reader "reopens" into. +const EPUB_URL = new URL('../fixtures/data/repro-bg-restore.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], 'repro-bg-restore.epub', { type: 'application/epub+zip' }); + const loader = new DocumentLoader(file); + const { book } = await loader.open(); + return book; +}; + +const waitForStabilized = (el: HTMLElement, timeout = 10000) => + new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error('stabilized timeout')), timeout); + el.addEventListener( + 'stabilized', + () => { + clearTimeout(timer); + resolve(); + }, + { once: true }, + ); + }); + +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)); + } +}; + +// Stand-in for a real (slower-than-instant) EPUB image: it loads the resource +// but only notifies `onload` after a delay, so the background image arrives +// AFTER the paginator has navigated and settled — the timing that triggers the +// late expand. The data-URI fixture image otherwise resolves so fast it expands +// during the initial fill and the drift is masked. +const NativeImage = globalThis.Image; +const IMAGE_DELAY = 250; +class DelayedImage { + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + naturalWidth = 0; + naturalHeight = 0; + complete = false; + set src(value: string) { + setTimeout(() => { + const real = new NativeImage(); + real.onload = () => { + this.naturalWidth = real.naturalWidth; + this.naturalHeight = real.naturalHeight; + this.complete = true; + this.onload?.(); + }; + real.onerror = () => { + this.complete = true; + this.onerror?.(); + }; + real.src = value; + }, IMAGE_DELAY); + } +} + +// A background image that fails to load (missing file, decode error, ...). +class ErroringImage { + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + naturalWidth = 0; + naturalHeight = 0; + complete = false; + set src(_value: string) { + setTimeout(() => { + this.complete = true; + this.onerror?.(); + }, 10); + } +} + +// A background image whose request hangs forever: never fires load or error. +class HangingImage { + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + naturalWidth = 0; + naturalHeight = 0; + complete = false; + set src(_value: string) { + /* intentionally never resolves */ + } +} + +describe('Paginator scrolled-mode restore over a background-image section (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; + Object.assign(el.style, { + width: '800px', + height: '600px', + position: 'absolute', + left: '0', + top: '0', + }); + document.body.appendChild(el); + return el; + }; + + afterEach(() => { + globalThis.Image = NativeImage; + if (paginator) { + try { + paginator.destroy(); + } catch { + /* iframe body may already be torn down */ + } + paginator.remove(); + } + }); + + // Regression: reopening a book in scrolled mode jumped from the saved + // paragraph to the top of the chapter. The chapter's preceding section is a + // full-page illustration whose body background image is loaded lazily (a + // separate async Image, not awaited by the iframe `load` event). When it + // finishes loading — after navigation has settled — the section's view + // expands to fit the image, above the restored position, and the content + // below shifts down by the image height. WebKit has no scroll anchoring to + // absorb this, so the viewport drifts to the illustration / chapter start. + // The view must be sized to the background image BEFORE it is rendered so it + // never grows after navigation. + it('keeps the text chapter anchored when the previous full-page background image loads', async () => { + globalThis.Image = DelayedImage as unknown as typeof Image; + + paginator = createPaginator(); + paginator.open(book); + paginator.setAttribute('flow', 'scrolled'); + + // WebKit reality this bug surfaces on: no native CSS scroll anchoring to + // silently hold position when content grows above the viewport. + const style = document.createElement('style'); + style.textContent = '#container, #container * { overflow-anchor: none !important; }'; + paginator.shadowRoot!.appendChild(style); + + // Restore into the text chapter (section 1); the illustration (section 0) + // is preloaded above it. + const stabilized = waitForStabilized(paginator); + await paginator.goTo({ index: 1, anchor: 0 }); + await stabilized; + await waitForFillComplete(paginator); + expect(paginator.getContents().map((c) => c.index)).toContain(0); + + const container = paginator.shadowRoot!.getElementById('container')!; + const text = paginator.getContents().find((c) => c.index === 1)!; + const doc = text.doc as Document; + const iframeEl = (doc.defaultView!.frameElement as HTMLElement)!; + const marker = doc.getElementById('chapter-title')!; + expect(marker).toBeTruthy(); + + // Distance of the chapter heading from the top of the viewport. goTo(1, + // anchor 0) puts it at the top, so this is ~0 (within the header margin). + const headingOffsetFromTop = () => + iframeEl.getBoundingClientRect().top + + marker.getBoundingClientRect().top - + container.getBoundingClientRect().top; + + const bgView = () => + Math.round( + ( + paginator.getContents().find((c) => c.index === 0)!.doc as Document + ).defaultView!.frameElement!.getBoundingClientRect().height, + ); + const dbg = () => + JSON.stringify({ offset: Math.round(headingOffsetFromTop()), bgViewHeight: bgView() }); + + // Let the delayed background image load and (without the fix) expand the + // illustration above the viewport. + await new Promise((r) => setTimeout(r, IMAGE_DELAY + 600)); + + // Confirm the illustration really did size to its tall background image — + // otherwise this test would pass vacuously. + expect(bgView(), dbg()).toBeGreaterThan(1500); + + // The chapter heading must still be at the top of the viewport. Without the + // fix the illustration above expands by thousands of px and the heading is + // pushed far down (the viewport shows the illustration / chapter start). + expect(Math.abs(headingOffsetFromTop()), dbg()).toBeLessThan(80); + }); + + // Robustness: a background image that errors (missing/broken) must not block + // the section from rendering — it simply renders without the background. + it('renders the illustration section even when its background image fails to load', async () => { + globalThis.Image = ErroringImage as unknown as typeof Image; + + paginator = createPaginator(); + paginator.open(book); + paginator.setAttribute('flow', 'scrolled'); + + const stabilized = waitForStabilized(paginator); + await paginator.goTo({ index: 0, anchor: 0 }); + await stabilized; // must resolve — a failed background image cannot hang load() + + expect(paginator.primaryIndex).toBe(0); + expect(paginator.getContents().some((c) => c.index === 0)).toBe(true); + }); + + // Robustness: a background image whose request hangs (never fires load or + // error) must not block rendering forever — the bounded wait releases it. + it('does not block rendering when the background image never loads or errors', async () => { + globalThis.Image = HangingImage as unknown as typeof Image; + + paginator = createPaginator(); + paginator.open(book); + paginator.setAttribute('flow', 'scrolled'); + + const stabilized = waitForStabilized(paginator, 8000); + await paginator.goTo({ index: 0, anchor: 0 }); + await stabilized; // resolves via the bounded timeout rather than hanging + + expect(paginator.primaryIndex).toBe(0); + expect(paginator.getContents().some((c) => c.index === 0)).toBe(true); + }); +}); diff --git a/apps/readest-app/src/__tests__/fixtures/data/repro-bg-restore.epub b/apps/readest-app/src/__tests__/fixtures/data/repro-bg-restore.epub new file mode 100644 index 0000000000000000000000000000000000000000..8aa5a2714afe4adc887f57551f4485fc61626b43 GIT binary patch literal 2344 zcmaJ@2{@Zs7Y>QFN_5f=imGi?E2y<-M}%T5jizcDQAuk_Nkm78Vk)TgN2|27iekiA zQd4UzwG_c$CQ7ZXy~`+mTPbRtFXKmf{{H8A&iCE>-1FXZ?>XN&?}xGgLj*w_;bv4j zd1DzTV8EE8{D2bh1Xtexk}H-(^1@%n`r^Hbs;(q5?)Y^)3AGCe7;lRJFtReafKV~F zKC8+x+uV-hV3sdj`6;kG2k4=%%`STreX)3=>oquq;DxzUKm;F!8n3Jf7Uz)_r%piZ zBkvb8VWVLF(ovauI5)g-<4kXZ$xk{Bl{y(BrLYE(1c`@2|6!`i+%-5yh4Y?yIsHyr z*stwzPPw7|I$7VU>-l-J0!F&>H37y|XP>i=dQ}B-a@?)*4&;YHE7VF)89(v6jdR9H z7I_Zw-tm5#CGy3k0^1V68%q)-dM!rtI<=}(dX~R`u@vvwwAzIsbu<||s%)%=B}_2a zc)lG8Uf_!7OpH+IJz*G;V7qhyYerlkkjS_av$-sEpf@?x6tAeF4 z9!Y#fYU)1ke)=)QIxzu4ZPOu)PT$OF{20L_IaTG`e{S#T0->*(vm5;-m3lLGPXzd?1~P4d+gAcA zyd43Y`+oy~Y9ZFCK`ZAD+F0jNbQMssosg1Zg&e`qYH0Z>rmN5$DFwxti))K|qzP=v zqXBXD)N_XvK?XgbHa_`{4!DV#&59(w_6aV0m!ZP_o=A-g4wjQu-H9TGwzgr- zQrXGW!!PFy2d;Tpq+K-69LQCYf=#?I@1KkJ^A%x-`hd(@8@9Zf{}4#Hr8K2`Qv1EsjyvrS%8>A-*R}3 z)SWsVz8`Zxbt%B{jZ=l<4|E$od?H;^86DIySC27@YZys#tTFw3O|PAkl(YF9VHOJk@Y3*it5#b$3bkofm;Ax#?U^E!WBabBTQ(O+k_&K(nIkvnd?cfM zFiUMdHVFzfmg2lskCodCasL)KGO*cQb!Jq2bZb9;e)U zz4r&{{uvP4fn3W8n+lZ{y2b6+c=q3N;^AYmW)$<`#kt0{LW3UsAbZ@6W&UmdI^PgUk+^6?}FC5{|A2{KqDVDxi zs;F(U)Xh%cxBlKSzbRQ6c4U3l3}@hpGZ0}>r2jQ*Ao?yqv9aiABl^HQ(c@PQJB1J^ zGz7sGg=m>WJ5L}`9l+!}h?Whs(-LuBgU6f=lh;Du{oj*rm?d>+U1@2))@B2=J4Xv$ zEbagxN3viLNOU_#oL1>d^o4tq+*0Cdt?Si zbL4p8>_Qp-+5YfwW0;9G?gn#%PSJg19b_n$@1NOEPlOGR52krqeEGEcu)l>io;y?V zYljN|f*g7usWLs6(sNY3erKHAkvyG&}T1=UrFi8!utA$T8P(w=Y zCy$a*r>P>raKHY*!^`c7kV$!?5b83h4&tSB(rp7x$-?Lty7R!DQx}SwI!j!NX@;qp znp%i_B+-Ek4@>HG>+6&6n+a+yG-&NHvclSEl{9O6W=m;1wR4+tAO6r=61TVlVqeK> zI{licSKxyBVSX(A{+ktHPpef&Wj&`kq}k{1Oi1$OX^Me2*T5g%7yNsY+Lnk zY-6@bbkurOZAMsBax$$0E)m4|>#ZWQwC#{#dcu)Q!CWW{E^cY?w}}%-);~h0a$Z|B zx)-%+v-#wZ=B7YFlL$Rv_!Wh6oPC=_IWXWW0nw@0uVK3=_5Zo%u+*k-TpI&hZE@Ca zzk6AWgW2@!v5nd0uuX?Mhu+)pcfeP { sectionLabel: 'Chapter 3', pageinfo: { current: 49, total: 100 }, }); - expect(getLocalProgressPreview(progress, false, t)).toBe('Chapter 3 (50%)'); + expect(getLocalProgressPreview(progress, false, t)).toBe('Chapter 3 (50.00%)'); }); it('falls back to page info when the section label is undefined', () => { @@ -50,7 +50,7 @@ describe('getLocalProgressPreview', () => { }); const result = getLocalProgressPreview(progress, false, t); expect(result).not.toContain('undefined'); - expect(result).toBe('Page 1 of 100 (1%)'); + expect(result).toBe('Page 1 of 100 (1.00%)'); }); it('falls back to page info when the section label is blank', () => { @@ -60,7 +60,7 @@ describe('getLocalProgressPreview', () => { }); const result = getLocalProgressPreview(progress, false, t); expect(result).not.toContain('undefined'); - expect(result).toBe('Page 10 of 50 (20%)'); + expect(result).toBe('Page 10 of 50 (20.00%)'); }); it('uses page info for fixed-layout books regardless of section label', () => { @@ -68,6 +68,6 @@ describe('getLocalProgressPreview', () => { sectionLabel: 'ignored', section: { current: 4, total: 20 }, }); - expect(getLocalProgressPreview(progress, true, t)).toBe('Page 5 of 20 (25%)'); + expect(getLocalProgressPreview(progress, true, t)).toBe('Page 5 of 20 (25.00%)'); }); }); diff --git a/apps/readest-app/src/__tests__/hooks/kosyncRemoteFraction.test.ts b/apps/readest-app/src/__tests__/hooks/kosyncRemoteFraction.test.ts new file mode 100644 index 00000000..8431625e --- /dev/null +++ b/apps/readest-app/src/__tests__/hooks/kosyncRemoteFraction.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { BookDoc } from '@/libs/document'; +import type { FoliateView } from '@/types/view'; +import { getRemoteLocalFraction } from '@/app/reader/hooks/kosyncProgress'; +import { getCFIFromXPointer } from '@/utils/xcfi'; + +// The XPointer↔CFI conversion accuracy is covered by the xcfi and +// page-progress-epub suites; here we only exercise how the helper wires the +// conversion, the fraction lookup, and its fallbacks. +vi.mock('@/utils/xcfi', () => ({ + getCFIFromXPointer: vi.fn(), +})); + +const mockGetCFIFromXPointer = vi.mocked(getCFIFromXPointer); + +const XPOINTER = '/body/DocFragment[4]/body/div/p[3]/text().0'; + +const makeView = (fraction: number | null): FoliateView => + ({ + renderer: { primaryIndex: 0, getContents: () => [] }, + getCFIProgress: vi.fn().mockResolvedValue(fraction === null ? null : { fraction }), + }) as unknown as FoliateView; + +const bookDoc = {} as BookDoc; + +describe('getRemoteLocalFraction', () => { + beforeEach(() => { + mockGetCFIFromXPointer.mockReset(); + }); + + it('resolves an XPointer position to the local book fraction (not the reported percentage)', async () => { + mockGetCFIFromXPointer.mockResolvedValue('epubcfi(/6/8!/4/2/6)'); + const view = makeView(0.42); + + // percentage deliberately wrong to prove the local fraction is used instead + const remote = { progress: XPOINTER, percentage: 0.99 }; + const fraction = await getRemoteLocalFraction(remote, view, bookDoc); + + expect(fraction).toBe(0.42); + expect(mockGetCFIFromXPointer).toHaveBeenCalledOnce(); + expect(view.getCFIProgress).toHaveBeenCalledWith('epubcfi(/6/8!/4/2/6)'); + }); + + it('returns undefined for non-XPointer progress without attempting conversion', async () => { + const view = makeView(0.42); + + expect( + await getRemoteLocalFraction({ progress: '42', percentage: 0.5 }, view, bookDoc), + ).toBeUndefined(); + expect(await getRemoteLocalFraction({ progress: undefined }, view, bookDoc)).toBeUndefined(); + expect(mockGetCFIFromXPointer).not.toHaveBeenCalled(); + }); + + it('returns undefined when the XPointer cannot be converted to a local CFI', async () => { + mockGetCFIFromXPointer.mockRejectedValue(new Error('section not found')); + const view = makeView(0.42); + + expect(await getRemoteLocalFraction({ progress: XPOINTER }, view, bookDoc)).toBeUndefined(); + }); + + it('returns undefined when the CFI resolves to no local progress', async () => { + mockGetCFIFromXPointer.mockResolvedValue('epubcfi(/99/999!/0)'); + const view = makeView(null); + + expect(await getRemoteLocalFraction({ progress: XPOINTER }, view, bookDoc)).toBeUndefined(); + }); +}); 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 761cface..2921daa2 100644 --- a/apps/readest-app/src/__tests__/utils/epubcfi-inert.test.ts +++ b/apps/readest-app/src/__tests__/utils/epubcfi-inert.test.ts @@ -739,4 +739,48 @@ describe('epubcfi cfi-inert element filtering', () => { } }); }); + + // A section whose has no CFI-visible content — e.g. a full-page + // background-image section — ends up holding only the injected, cfi-inert + // accessibility skip-links (prepended by a11y.ts, so no whitespace text + // nodes between them). The paginator's visible-range walker can anchor the + // relocate range on such an inert node; `getChildNodes(body)` is then empty, + // which used to crash `fromRange` with "Cannot destructure property + // 'nodeType' of 'param' as it is undefined". + describe('range anchored on an inert-only body (background-image section)', () => { + const inertOnlyBody = () => { + const doc = XHTML( + `t`, + ); + const body = doc.getElementById('body01')!; + // mirror a11y.ts: prepend cfi-inert skip-links into an otherwise empty body + for (const id of ['skip-next', 'skip-link']) { + const el = doc.createElement('div'); + el.setAttribute('cfi-inert', ''); + el.setAttribute('id', id); + body.prepend(el); + } + return doc; + }; + + it('does not throw for a collapsed range on the sole inert element', () => { + const doc = inertOnlyBody(); + const inert = doc.getElementById('skip-link')!; + const range = doc.createRange(); + range.setStart(inert, 0); + range.setEnd(inert, 0); + expect(() => CFI.fromRange(range)).not.toThrow(); + expect(typeof CFI.fromRange(range)).toBe('string'); + }); + + it('does not throw for a range spanning two inert siblings', () => { + const doc = inertOnlyBody(); + const first = doc.getElementById('skip-link')!; + const last = doc.getElementById('skip-next')!; + const range = doc.createRange(); + range.setStart(first, 0); + range.setEnd(last, 0); + expect(() => CFI.fromRange(range)).not.toThrow(); + }); + }); }); diff --git a/apps/readest-app/src/app/library/components/SettingsMenu.tsx b/apps/readest-app/src/app/library/components/SettingsMenu.tsx index f589544f..4af5d2d9 100644 --- a/apps/readest-app/src/app/library/components/SettingsMenu.tsx +++ b/apps/readest-app/src/app/library/components/SettingsMenu.tsx @@ -289,6 +289,12 @@ const SettingsMenu: React.FC = ({ onPullLibrary, setIsDropdow const coverDir = savedBookCoverPath ? savedBookCoverPath.split('/').pop() : 'Images'; const savedBookCoverDescription = `💾 ${coverDir}/last-book-cover.png`; + const lastSyncTime = Math.max( + settings.lastSyncedAtBooks || 0, + settings.lastSyncedAtConfigs || 0, + settings.lastSyncedAtNotes || 0, + ); + return ( = ({ onPullLibrary, setIsDropdow /> { + if (!user) { + navigateToLogin(router); + return; + } await pullLibrary(false, true); checkOPDSSubscriptions(true); }, async () => { + if (!user) { + navigateToLogin(router); + return; + } await pullLibrary(true, true); checkOPDSSubscriptions(true); }, diff --git a/apps/readest-app/src/app/reader/hooks/kosyncPreview.ts b/apps/readest-app/src/app/reader/hooks/kosyncPreview.ts index e2184196..f80a17f6 100644 --- a/apps/readest-app/src/app/reader/hooks/kosyncPreview.ts +++ b/apps/readest-app/src/app/reader/hooks/kosyncPreview.ts @@ -5,6 +5,9 @@ import { TranslationFunc } from '@/hooks/useTranslation'; export const getProgressPercentage = (pageInfo?: PageInfo): number => pageInfo && pageInfo.total > 0 ? (pageInfo.current + 1) / pageInfo.total : 0; +/** Formats a 0–1 progress fraction as a percentage string with 2 decimals. */ +export const formatProgressPercentage = (fraction: number): string => (fraction * 100).toFixed(2); + /** * Human-readable summary of the local reading position shown in the KOReader * sync-conflict dialog. @@ -20,7 +23,7 @@ export const getLocalProgressPreview = ( _: TranslationFunc, ): string => { const pageInfo = isFixedLayout ? local.section : local.pageinfo; - const percentage = Math.round(getProgressPercentage(pageInfo) * 100); + const percentage = formatProgressPercentage(getProgressPercentage(pageInfo)); const sectionLabel = local.sectionLabel?.trim(); if (!isFixedLayout && sectionLabel) { diff --git a/apps/readest-app/src/app/reader/hooks/kosyncProgress.ts b/apps/readest-app/src/app/reader/hooks/kosyncProgress.ts index 7a64397f..c819d252 100644 --- a/apps/readest-app/src/app/reader/hooks/kosyncProgress.ts +++ b/apps/readest-app/src/app/reader/hooks/kosyncProgress.ts @@ -1,3 +1,6 @@ +import { BookDoc } from '@/libs/document'; +import { FoliateView } from '@/types/view'; +import { getCFIFromXPointer } from '@/utils/xcfi'; import { KoSyncProgress } from '@/services/sync/KOSyncClient'; /** @@ -23,3 +26,34 @@ export const getRemoteFraction = (remote: KoSyncProgress): number | undefined => } return Math.min(percentage, 1); }; + +/** + * Resolves a remote KOReader position to a 0–1 progress fraction expressed in + * the LOCAL book's pagination terms. + * + * KOReader and Readest paginate differently, so the server-reported + * `percentage` is not directly comparable to Readest's own progress. When the + * remote position is a CREngine XPointer we convert it to a local CFI and ask + * the view for the equivalent fraction, giving an apples-to-apples value. + * Returns `undefined` when the position can't be resolved locally (non-XPointer + * progress, conversion failure, or an unknown CFI) so callers can fall back to + * the server-reported percentage. + */ +export const getRemoteLocalFraction = async ( + remote: KoSyncProgress, + view: FoliateView, + bookDoc: BookDoc, +): Promise => { + if (!isXPointerProgress(remote.progress)) return undefined; + try { + // Resolve against the XPointer's own spine section; the converter loads the + // correct off-screen document when it differs from the primary view. + const content = view.renderer.getContents().find((x) => x.index === view.renderer.primaryIndex); + const cfi = await getCFIFromXPointer(remote.progress!, content?.doc, content?.index, bookDoc); + const progress = await view.getCFIProgress(cfi); + return progress?.fraction; + } catch (error) { + console.error('Failed to resolve remote progress to a local fraction', error); + return undefined; + } +}; diff --git a/apps/readest-app/src/app/reader/hooks/useKOSync.ts b/apps/readest-app/src/app/reader/hooks/useKOSync.ts index f07cb3df..5f8fb859 100644 --- a/apps/readest-app/src/app/reader/hooks/useKOSync.ts +++ b/apps/readest-app/src/app/reader/hooks/useKOSync.ts @@ -10,8 +10,12 @@ import { BookDoc } from '@/libs/document'; import { debounce } from '@/utils/debounce'; import { eventDispatcher } from '@/utils/event'; import { getCFIFromXPointer, getXPointerFromCFI } from '@/utils/xcfi'; -import { getLocalProgressPreview, getProgressPercentage } from './kosyncPreview'; -import { getRemoteFraction, isXPointerProgress } from './kosyncProgress'; +import { + formatProgressPercentage, + getLocalProgressPreview, + getProgressPercentage, +} from './kosyncPreview'; +import { getRemoteFraction, getRemoteLocalFraction, isXPointerProgress } from './kosyncProgress'; import { useWindowActiveChanged } from './useWindowActiveChanged'; type SyncState = 'idle' | 'checking' | 'conflict' | 'synced' | 'error'; @@ -154,7 +158,14 @@ export const useKOSync = (bookKey: string) => { ) => { let remotePreview = ''; const remotePercentage = remote.percentage || 0; - const conflictProgressDiffThreshold = 0.0001; + // Progress last pushed from this same device is just our own earlier + // position; only treat a sizeable jump (≥1%) as a conflict so we don't + // prompt on the sub-page drift between a push and the next pull. + const isSameDevice = !!remote.device_id && remote.device_id === settings.kosync.deviceId; + const conflictProgressDiffThreshold = isSameDevice ? 0.01 : 0.0001; + // The remote progress as a percentage to compare against the local one; + // refined to a locally-resolved fraction for reflowable books below. + let remoteComparePercentage = remotePercentage; let showConflictDetails = false; const isFixedLayout = FIXED_LAYOUT_FORMATS.has(book.format); @@ -173,28 +184,36 @@ export const useKOSync = (bookKey: string) => { remotePreview = _('Page {{page}} of {{total}} ({{percentage}}%)', { page: remotePage, total: remoteTotalPages, - percentage: Math.round(remotePercentage * 100), + percentage: formatProgressPercentage(remotePercentage), }); } else { remotePreview = _('Approximately page {{page}} of {{total}} ({{percentage}}%)', { page: remotePage, total: remoteTotalPages, - percentage: Math.round(remotePercentage * 100), + percentage: formatProgressPercentage(remotePercentage), }); } showConflictDetails = Math.abs(localPercentage - remotePercentage) > conflictProgressDiffThreshold; } else { remotePreview = _('Approximately {{percentage}}%', { - percentage: Math.round(remotePercentage * 100), + percentage: formatProgressPercentage(remotePercentage), }); } } else { + // KOReader's reported percentage comes from its own pagination, so it's + // not directly comparable to Readest's progress. Resolve the remote + // position to a local fraction for an apples-to-apples comparison and + // fall back to the reported percentage only when it can't be resolved + // locally (non-XPointer progress or a missing section). + const view = getView(bookKey); + const localFraction = view ? await getRemoteLocalFraction(remote, view, bookDoc) : undefined; + remoteComparePercentage = localFraction ?? remotePercentage; remotePreview = _('Approximately {{percentage}}%', { - percentage: Math.round(remotePercentage * 100), + percentage: formatProgressPercentage(remoteComparePercentage), }); showConflictDetails = - Math.abs(localPercentage - remotePercentage) > conflictProgressDiffThreshold; + Math.abs(localPercentage - remoteComparePercentage) > conflictProgressDiffThreshold; } if (showConflictDetails) { @@ -205,6 +224,7 @@ export const useKOSync = (bookKey: string) => { remote: { ...remote, preview: remotePreview }, }); } + return showConflictDetails; }; const pushProgress = useMemo( @@ -218,6 +238,7 @@ export const useKOSync = (bookKey: string) => { const progress = await generateKOProgress(); if (!currentBook || !progress || !progress.koProgress) return; + console.log('[KOSync] Pushing progress'); await kosyncClient.updateProgress(currentBook, progress.koProgress, progress.percentage); }, 5000), // eslint-disable-next-line react-hooks/exhaustive-deps @@ -248,6 +269,7 @@ export const useKOSync = (bookKey: string) => { setSyncState('synced'); return; } + console.log('[KOSync] Pulled remote progress', { bookKey, remoteProgress }); const localTimestamp = bookData?.config?.updatedAt || book.updatedAt; const remoteTimestamp = remoteProgress.timestamp @@ -258,8 +280,10 @@ export const useKOSync = (bookKey: string) => { applyRemoteProgress(book, bookDoc, remoteProgress); setSyncState('synced'); } else if (strategy === 'prompt') { - promptedSync(book, bookDoc, progress, remoteProgress); - setSyncState('conflict'); + // Only stay in the conflict state when there's an actual conflict to + // resolve; otherwise return to 'synced' so auto-push keeps working. + const hasConflict = await promptedSync(book, bookDoc, progress, remoteProgress); + setSyncState(hasConflict ? 'conflict' : 'synced'); } else { setSyncState('synced'); } diff --git a/apps/readest-app/src/components/settings/SettingsDialog.tsx b/apps/readest-app/src/components/settings/SettingsDialog.tsx index 61aaf12b..38337a1b 100644 --- a/apps/readest-app/src/components/settings/SettingsDialog.tsx +++ b/apps/readest-app/src/components/settings/SettingsDialog.tsx @@ -107,9 +107,9 @@ const SettingsDialog: React.FC<{ bookKey: string }> = ({ bookKey }) => { label: _('Language'), }, { - tab: 'TTS', - icon: PiSpeakerHigh, - label: _('TTS'), + tab: 'Integrations', + icon: RiShareLine, + label: _('Integrations'), }, { tab: 'AI', @@ -118,9 +118,9 @@ const SettingsDialog: React.FC<{ bookKey: string }> = ({ bookKey }) => { disabled: process.env.NODE_ENV === 'production', }, { - tab: 'Integrations', - icon: RiShareLine, - label: _('Integrations'), + tab: 'TTS', + icon: PiSpeakerHigh, + label: _('TTS'), }, { tab: 'Custom', diff --git a/packages/foliate-js b/packages/foliate-js index c3b2d095..569cc060 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit c3b2d095d520946db68bcff3e418489d4911796b +Subproject commit 569cc06076165f057239332484d158fd1bb79313