Bookhoard/bookhoard
0
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 17 Wiki Activity

fix(reader): save progress on real position change, not on detected intent

Browse Source 
The userMoved gate from ce3ae31 broke progress saving entirely for
EPUBs. The flag was set only in the app's navigation wrappers
(goLeft/goRight, keys, slider, search/TOC/bookmark/back-stack jumps),
but foliate-js's paginator handles the most common reading gestures
itself — touch-swipe paging, scrolled-mode reading, in-content links,
selection auto-advance — dispatching relocate directly without ever
calling those wrappers. Every one of those relocations hit the
"if (!this.userMoved) return" guard, so the position never saved at
all on EPUB; only tap-zone-paged formats (comics/fixed layout) kept
saving, which matched the intermittent reports.

Detecting intent was the wrong tool: the set of foliate-internal
navigation paths is open-ended and lives in a forked dependency.
Compare the position itself instead:

- The relocate handler records the latest CFI (lastCfi), and the
  baseline (lastSyncedCfi/lastSyncedFraction) is captured right after
  view.init() resolves — i.e. the restored position, or the start of
  the book on a fresh open.
- debouncedSaveProgress saves only when the position actually moved:
  CFI comparison for reflowable books, fraction comparison (1e-4
  epsilon) for CFI-less fixed layout and PDF.
- The baseline updates after each successful save, and a
  bfcache-resurrected page re-baselines to its frozen position, so the
  anti-clobber property survives: a displayed/restored position can
  still never overwrite a newer device push.

