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

fix(reader): read position from the API at open; never write a restored position

Browse Source 
The reader page embedded a snapshot of reading state (position,
bookmarks) server-side at render time. Browsers may reuse that HTML
(heuristic caching, bfcache), so opening a book could restore a stale
position — and worse, the restore's relocate auto-saved it back,
overwriting a newer device push minutes later. A KOReader sync followed
by opening the web reader would silently revert the row to the old web
position; the row's source and the rendered page disagreed.

The web reader is intrinsically tied to the server, so it has no business
preserving reading state client-side:

- The rendered page now carries only immutable book metadata. The reader
  fetches progress fresh (cache: no-store) from the existing progress
  API at open and restores with the same priority as before (page for
  fixed-layout, CFI, percentage, fresh start); a failed fetch opens at
  the start and writes nothing. Initial bookmarks likewise come from
  their endpoint instead of the embed; annotations already did.
- Progress saves are gated on deliberate navigation only (page turns,
  keys, slider, search/TOC/bookmark/back-stack jumps, tap zones — each
  marks the session as user-moved). Restores and section-load
  relocations never write, so displaying a position can no longer
  clobber a newer one. A bfcache-resurrected page resets the flag and
  cannot write its frozen position either. This replaces the old
  five-second post-init suppression, which a stale page bypassed.
- The server-rendered initial progress badges render a neutral
  placeholder until the first relocate fills them (sub-second).

