Skip to content

kfx: resolve nav targets that never appear in chapter markup - #30

Open
Imaclean74 wants to merge 2 commits into
zacharydenton:masterfrom
Imaclean74:fix/kfx-nav-targets
Open

Imaclean74 wants to merge 2 commits into
zacharydenton:masterfrom
Imaclean74:fix/kfx-nav-targets

Conversation

@Imaclean74

Copy link
Copy Markdown
Contributor

When a KFX book is exported to EPUB, some TOC and landmark entries end up in toc.ncx / nav.xhtml as bare fragments that can never resolve, so those entries don't navigate anywhere. There are two separate causes:

1. #id:offset positions (e.g. #927:16). Kindle nav entries target a {id, offset} position, and the KFX importer writes these as #id:offset. KfxImporter::resolve_href already resolves them to the element, which is correct since the offset is inside that element. However, normalize_book only remapped importer-specific hrefs that appear in chapter markup, and TOC/landmark hrefs never do. They're now remapped from their resolved targets too, giving chapter_3.xhtml#927.

2. Section page_template ids (e.g. #1046). Kindle-produced books point the cover nav entry at the cover section's page-template container id, which no element carries. index_section_storylines now records template id → section, and resolve_href falls back to that section's chapter (chapter_0.xhtml).

In a sample of 83 KFX books, 9 had broken #id:offset entries (141 in total, 110 in one book) and 14 had a broken cover entry. After this change, all exported nav targets resolve.

Reproducer: tests/fixtures/nav_targets_stripped.kfx.gz (136 KB) is one of those books with its content removed. Text is replaced by same-length filler so position offsets stay valid, images by an 8×8 JPEG, and metadata is anonymised. Structure and navigation are untouched. On master, 85 of its 99 exported NCX/nav targets are bare fragments; with this PR all resolve.

Tests (all fail without the fix):

  • export::normalize::tests::toc_and_landmark_hrefs_use_resolved_targets: a minimal importer with position-style hrefs. Book::from_backend is now pub(crate) so the test can construct the book.
  • tests/kfx_nav_targets.rs:
    • kfx_section_template_id_resolves_to_its_chapter: an EPUB→KFX round trip asserting that every exported page_template id (bare, #id and #id:offset forms) resolves to its section's chapter.
    • kindle_book_toc_targets_resolve_in_epub: every NCX/nav target exported from the fixture points at an existing file and id.

Checked with Rust 1.99.0: cargo fmt --check, cargo test and cargo doc pass. Clippy is clean once #29 (the 1.99 lint fix) is applied; until then it fails on master's vacuum.rs, not on this change.

KFX TOC and landmark hrefs leaked into exported EPUBs as bare fragments
(toc.ncx/nav.xhtml "#927:16", "#1046") that can never resolve:

- "#id:offset" positions: KfxImporter::resolve_href already resolves
  these, but normalize_book only bridged importer-specific hrefs found in
  chapter markup. TOC entries and landmarks are now remapped from their
  resolved targets as well.
- Section page_template ids: Kindle points the cover nav entry at the
  cover section's template container id, which no IR node carries.
  index_section_storylines records template id -> section and
  resolve_href falls back to that section's chapter.

Tests: a normalize unit test with a position-style importer
(Book::from_backend made pub(crate) for it), and an EPUB->KFX round trip
asserting every exported page_template id resolves to its chapter. Both
fail without the fix.
nav_targets_stripped.kfx.gz (136 KB) is a real Kindle-produced KFX book
with its content removed: every text character in content, ruby and inline
storyline text and in nav labels replaced by same-length filler (so
position offsets stay valid), anchor URIs replaced, identifying metadata
anonymized and all images replaced by an 8x8 JPEG. Structure, styles,
navigation and position maps are untouched.

Its TOC has 84 #id:offset targets and a cover entry pointing at a section
page_template id. On master, 85 of its 99 exported NCX/nav targets are
bare fragments (#915, #917:2, ...); the new test asserts every one points
at an existing file and id.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant