From 213972badc4dc5b44cdb199e74186007773d9d69 Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Tue, 26 May 2026 20:14:32 +0300 Subject: [PATCH] fix: navigate to TOC anchor when selecting sub-chapters (#1981) Chapter selection previously only used the spine index, causing navigation to always land on page 0 of the spine item. Sub-chapters that share a spine file but differ by anchor (e.g. `chapter.xhtml#sec2`) were silently ignored. Now the TOC anchor is passed through `ChapterResult` and applied via the existing `pendingAnchor` mechanism. ## Summary * This PR implements navigation to sub-chapters which didn't work correctly previously. If a sub-chapter of the current top-level chapter was selected, nothing happened. If a sub-chapter of another top-level chapter was selected, reader switched to the beginning of the top-level chapter. * In addition, chapters now always start from a new page. This fixes anchor to page calculation for the cases when the actual chapter content doesn't fit on the page where the corresponding ToC anchor was found. ## Additional Context * I might misuse `pendingAnchor` here which was previously used for footnote navigation, please double check. I'm open to suggestions for improvements. * Note that the chapter selected by default when `EpubReaderChapterSelectionActivity` opens is still wrong. I'm going to fix this separately. This PR addresses only navigation to the selected chapter. * I tested this PR on my X4 and verified that navigation to a different sub-chapter works correctly, both inside and outside the current spine. * Some of the changes were borrowed from https://github.com/crosspoint-reader/crosspoint-reader/pull/1455 --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**PARTIALLY**_ --------- Co-authored-by: Claude Sonnet 4.6 --- lib/Epub/Epub/Section.cpp | 15 +++++++- .../Epub/parsers/ChapterHtmlSlimParser.cpp | 35 +++++++++++++------ lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h | 8 +++-- src/activities/ActivityResult.h | 1 + src/activities/reader/EpubReaderActivity.cpp | 12 +++++-- .../EpubReaderChapterSelectionActivity.cpp | 6 ++-- 6 files changed, 59 insertions(+), 18 deletions(-) diff --git a/lib/Epub/Epub/Section.cpp b/lib/Epub/Epub/Section.cpp index 5777a7d2..e8666dad 100644 --- a/lib/Epub/Epub/Section.cpp +++ b/lib/Epub/Epub/Section.cpp @@ -221,13 +221,26 @@ bool Section::createSectionFile(const int fontId, const float lineCompression, c } } + // Collect TOC anchors for this spine so the parser can insert page breaks at chapter boundaries + std::vector tocAnchors; + const int startTocIndex = epub->getTocIndexForSpineIndex(spineIndex); + if (startTocIndex >= 0) { + for (int i = startTocIndex; i < epub->getTocItemsCount(); i++) { + auto entry = epub->getTocItem(i); + if (entry.spineIndex != spineIndex) break; + if (!entry.anchor.empty()) { + tocAnchors.push_back(std::move(entry.anchor)); + } + } + } + ChapterHtmlSlimParser visitor( epub, tmpHtmlPath, renderer, fontId, lineCompression, extraParagraphSpacing, paragraphAlignment, viewportWidth, viewportHeight, hyphenationEnabled, focusReadingEnabled, [this, &lut](std::unique_ptr page, const uint16_t paragraphIndex, const uint16_t listItemIndex) { lut.push_back({this->onPageComplete(std::move(page)), paragraphIndex, listItemIndex}); }, - embeddedStyle, contentBase, imageBasePath, imageRendering, popupFn, cssParser); + embeddedStyle, contentBase, imageBasePath, imageRendering, std::move(tocAnchors), popupFn, cssParser); Hyphenator::setPreferredLanguage(epub->getLanguage()); success = visitor.parseAndBuildPages(); diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index 62a5409d..1c763cba 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -99,6 +99,25 @@ void ChapterHtmlSlimParser::updateEffectiveInlineStyle() { } } +void ChapterHtmlSlimParser::flushPendingAnchor() { + if (pendingAnchorId.empty()) return; + + // If the pending anchor is a TOC chapter boundary, force a page break after the previous + // block is flushed so the chapter starts on a fresh page. + if (std::find(tocAnchors.begin(), tocAnchors.end(), pendingAnchorId) != tocAnchors.end()) { + if (currentPage && !currentPage->elements.empty()) { + completePageFn(std::move(currentPage), xpathParagraphIndex, xpathListItemIndex); + completedPageCount++; + currentPage.reset(new Page()); + currentPageNextY = 0; + } + } + + // Record deferred anchor after previous block is flushed (and any TOC page break) + anchorData.push_back({std::move(pendingAnchorId), static_cast(completedPageCount)}); + pendingAnchorId.clear(); +} + // flush the contents of partWordBuffer to currentTextBlock void ChapterHtmlSlimParser::flushPartWordBuffer() { // Determine font style from depth-based tracking and CSS effective style @@ -144,20 +163,15 @@ void ChapterHtmlSlimParser::startNewTextBlock(const BlockStyle& blockStyle) { const auto style = currentTextBlock->getBlockStyle(); currentTextBlock->setBlockStyle(style.getCombinedBlockStyle(blockStyle, BlockStyle::CombineAxis::Vertical)); - if (!pendingAnchorId.empty()) { - anchorData.push_back({std::move(pendingAnchorId), static_cast(completedPageCount)}); - pendingAnchorId.clear(); - } + flushPendingAnchor(); return; } makePages(); } - // Record deferred anchor after previous block is flushed - if (!pendingAnchorId.empty()) { - anchorData.push_back({std::move(pendingAnchorId), static_cast(completedPageCount)}); - pendingAnchorId.clear(); - } + // If the pending anchor is a TOC chapter boundary, force a page break after the previous + // block is flushed so the chapter starts on a fresh page. + flushPendingAnchor(); currentTextBlock.reset(new ParsedText(extraParagraphSpacing, hyphenationEnabled, focusReadingEnabled, blockStyle)); wordsExtractedInBlock = 0; } @@ -250,7 +264,8 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* } else if (strcmp(atts[i], "style") == 0) { styleAttr = atts[i + 1]; } else if (strcmp(atts[i], "id") == 0) { - // Defer recording until startNewTextBlock, after previous block is flushed to pages + // Defer both anchor recording and TOC page breaks until startNewTextBlock, + // after the previous block is flushed to pages via makePages(). self->pendingAnchorId = atts[i + 1]; } } diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h index f11c6256..2c050b33 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h @@ -79,7 +79,8 @@ class ChapterHtmlSlimParser { // Anchor-to-page mapping: tracks which page each HTML id attribute lands on int completedPageCount = 0; std::vector> anchorData; - std::string pendingAnchorId; // deferred until after previous text block is flushed + std::string pendingAnchorId; // deferred until after previous text block is flushed + std::vector tocAnchors; // the list of anchors that are TOC chapter boundaries uint16_t xpathParagraphIndex = 0; uint16_t xpathListItemIndex = 0; @@ -93,6 +94,7 @@ class ChapterHtmlSlimParser { void updateEffectiveInlineStyle(); void startNewTextBlock(const BlockStyle& blockStyle); + void flushPendingAnchor(); void flushPartWordBuffer(); void makePages(); void emitHorizontalRule(const BlockStyle& blockStyle); @@ -111,6 +113,7 @@ class ChapterHtmlSlimParser { const std::function, uint16_t, uint16_t)>& completePageFn, const bool embeddedStyle, const std::string& contentBase, const std::string& imageBasePath, const uint8_t imageRendering = 0, + std::vector tocAnchors = {}, const std::function& popupFn = nullptr, const CssParser* cssParser = nullptr) : epub(epub), @@ -130,7 +133,8 @@ class ChapterHtmlSlimParser { embeddedStyle(embeddedStyle), imageRendering(imageRendering), contentBase(contentBase), - imageBasePath(imageBasePath) {} + imageBasePath(imageBasePath), + tocAnchors(std::move(tocAnchors)) {} ~ChapterHtmlSlimParser() = default; bool parseAndBuildPages(); diff --git a/src/activities/ActivityResult.h b/src/activities/ActivityResult.h index d25df090..5d8b354f 100644 --- a/src/activities/ActivityResult.h +++ b/src/activities/ActivityResult.h @@ -25,6 +25,7 @@ struct MenuResult { struct ChapterResult { int spineIndex = 0; + std::string anchor; }; struct PercentResult { diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index d36afc59..2157c6fb 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -417,10 +417,18 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction startActivityForResult( std::make_unique(renderer, mappedInput, epub, path, spineIdx), [this](const ActivityResult& result) { - if (!result.isCancelled && currentSpineIndex != std::get(result.data).spineIndex) { + if (!result.isCancelled) { + const auto& chapterResult = std::get(result.data); RenderLock lock(*this); - currentSpineIndex = std::get(result.data).spineIndex; + + currentSpineIndex = chapterResult.spineIndex; + + // If anchor is not empty, it will be used later to calculate the page number. + pendingAnchor = chapterResult.anchor; + + // Otherwise page 0 will be used. nextPageNumber = 0; + section.reset(); } }); diff --git a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp index 0f085dce..6306346c 100644 --- a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp +++ b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp @@ -32,14 +32,14 @@ void EpubReaderChapterSelectionActivity::loop() { const int totalItems = getTotalItems(); if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { - const auto newSpineIndex = epub->getSpineIndexForTocIndex(selectorIndex); - if (newSpineIndex == -1) { + const auto tocItem = epub->getTocItem(selectorIndex); + if (tocItem.spineIndex == -1) { ActivityResult result; result.isCancelled = true; setResult(std::move(result)); finish(); } else { - setResult(ChapterResult{newSpineIndex}); + setResult(ChapterResult{tocItem.spineIndex, tocItem.anchor}); finish(); } } else if (mappedInput.wasReleased(MappedInputManager::Button::Back)) {