The twelve userMoved assignments in the wrapper methods are gone —
change detection covers deliberate jumps and internal gestures alike.
Backend untouched; SaveProgress was never the problem.
This commit is contained in:
john-okeefe
2026-09-11 21:50:03 -04:00
parent 3514b4dc1c
commit 15b0297cc9
1 changed files with 38 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/reader/reader.ts
+38 -26
View File
@@ -482,11 +482,14 @@ document.addEventListener("alpine:init", () => {
tocItems: [] as any[],
mediaItemId: "" as string,
saveTimeout: null as ReturnType<typeof setTimeout> | null,
// Set only by deliberate navigation (page turns, jumps, slider). The
// restore at open time and section-load relocations never set it, so
// progress saves can only ever write a position the user actually
// moved to — never a stale restore clobbering a newer device push.
userMoved: false as boolean,
// Position last known to be stored (the restore at open time, or the
// last successful save). Relocations that don't move from it are never
// written back, so a restored position can't clobber a newer device
// push — while swipe/scroll paging (handled inside foliate, with no
// wrapper method to flag) still saves normally.
lastSyncedCfi: "" as string,
lastSyncedFraction: -1 as number,
lastCfi: "" as string,
contextText: "" as string,
readingTheme: "light" as string,
readingMode: "light" as string,
@@ -883,6 +886,7 @@ document.addEventListener("alpine:init", () => {
const { fraction, location, pageItem, cfi, tocItem, section } =
e.detail;
this.hideSelectionPopover();
this.lastCfi = cfi || "";
this.lastRelocateDetail = {
fraction,
location,
@@ -944,6 +948,11 @@ document.addEventListener("alpine:init", () => {
} else {
await this.view.init({})
}
// The position restored above (or the start of the book on a fresh
// open) is the baseline: only relocations that actually move from
// it may write progress.
this.lastSyncedCfi = this.lastCfi;
this.lastSyncedFraction = this.lastRelocateDetail?.fraction ?? -1;
// The renderer only knows it's a PDF once frames exist (they carry
// pdf.js onZoom), i.e. after init has rendered the first spread.
// Read it now and apply the saved pointer mode — this also makes the
@@ -953,10 +962,15 @@ document.addEventListener("alpine:init", () => {
this.renderer.setAttribute("interaction-mode", this.interactionMode);
}
this.fxZoomed = this.isFixedLayout && this.renderer?.zoom != null;
// A bfcache-resurrected page is stale by definition: forbid it from
// writing its frozen position back until the user navigates again.
// A bfcache-resurrected page is stale by definition: re-baseline to
// its frozen position so it can't write that back until the user
// actually navigates again.
window.addEventListener("pageshow", (e: PageTransitionEvent) => {
if (e.persisted) this.userMoved = false;
if (e.persisted) {
this.lastSyncedCfi = this.lastCfi;
this.lastSyncedFraction =
this.lastRelocateDetail?.fraction ?? -1;
}
});
this.fetchReadingSpeed();
this.refreshAnnotations();
@@ -1557,9 +1571,9 @@ document.addEventListener("alpine:init", () => {
}
},
// Fresh reading position from the database — the single source of
// truth at open time. Fails soft to a fresh start: the userMoved gate
// guarantees merely opening (even at the wrong spot) can never
// overwrite the stored position.
// truth at open time. Fails soft to a fresh start: change detection
// against the restored baseline guarantees merely opening (even at
// the wrong spot) can never overwrite the stored position.
async fetchSavedLocation(): Promise<{
cfi?: string;
page?: number;
@@ -1591,9 +1605,17 @@ document.addEventListener("alpine:init", () => {
}
},
debouncedSaveProgress(fraction: number, location: any, cfi: string) {
// Only deliberate navigation writes progress: displaying a restored
// position must never overwrite a newer device push.
if (!this.userMoved) return;
// Only an actual change from the last stored position writes
// progress: displaying a restored position must never overwrite a
// newer device push. Swipes and scrolls are handled inside foliate
// with no wrapper method to flag, so position — not intent — is the
// signal. Books without CFIs (fixed layout, PDF) compare fraction.
const currentCfi = cfi || "";
const changed =
currentCfi || this.lastSyncedCfi
? currentCfi !== this.lastSyncedCfi
: Math.abs(fraction - this.lastSyncedFraction) > 1e-4;
if (!changed) return;
if (this.saveTimeout) clearTimeout(this.saveTimeout);
this.saveTimeout = setTimeout(() => {
this.saveProgress(fraction, location, cfi);
@@ -1644,6 +1666,8 @@ document.addEventListener("alpine:init", () => {
},
body: JSON.stringify(body),
});
this.lastSyncedCfi = cfi || "";
this.lastSyncedFraction = fraction;
} catch (_e) {
// silent fail — progress save is non-critical
}
@@ -1734,23 +1758,18 @@ document.addEventListener("alpine:init", () => {
saveSettings({ double_page_spread: this.doublePageSpread });
},
goLeft() {
this.userMoved = true;
this.view?.goLeft?.();
},
goRight() {
this.userMoved = true;
this.view?.goRight?.();
},
nextPage() {
this.userMoved = true;
this.view?.next?.();
},
previousPage() {
this.userMoved = true;
this.view?.prev?.();
},
goToFraction(value: string) {
this.userMoved = true;
this.view?.goToFraction?.(parseFloat(value));
},
toggleTOC() {
@@ -1929,11 +1948,9 @@ document.addEventListener("alpine:init", () => {
},
goToSearchResult(item: { cfi?: string; page?: number | null }) {
if (item.cfi) {
this.userMoved = true;
this.pushBackStack();
this.view?.goTo?.(item.cfi);
} else if (item.page != null) {
this.userMoved = true;
this.pushBackStack();
this.view?.goTo?.(item.page);
} else return;
@@ -1962,7 +1979,6 @@ document.addEventListener("alpine:init", () => {
goBackToLocation() {
const loc = this.backStack.pop();
if (!loc) return;
this.userMoved = true;
if (loc.cfi) this.view?.goTo?.(loc.cfi);
else if (typeof loc.page === "number") this.view?.goTo?.(loc.page);
},
@@ -1978,7 +1994,6 @@ document.addEventListener("alpine:init", () => {
},
goToTOCItem(item: any) {
if (this.view && item.href) {
this.userMoved = true;
this.pushBackStack();
this.view.goTo(item.href);
this.tocOpen = false;
@@ -2106,7 +2121,6 @@ document.addEventListener("alpine:init", () => {
},
goToPage(index: number) {
if (!this.view || typeof index !== "number" || index < 0) return;
this.userMoved = true;
this.pushBackStack();
this.view.goTo(index);
this.tocOpen = false;
@@ -2114,11 +2128,9 @@ document.addEventListener("alpine:init", () => {
goToBookmark(item: { cfi: string; page: number | null }) {
if (!this.view) return;
if (item.cfi) {
this.userMoved = true;
this.pushBackStack();
this.view.goTo(item.cfi);
} else if (item.page != null && item.page > 0) {
this.userMoved = true;
this.pushBackStack();
// Fixed-layout/comic: sections are pages; foliate takes an index.
this.view.goTo(item.page - 1);