No API, schema, or sync-engine changes. Normal reading saves exactly as
before — the first save now simply waits for the first real page turn.
This commit is contained in:
john-okeefe
2026-09-09 14:52:30 -04:00
parent 75c1d9bb95
commit ce3ae31ced
4 changed files with 130 additions and 272 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/router/reader.go
+6 -69
View File
@@ -1,18 +1,15 @@
package router
import (
"bookhoard/internal/database"
"bookhoard/internal/handlers"
"bookhoard/internal/services"
"bookhoard/internal/sync"
"bookhoard/internal/utils"
"bookhoard/templates"
"bytes"
"errors"
"net/http"
"github.com/google/uuid"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/labstack/echo/v5"
)
@@ -69,21 +66,10 @@ func registerReaderRoutes(cfg *Config) {
if !visible {
return renderErrorPage(c, "Access denied", "access_denied")
}
// Get reading progress
var progress database.ReadingProgress
progress, err = cfg.Queries.GetReadingProgress(c.Request().Context(), database.GetReadingProgressParams{
MediaItemID: pgtype.UUID{Bytes: parsedUUID, Valid: true},
UserID: uuidToPGType(userUUID),
})
if err != nil && !errors.Is(err, pgx.ErrNoRows) {
progress = database.ReadingProgress{}
}
// Get bookmarks
bookmarks, _ := cfg.Queries.GetMediaBookmarks(c.Request().Context(), database.GetMediaBookmarksParams{
MediaItemID: pgtype.UUID{Bytes: parsedUUID, Valid: true},
UserID: uuidToPGType(userUUID),
})
// Convert to template types
// Convert to template types. Reading state is deliberately NOT
// fetched or embedded: the reader pulls position, bookmarks, and
// annotations from the APIs at open time so the page can never
// carry (nor write back) a stale snapshot.
mediaUUID, _ := uuid.FromBytes(mediaItem.ID.Bytes[0:16])
libUUID, _ := uuid.FromBytes(mediaItem.LibraryID.Bytes[0:16])
metadata := templates.ReaderMetadata{
@@ -104,58 +90,9 @@ func registerReaderRoutes(cfg *Config) {
TotalCharacters: mediaItem.TotalCharacters.Int64,
EstimatedPages: sync.EstimatedPages(mediaItem.TotalCharacters.Int64),
}
// Progress conversion (inline)
progressUUID, _ := uuid.FromBytes(progress.ID.Bytes[0:16])
progressMediaUUID, _ := uuid.FromBytes(progress.MediaItemID.Bytes[0:16])
progressUserUUID, _ := uuid.FromBytes(progress.UserID.Bytes[0:16])
templateProgress := templates.ReadingProgress{
ID: progressUUID.String(),
MediaItemID: progressMediaUUID.String(),
UserID: progressUserUUID.String(),
CurrentPage: int(progress.CurrentPage.Int32),
TotalPages: int(progress.TotalPages.Int32),
Percentage: progress.Percentage.Float64 * 100,
EpubCfi: textToString(progress.Epubcfi),
LastReadAt: progress.LastReadAt.Time,
Chapter: int(progress.Chapter.Int32),
ChapterProgress: progress.ChapterProgress.Float64 * 100,
FormatGroup: mediaItem.FormatGroup,
}
// Bookmarks conversion (inline, with loop)
templateBookmarks := make([]templates.Bookmark, len(bookmarks))
for i, b := range bookmarks {
bookmarkUUID, _ := uuid.FromBytes(b.ID.Bytes[0:16])
bookmarkMediaUUID, _ := uuid.FromBytes(b.MediaItemID.Bytes[0:16])
bookmarkUserUUID, _ := uuid.FromBytes(b.UserID.Bytes[0:16])
var pageNumber *int
if b.PageNumber.Valid {
val := int(b.PageNumber.Int32)
pageNumber = &val
}
var chapterNumber *int
if b.ChapterNumber.Valid {
val := int(b.ChapterNumber.Int32)
chapterNumber = &val
}
templateBookmarks[i] = templates.Bookmark{
ID: bookmarkUUID.String(),
MediaItemID: bookmarkMediaUUID.String(),
UserID: bookmarkUserUUID.String(),
PageNumber: pageNumber,
ChapterNumber: chapterNumber,
CfiPosition: textToString(b.CfiPosition),
Title: b.Title,
Position: textToString(b.Position),
Notes: textToString(b.Notes),
CreatedAt: b.CreatedAt.Time,
}
}
// 8. Render template
// Render template
var buf bytes.Buffer
err = templates.Reader(user, metadata, templateProgress, templateBookmarks).Render(c.Request().Context(), &buf)
err = templates.Reader(user, metadata).Render(c.Request().Context(), &buf)
if err != nil {
return renderErrorPage(c, "Error rendering reader", "render_error")
}
templates/reader.templ
+11 -54
View File
@@ -5,7 +5,11 @@ import (
"fmt"
)
func readerInitExpr(metadata ReaderMetadata, progress ReadingProgress, bookmarks []Bookmark) string {
// The init config carries only immutable book metadata. Reading state
// (position, bookmarks, annotations) is never embedded: the reader fetches
// it from the APIs at open time, so the page can never carry — nor write
// back — a stale snapshot of it.
func readerInitExpr(metadata ReaderMetadata) string {
config := map[string]interface{}{
"mediaItemId": metadata.MediaItemID,
"fileUrl": metadata.FileURL,
@@ -13,42 +17,11 @@ func readerInitExpr(metadata ReaderMetadata, progress ReadingProgress, bookmarks
"readingDirection": metadata.ReadingDirection,
"mangaType": metadata.MangaType,
}
if progress.Percentage > 0 {
config["savedPercentage"] = progress.Percentage / 100
}
if progress.EpubCfi != "" {
config["savedCfi"] = progress.EpubCfi
}
// Fixed-layout & comic formats: the page index is the canonical, exact
// locator (pages are fixed images). Pass it so the reader restores by page.
if (metadata.FormatGroup == "fixed_layout" || metadata.FormatGroup == "comic_archive") && progress.CurrentPage > 0 {
config["savedPage"] = progress.CurrentPage
if progress.TotalPages > 0 {
config["savedTotalPages"] = progress.TotalPages
}
}
if len(bookmarks) > 0 {
items := make([]map[string]interface{}, 0, len(bookmarks))
for _, b := range bookmarks {
var page any
if b.PageNumber != nil {
page = *b.PageNumber
}
items = append(items, map[string]interface{}{
"id": b.ID,
"title": b.Title,
"positionLabel": b.Position,
"cfi": b.CfiPosition,
"page": page,
})
}
config["bookmarks"] = items
}
jsonBytes, _ := json.Marshal(config)
return fmt.Sprintf("initReader(%s)", string(jsonBytes))
}
templ Reader(user User, metadata ReaderMetadata, progress ReadingProgress, bookmarks []Bookmark) {
templ Reader(user User, metadata ReaderMetadata) {
<!DOCTYPE html>
<html lang="en">
<head>
@@ -64,7 +37,7 @@ templ Reader(user User, metadata ReaderMetadata, progress ReadingProgress, bookm
</head>
<body
x-data="readerShell"
x-init={ readerInitExpr(metadata, progress, bookmarks) }
x-init={ readerInitExpr(metadata) }
class={ "theme-" + user.Theme + " h-screen overflow-hidden" }
>
<!-- Reading surface: edge-to-edge. Chrome overlays translucently;
@@ -96,7 +69,7 @@ templ Reader(user User, metadata ReaderMetadata, progress ReadingProgress, bookm
</div>
</div>
@ReaderChrome(metadata, progress)
@ReaderChrome(metadata)
<!-- Drawer scrim -->
<div
@@ -311,7 +284,7 @@ templ Reader(user User, metadata ReaderMetadata, progress ReadingProgress, bookm
</html>
}
templ ReaderChrome(metadata ReaderMetadata, progress ReadingProgress) {
templ ReaderChrome(metadata ReaderMetadata) {
<div id="reader-chrome" class="transition-opacity duration-300" :class="chromeVisible ? 'opacity-100' : 'chrome-hidden opacity-0 pointer-events-none'">
<!-- Top bar -->
<div id="reader-topbar" class="fixed top-0 left-0 right-0 border-b z-40 pt-[env(safe-area-inset-top)] reader-glass">
@@ -365,17 +338,7 @@ templ ReaderChrome(metadata ReaderMetadata, progress ReadingProgress) {
<div class="flex items-center gap-1">
<div class="w-px h-6 reader-sep"></div>
<div id="progress-display" @click="cycleProgressMode()" :title="progressTooltip()" class="text-sm min-w-[4rem] max-w-[5rem] sm:max-w-none text-center cursor-pointer truncate whitespace-nowrap overflow-hidden">
<span class="hidden sm:inline" x-text="progressLabel"></span><span x-text="progressMain">
if progress.FormatGroup == "reflowable" {
if metadata.EstimatedPages > 0 {
{ fmt.Sprintf("%.0f%% · Page %d/%d", progress.Percentage, progress.CurrentPage, metadata.EstimatedPages) }
} else {
{ fmt.Sprintf("%.0f%%", progress.Percentage) }
}
} else {
{ fmt.Sprintf("%d/%d", progress.CurrentPage, progress.TotalPages) }
}
</span>
<span class="hidden sm:inline" x-text="progressLabel"></span><span x-text="progressMain">—</span>
</div>
<div class="w-px h-6 reader-sep"></div>
<button @click="toggleTOC()" class="p-1.5 sm:p-2 rounded-lg hover:bg-gray-700" title="Table of Contents (t)">📖</button>
@@ -467,13 +430,7 @@ templ ReaderChrome(metadata ReaderMetadata, progress ReadingProgress) {
<!-- Progress + TOC -->
<div class="flex items-center gap-1">
<div id="progress-display-fx" @click="cycleProgressMode()" :title="progressTooltip()" class="text-sm min-w-[3.5rem] text-center cursor-pointer truncate whitespace-nowrap overflow-hidden">
<span x-text="progressMain">
if progress.FormatGroup == "reflowable" {
{ fmt.Sprintf("%.0f%%", progress.Percentage) }
} else {
{ fmt.Sprintf("%d/%d", progress.CurrentPage, progress.TotalPages) }
}
</span>
<span x-text="progressMain">—</span>
</div>
<button @click="toggleTOC()" class="p-1.5 sm:p-2 rounded-lg hover:bg-gray-700" title="Table of Contents (t)">📖</button>
</div>
templates/reader_templ.go
+39 -128
View File
File diff suppressed because one or more lines are too long
web/src/reader/reader.ts
+74 -21
View File
@@ -482,7 +482,11 @@ document.addEventListener("alpine:init", () => {
tocItems: [] as any[],
mediaItemId: "" as string,
saveTimeout: null as ReturnType<typeof setTimeout> | null,
initTime: 0 as number,
// 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,
contextText: "" as string,
readingTheme: "light" as string,
readingMode: "light" as string,
@@ -564,20 +568,13 @@ document.addEventListener("alpine:init", () => {
formatGroup: string;
readingDirection: string;
mangaType: string;
savedPercentage?: number;
savedCfi?: string;
savedPage?: number;
savedTotalPages?: number;
bookmarks?: {
id: string;
title: string;
positionLabel: string;
cfi: string;
page: number | null;
}[];
}) {
this.mediaItemId = config.mediaItemId;
this.bookmarkItems = config.bookmarks ?? [];
// Reading state (position, bookmarks, annotations) is never baked
// into the rendered page: the web reader is intrinsically tied to
// the server, so it reads all of it from the APIs at open time —
// a device sync between render and open can never be shadowed by a
// stale snapshot.
this.isComic = config.formatGroup === "comic_archive";
// Reading flow for comics is a per-book preference (a webtoon title
// vs. a paged manga volume); read before the renderer is chosen.
@@ -916,15 +913,18 @@ document.addEventListener("alpine:init", () => {
document.addEventListener("keydown", (ev: KeyboardEvent) =>
this.handleKeydown(ev),
);
if (this.isFixedLayout && config.savedPage != null && config.savedPage > 0) {
// Reading position comes from the database, fetched fresh at open
// (the rendered page carries no snapshot of it).
const saved = await this.fetchSavedLocation();
if (this.isFixedLayout && saved.page != null && saved.page > 0) {
// Fixed-layout & comics: a page index is the exact, universal locator.
// A bare number navigates directly to the section index in foliate.
await this.view.init({ lastLocation: config.savedPage - 1 })
} else if (config.savedCfi) {
await this.view.init({ lastLocation: config.savedCfi })
} else if (config.savedPercentage && config.savedPercentage > 0) {
await this.view.init({ lastLocation: saved.page - 1 })
} else if (saved.cfi) {
await this.view.init({ lastLocation: saved.cfi })
} else if (saved.percentage != null && saved.percentage > 0) {
await this.view.init({
lastLocation: { fraction: config.savedPercentage },
lastLocation: { fraction: saved.percentage },
})
} else {
await this.view.init({})
@@ -938,9 +938,14 @@ document.addEventListener("alpine:init", () => {
this.renderer.setAttribute("interaction-mode", this.interactionMode);
}
this.fxZoomed = this.isFixedLayout && this.renderer?.zoom != null;
this.initTime = Date.now();
// A bfcache-resurrected page is stale by definition: forbid it from
// writing its frozen position back until the user navigates again.
window.addEventListener("pageshow", (e: PageTransitionEvent) => {
if (e.persisted) this.userMoved = false;
});
this.fetchReadingSpeed();
this.refreshAnnotations();
this.refreshBookmarks();
this.setupChrome();
this.setupTapZones();
},
@@ -1536,8 +1541,44 @@ document.addEventListener("alpine:init", () => {
/* ignore note errors */
}
},
// 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.
async fetchSavedLocation(): Promise<{
cfi?: string;
page?: number;
percentage?: number;
}> {
const token = getToken();
if (!token || !this.mediaItemId) return {};
try {
const resp = await fetch(
`/api/media-items/${this.mediaItemId}/progress`,
{
headers: { Authorization: `Bearer ${token}` },
cache: "no-store",
},
);
if (!resp.ok) return {};
const row: any = await resp.json();
const cfi: string = row?.epubcfi?.String ?? row?.epubcfi ?? "";
const page: number = row?.current_page?.Int32 ?? row?.current_page ?? 0;
// The stored percentage is a 0-1 fraction.
const pct: number = row?.percentage?.Float64 ?? row?.percentage ?? 0;
return {
cfi: typeof cfi === "string" ? cfi : "",
page: typeof page === "number" ? page : 0,
percentage: typeof pct === "number" ? pct : 0,
};
} catch (_e) {
return {};
}
},
debouncedSaveProgress(fraction: number, location: any, cfi: string) {
if (Date.now() - this.initTime < 5000) return;
// Only deliberate navigation writes progress: displaying a restored
// position must never overwrite a newer device push.
if (!this.userMoved) return;
if (this.saveTimeout) clearTimeout(this.saveTimeout);
this.saveTimeout = setTimeout(() => {
this.saveProgress(fraction, location, cfi);
@@ -1678,18 +1719,23 @@ 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() {
@@ -1868,9 +1914,11 @@ 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;
@@ -1899,6 +1947,7 @@ 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);
},
@@ -1914,6 +1963,7 @@ 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;
@@ -2041,6 +2091,7 @@ 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;
@@ -2048,9 +2099,11 @@ 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);