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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
c5e861d71c
commit
213972badc
@@ -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<std::string> 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> 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();
|
||||
|
||||
|
||||
@@ -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<uint16_t>(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<uint16_t>(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<uint16_t>(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];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -79,7 +79,8 @@ class ChapterHtmlSlimParser {
|
||||
// Anchor-to-page mapping: tracks which page each HTML id attribute lands on
|
||||
int completedPageCount = 0;
|
||||
std::vector<std::pair<std::string, uint16_t>> 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<std::string> 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<void(std::unique_ptr<Page>, uint16_t, uint16_t)>& completePageFn,
|
||||
const bool embeddedStyle, const std::string& contentBase,
|
||||
const std::string& imageBasePath, const uint8_t imageRendering = 0,
|
||||
std::vector<std::string> tocAnchors = {},
|
||||
const std::function<void()>& 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();
|
||||
|
||||
@@ -25,6 +25,7 @@ struct MenuResult {
|
||||
|
||||
struct ChapterResult {
|
||||
int spineIndex = 0;
|
||||
std::string anchor;
|
||||
};
|
||||
|
||||
struct PercentResult {
|
||||
|
||||
@@ -417,10 +417,18 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
|
||||
startActivityForResult(
|
||||
std::make_unique<EpubReaderChapterSelectionActivity>(renderer, mappedInput, epub, path, spineIdx),
|
||||
[this](const ActivityResult& result) {
|
||||
if (!result.isCancelled && currentSpineIndex != std::get<ChapterResult>(result.data).spineIndex) {
|
||||
if (!result.isCancelled) {
|
||||
const auto& chapterResult = std::get<ChapterResult>(result.data);
|
||||
RenderLock lock(*this);
|
||||
currentSpineIndex = std::get<ChapterResult>(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();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
Reference in New Issue
Block a user