fix: Accurate bookmark chapter titles and page numbers

lib/Epub/Epub.h

@@ -37,6 +37,10 @@ class Epub {
   void discoverCssFilesFromZip();
   void parseCssFiles() const;
 
+  // Byte-weighted primitive: maps a [0,1] fraction through a spine to book
+  // progress. Page-based callers use calculateProgressForPage (public) instead.
+  float calculateProgress(uint16_t currentSpineIndex, float currentSpineRead) const;
+
  public:
   explicit Epub(std::string filepath, const std::string& cacheDir) : filepath(std::move(filepath)) {
     // create a cache key based on the filepath
@@ -83,10 +87,6 @@ class Epub {
 
   size_t getBookSize() const;
 
-  // Byte-weighted primitive: maps a [0,1] fraction through a spine to book
-  // progress. Page-based callers use calculateProgressForPage (public) instead.
-  float calculateProgress(uint16_t currentSpineIndex, float currentSpineRead) const;
-
   // Book progress (0.0-1.0) for a 0-based page within a spine. The 1-based
   // fraction ((pageInSpine + 1) / spinePageCount) makes the final page read as
   // a full 1.0. Shared by all progress readouts so the convention can't drift.

lib/Epub/Epub/SpineItem.cpp

@@ -27,6 +27,16 @@ bool SpineItem::loadCacheFile(const int fontId, const float lineCompression, con
   return true;
 }
 
+bool SpineItem::loadForQuery() {
+  // loadCachedPageCount must run first: buildTocBoundariesFromFile reads
+  // getPageCount() when filling the last boundary's endPage.
+  if (!section_.loadCachedPageCount()) {
+    return false;
+  }
+  buildTocBoundariesFromFile();
+  return true;
+}
+
 bool SpineItem::createCacheFile(const int fontId, const float lineCompression, const bool extraParagraphSpacing,
                                 const uint8_t paragraphAlignment, const uint16_t viewportWidth,
                                 const uint16_t viewportHeight, const bool hyphenationEnabled, const bool embeddedStyle,

lib/Epub/Epub/SpineItem.h

@@ -73,6 +73,11 @@ class SpineItem {
                        uint16_t viewportWidth, uint16_t viewportHeight, bool hyphenationEnabled, bool embeddedStyle,
                        uint8_t imageRendering, bool focusReadingEnabled, FunctionRef<void()> popupFn = nullptr);
 
+  // Read-only load for TOC/page queries: populates pageCount and tocBoundaries
+  // from the cache, without loadCacheFile's render-param validation (which
+  // removes the cache on mismatch). False if the cache is missing or stale.
+  bool loadForQuery();
+
   bool clearCache() const { return section_.clearCache(); }
   std::unique_ptr<Page> loadPageFromSectionFile();
 

lib/Typesetter/Typesetter/Section.cpp

@@ -248,6 +248,15 @@ std::optional<uint16_t> Section::getCachedPageCount() const {
   return count;
 }
 
+bool Section::loadCachedPageCount() {
+  const auto count = getCachedPageCount();
+  if (!count) {
+    return false;
+  }
+  pageCount_ = *count;
+  return true;
+}
+
 bool Section::forEachAnchor(FunctionRef<bool(uint32_t keyLen)> predicate,
                             FunctionRef<bool(const std::string& key, uint16_t page)> consumer) const {
   HalFile f;

lib/Typesetter/Typesetter/Section.h

@@ -105,6 +105,11 @@ class Section {
   // settings changes.
   std::optional<uint16_t> getCachedPageCount() const;
 
+  // Populate pageCount from the cached header without validating render params
+  // and (unlike loadHeader) without removing the file on mismatch. False if the
+  // file is missing or the version doesn't match.
+  bool loadCachedPageCount();
+
   // Iterate the on-disk anchor map. Length-aware so callers can skip
   // allocating the key string for entries that can't match anything they
   // care about.

src/activities/reader/EpubReaderBookmarksActivity.cpp

@@ -1,11 +1,12 @@
 #include "EpubReaderBookmarksActivity.h"
 
+#include <Epub/SpineItem.h>
 #include <GfxRenderer.h>
 #include <I18n.h>
 #include <Logging.h>
-#include <Typesetter/Section.h>
 
 #include <algorithm>
+#include <optional>
 #include <string>
 
 #include "MappedInputManager.h"
@@ -20,12 +21,6 @@ constexpr int DELETE_MODE_DISPLAY = 1;
 constexpr int DELETE_MODE_CONFIRM = 2;
 
 constexpr int LINE_HEIGHT = 60;
-
-// Reuse the SpineItem cache-path convention so an ad-hoc Section for a
-// bookmark's spine reads the same cache file the EpubReaderActivity wrote.
-std::string sectionPathForSpine(const Epub& epub, uint16_t spineIndex) {
-  return epub.getCachePath() + "/sections/" + std::to_string(spineIndex) + ".bin";
-}
 }  // namespace
 
 void EpubReaderBookmarksActivity::onEnter() {
@@ -67,24 +62,49 @@ EpubReaderBookmarksActivity::EntryView EpubReaderBookmarksActivity::resolveEntry
   view.bookmark = entry;
   view.valid = true;
 
-  Section sec(sectionPathForSpine(*epub, entry.spineIndex));
-  view.chapterPageCount = sec.getCachedPageCount().value_or(0);
+  // Read-only load: never validates render params or removes the cache, so
+  // browsing bookmarks can't disturb the spine caches the reader relies on.
+  SpineItem spine(epub, entry.spineIndex, renderer);
+  if (!spine.loadForQuery()) {
+    // Cache missing or stale: fall back to a best-effort spine-level title and
+    // leave the page/percent readout suppressed (spinePageCount stays 0).
+    const auto tocIdx = epub->getTocIndexForSpineIndex(entry.spineIndex);
+    view.chapterTitle = tocIdx ? epub->getTocItem(*tocIdx).title : "";
+    return view;
+  }
+  view.spinePageCount = spine.getPageCount();
 
-  // Resolve paragraphIndex (with optional liIndex refinement) to a page in
-  // the chapter. liIndex first-appearance can land on a later page than the
-  // paragraph's first-appearance when the bookmark sits inside a list that
-  // spans pages; taking max keeps the resolution from regressing past the
-  // paragraph start.
-  uint16_t page = sec.getPageForParagraphIndex(entry.paragraphIndex).value_or(0);
+  // Resolve paragraphIndex (with optional liIndex refinement) to a spine page.
+  // liIndex first-appearance can land on a later page than the paragraph's
+  // first-appearance when the bookmark sits inside a list that spans pages;
+  // taking max keeps the resolution from regressing past the paragraph start.
+  uint16_t spinePage = spine.getPageForParagraphIndex(entry.paragraphIndex).value_or(0);
   if (entry.liIndex != BookmarkEntry::NO_LI_INDEX) {
-    if (const auto liPage = sec.getPageForListItemIndex(entry.liIndex)) {
-      page = std::max(page, *liPage);
+    if (const auto liPage = spine.getPageForListItemIndex(entry.liIndex)) {
+      spinePage = std::max(spinePage, *liPage);
     }
   }
-  view.resolvedPage = page;
+  view.spinePage = spinePage;
 
-  const auto tocIdx = epub->getTocIndexForSpineIndex(entry.spineIndex);
+  // Page-accurate chapter title: getTocIndexForPage walks this spine's
+  // anchor-derived boundaries, so a bookmark in a later sub-chapter of a
+  // multi-chapter spine resolves to that sub-chapter, not the spine's first.
+  const auto tocIdx = spine.getTocIndexForPage(spinePage);
   view.chapterTitle = tocIdx ? epub->getTocItem(*tocIdx).title : "";
+
+  // Chapter-relative page readout. getPageRangeForTocIndex gives the chapter's
+  // span within this spine: for single-spine chapters it matches the reader's
+  // status bar exactly. For a chapter spanning multiple spines it reflects only
+  // this spine's portion -- resolving the full span would mean loading every
+  // sibling spine, too much SD I/O for a list that resolves a window at a time.
+  const auto range = tocIdx ? spine.getPageRangeForTocIndex(*tocIdx) : std::nullopt;
+  if (range && range->endPage > range->startPage && spinePage >= range->startPage) {
+    view.chapterPage = spinePage - range->startPage;
+    view.chapterPageCount = range->endPage - range->startPage;
+  } else {
+    view.chapterPage = spinePage;
+    view.chapterPageCount = view.spinePageCount;
+  }
   return view;
 }
 
@@ -154,7 +174,7 @@ void EpubReaderBookmarksActivity::loop() {
               static_cast<unsigned>(windowStart + windowCount));
       return;
     }
-    setResult(PositionResult{view->bookmark.spineIndex, view->resolvedPage});
+    setResult(PositionResult{view->bookmark.spineIndex, view->spinePage});
     finish();
     return;
   }
@@ -232,10 +252,11 @@ void EpubReaderBookmarksActivity::render(RenderLock&&) {
     const auto* view = viewForIndex(target);
     if (!view) return "";
     const std::string title = view->chapterTitle.empty() ? std::string(tr(STR_UNNAMED)) : view->chapterTitle;
-    if (view->chapterPageCount == 0) return title;
-    const float intra = static_cast<float>(view->resolvedPage) / static_cast<float>(view->chapterPageCount);
-    const int percent = static_cast<int>(epub->calculateProgress(view->bookmark.spineIndex, intra) * 100.0f + 0.5f);
-    return std::to_string(percent) + "% - " + std::to_string(view->resolvedPage + 1) + "/" +
+    if (view->spinePageCount == 0) return title;
+    const int percent = static_cast<int>(
+        epub->calculateProgressForPage(view->bookmark.spineIndex, view->spinePage, view->spinePageCount) * 100.0f +
+        0.5f);
+    return std::to_string(percent) + "% - " + std::to_string(view->chapterPage + 1) + "/" +
            std::to_string(view->chapterPageCount) + " - " + title;
   };
   const auto getBookmarkIcon = [isPortrait](int /*index*/) { return isPortrait ? UIIcon::Bookmark : UIIcon::None; };

src/activities/reader/EpubReaderBookmarksActivity.h

@@ -25,14 +25,20 @@ class EpubReaderBookmarksActivity final : public Activity {
   static constexpr uint16_t kMaxVisible = 16;
 
   // Resolved view of one bookmark entry: the on-disk record plus the small
-  // amount of derived state we want for display (chapter title, page within
-  // chapter, percent through book). Computed once per window load via ad-hoc
-  // Section reads against the spine's cache file.
+  // amount of derived state we want for display. Computed once per window load
+  // via a read-only SpineItem load against the spine's cache file.
+  //
+  // Two page coordinates are kept because they answer different questions:
+  // the spine-local pair drives the book-progress percent (Epub::calculate-
+  // ProgressForPage is spine-relative), while the chapter-relative pair drives
+  // the "X/Y" readout so it matches the reader's status bar.
   struct EntryView {
     BookmarkEntry bookmark;
-    uint16_t resolvedPage = 0;      // 0-based page within the chapter
-    uint16_t chapterPageCount = 0;  // total pages in chapter at last cache
-    std::string chapterTitle;       // empty string when no TOC entry maps
+    uint16_t spinePage = 0;         // 0-based page within the spine (percent)
+    uint16_t spinePageCount = 0;    // total pages in the spine (percent)
+    uint16_t chapterPage = 0;       // 0-based page within the chapter (display)
+    uint16_t chapterPageCount = 0;  // total pages in the chapter (display)
+    std::string chapterTitle;       // page-accurate title, empty when no TOC entry maps
     bool valid = false;
   };
 

test/section/SectionTest.cpp

@@ -146,6 +146,53 @@ TEST_F(SectionTest, MultiPageRoundTrip) {
   }
 }
 
+// --- Query-mode page count ------------------------------------------------
+
+TEST_F(SectionTest, LoadCachedPageCountReadsCountWithoutValidatingParams) {
+  Section sec(kPath);
+  Params p;
+  ASSERT_TRUE(sec.openForWrite());
+  writeHeaderFrom(sec, p);
+  std::vector<PageLutEntry> lut;
+  for (uint16_t i = 0; i < 5; i++) {
+    const uint32_t off = sec.writePage(makePage(i));
+    ASSERT_NE(off, 0u);
+    lut.push_back({off, 0, 0});
+  }
+  ASSERT_TRUE(sec.finalize(lut, /*anchors=*/{}));
+
+  // Query-mode load populates pageCount without taking render params and,
+  // unlike loadHeader, leaves the file in place.
+  Section reader(kPath);
+  ASSERT_TRUE(reader.loadCachedPageCount());
+  EXPECT_EQ(reader.getPageCount(), 5u);
+  EXPECT_TRUE(halStorage.exists(kPath));
+}
+
+TEST_F(SectionTest, LoadCachedPageCountRejectsVersionMismatchWithoutRemoving) {
+  Section sec(kPath);
+  Params p;
+  ASSERT_TRUE(sec.openForWrite());
+  writeHeaderFrom(sec, p);
+  ASSERT_TRUE(sec.finalize({}, {}));
+
+  // Tamper the version byte (offset 0). Unlike loadHeader, query-mode load
+  // must reject without deleting the file.
+  std::string fileBytes;
+  {
+    HalFile f;
+    ASSERT_TRUE(halStorage.openFileForRead("T", kPath, f));
+    fileBytes.resize(f.size());
+    f.read(fileBytes.data(), fileBytes.size());
+  }
+  fileBytes[0] = static_cast<char>(Section::FILE_VERSION + 1);
+  test_stubs::seedHalFileContent(kPath, fileBytes);
+
+  Section reader(kPath);
+  EXPECT_FALSE(reader.loadCachedPageCount());
+  EXPECT_TRUE(halStorage.exists(kPath));
+}
+
 // --- Anchor map -----------------------------------------------------------
 
 TEST_F(SectionTest, GetPageForAnchorFindsKnownAndRejectsUnknown) {