diff --git a/docs/contributing/koreader-sync-xpath-mapping.md b/docs/contributing/koreader-sync-xpath-mapping.md index 9df844c8..3e80ebaf 100644 --- a/docs/contributing/koreader-sync-xpath-mapping.md +++ b/docs/contributing/koreader-sync-xpath-mapping.md @@ -6,7 +6,7 @@ This note documents how CrossPoint maps reading positions to and from KOReader s CrossPoint internally stores position as: -- `spineIndex` (chapter index) +- `spineIndex` (chapter index, 0-based) - `pageNumber` + `totalPages` KOReader sync payload stores: @@ -16,6 +16,19 @@ KOReader sync payload stores: A direct 1:1 mapping is not guaranteed because page layout differs between engines/devices. +## DocFragment Index Convention + +KOReader uses **1-based** XPath predicates throughout, following standard XPath conventions. +The first EPUB spine item is `DocFragment[1]`, the second is `DocFragment[2]`, and so on. + +CrossPoint stores spine items as 0-based indices internally. The conversion is: + +- **Generating XPath (to KOReader):** `DocFragment[spineIndex + 1]` +- **Parsing XPath (from KOReader):** `spineIndex = DocFragment[N] - 1` + +Reference: [koreader/koreader#11585](https://github.com/koreader/koreader/issues/11585) confirms this +via a KOReader contributor mapping spine items to DocFragment numbers. + ## Current Strategy ### CrossPoint -> KOReader @@ -25,13 +38,13 @@ Implemented in `ProgressMapper::toKOReader`. 1. Compute overall `percentage` from chapter/page. 2. Attempt to compute a real element-level XPath via `ChapterXPathIndexer::findXPathForProgress`. 3. If XPath extraction fails, fallback to synthetic chapter path: - - `/body/DocFragment[N]/body` + - `/body/DocFragment[spineIndex + 1]/body` ### KOReader -> CrossPoint Implemented in `ProgressMapper::toCrossPoint`. -1. Attempt to parse `DocFragment[N]` from incoming XPath. +1. Attempt to parse `DocFragment[N]` from incoming XPath; convert N to 0-based `spineIndex = N - 1`. 2. If valid, attempt XPath-to-offset mapping via `ChapterXPathIndexer::findProgressForXPath`. 3. Convert resolved intra-spine progress to page estimate. 4. If XPath path is invalid/unresolvable, fallback to percentage-based chapter/page estimation. @@ -44,12 +57,18 @@ Source-of-truth note: XPath anchors are built from the original EPUB spine XHTML - anchor: `` - `textOffset` counts non-whitespace bytes +- When multiple anchors exist for the same path, the one with the **smallest** textOffset is used + (start of element), not the latest periodic anchor. + +Forward lookup (CrossPoint → XPath): uses `upper_bound` to find the last anchor at or before the +target text offset, ensuring the returned XPath corresponds to the element the user is currently +inside rather than the next element. Matching for reverse lookup: -1. exact path match -2. index-insensitive path match (`div[2]` vs `div[3]` tolerated) -3. ancestor fallback +1. exact path match — reported as `exact=yes` +2. index-insensitive path match (`div[2]` vs `div[3]` tolerated) — reported as `exact=no` +3. ancestor fallback — reported as `exact=no` If no match is found, caller must fallback to percentage. @@ -76,4 +95,5 @@ The implementation intentionally avoids full DOM storage. - `xpath` when XPath mapping path was used - `percentage` when fallback path was used -It also logs exactness (`exact=yes/no`) for XPath matches. +It also logs exactness (`exact=yes/no`) for XPath matches. Note that `exact=yes` is only set for +a full path match with correct indices; index-insensitive and ancestor matches always log `exact=no`. diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp index eee2e877..984f8d00 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -42,7 +42,7 @@ struct ParserState { std::vector> siblingCounters; std::vector anchors; - std::string baseXPath() const { return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body"; } + std::string baseXPath() const { return "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body"; } // Canonicalize incoming KOReader XPath before matching: // - remove all whitespace @@ -65,8 +65,8 @@ struct ParserState { } const std::string textSuffix = "/text()"; - size_t textPos = out.find(textSuffix); - if (textPos != std::string::npos) { + const size_t textPos = out.rfind(textSuffix); + if (textPos != std::string::npos && textPos + textSuffix.size() == out.size()) { out.erase(textPos); } @@ -132,7 +132,7 @@ struct ParserState { const std::string anchorPath = ignoreIndices ? removeIndices(anchor.xpath) : anchor.xpath; if (anchorPath == probe) { const int depth = pathDepth(anchorPath); - if (!found || depth > bestDepth || (depth == bestDepth && anchor.textOffset > bestOffset)) { + if (!found || depth > bestDepth || (depth == bestDepth && anchor.textOffset < bestOffset)) { found = true; bestDepth = depth; bestOffset = anchor.textOffset; @@ -282,11 +282,12 @@ struct ParserState { const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress)); const size_t target = static_cast(clampedProgress * static_cast(totalTextBytes)); - auto it = std::lower_bound(anchors.begin(), anchors.end(), target, - [](const XPathAnchor& anchor, const size_t value) { return anchor.textOffset < value; }); - - if (it == anchors.end()) { - return anchors.back().xpath; + // upper_bound returns the first anchor strictly after target; step back to get + // the last anchor at-or-before target (the element the user is currently inside). + auto it = std::upper_bound(anchors.begin(), anchors.end(), target, + [](const size_t value, const XPathAnchor& anchor) { return value < anchor.textOffset; }); + if (it != anchors.begin()) { + --it; } return it->xpath; } @@ -308,6 +309,7 @@ struct ParserState { if (!matched) { matched = pickBestAnchorByPath(normalized, true, matchedOffset, exact); + if (matched) exact = false; } if (!matched) { @@ -476,10 +478,11 @@ bool ChapterXPathIndexer::tryExtractSpineIndexFromXPath(const std::string& xpath const std::string value = normalized.substr(start, end - start); const long parsed = std::strtol(value.c_str(), nullptr, 10); - if (parsed < 0 || parsed > std::numeric_limits::max()) { + // KOReader uses 1-based DocFragment indices; convert to 0-based spine index. + if (parsed < 1 || parsed > std::numeric_limits::max()) { return false; } - outSpineIndex = static_cast(parsed); + outSpineIndex = static_cast(parsed) - 1; return true; } diff --git a/lib/KOReaderSync/ProgressMapper.cpp b/lib/KOReaderSync/ProgressMapper.cpp index f5e58171..b0199bf6 100644 --- a/lib/KOReaderSync/ProgressMapper.cpp +++ b/lib/KOReaderSync/ProgressMapper.cpp @@ -146,6 +146,6 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu std::string ProgressMapper::generateXPath(int spineIndex, int pageNumber, int totalPages) { // Fallback path when element-level XPath extraction is unavailable. - // Uses 0-based DocFragment indices for KOReader compatibility. - return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body"; + // KOReader uses 1-based XPath predicates; spineIndex is 0-based internally. + return "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body"; }