From 42625348eb26e708d211c13206c487f56e90316f Mon Sep 17 00:00:00 2001 From: cary-rowen Date: Fri, 7 Aug 2026 10:44:32 +0800 Subject: [PATCH] Fix EPUB spine reading order --- bookworm/document/formats/epub.py | 42 ++++++++++------ tests/assets/bookworm-order-bug-sample.epub | Bin 0 -> 3454 bytes tests/test_epub.py | 52 ++++++++++++++++++++ 3 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 tests/assets/bookworm-order-bug-sample.epub diff --git a/bookworm/document/formats/epub.py b/bookworm/document/formats/epub.py index 4fcb23ba..c4edab33 100644 --- a/bookworm/document/formats/epub.py +++ b/bookworm/document/formats/epub.py @@ -301,7 +301,6 @@ def get_section_at_position(self, pos): @cached_property def epub_html_items(self) -> tuple[str]: - items = () if html_items := tuple(self.epub.get_items_of_type(ebooklib.ITEM_DOCUMENT)): items = html_items else: @@ -311,19 +310,34 @@ def epub_html_items(self) -> tuple[str]: self.epub.items, ) ) - # Previously the chapters order wouldn't respect the table of content - # In most cases this is not an issue - # However this poses a problem when the chapters do not follow a conventional numeric scheme but rather use something like roman numbers - # As reported in issue 243 - # We will now sort the items obtained earlier based on the position that the chapter itself occupies in the TOC - spine = [x[0].split("/")[-1] for x in self.epub.spine] - log.debug(spine) - try: - items = sorted(items, key=lambda x: spine.index(x.id)) - except ValueError: - log.warn( - "Failed to order chapters based on the table of content. Order may be inconsistent" + items_by_id = {item.id: item for item in items} + ordered_items = [] + ordered_item_ids = set() + unresolved_item_ids = [] + for item_id, _linear in self.epub.spine: + item = items_by_id.get(item_id) + if item is None and item_id: + item = items_by_id.get(item_id.rsplit("/", 1)[-1]) + if item is None: + unresolved_item_ids.append(item_id) + continue + ordered_items.append(item) + ordered_item_ids.add(item.id) + if ordered_items: + if unresolved_item_ids: + log.warning( + "Could not resolve some EPUB spine documents as HTML: %s", + unresolved_item_ids, + ) + ordered_items.extend( + item + for item in items + if item.id not in ordered_item_ids and not isinstance(item, ebooklib.epub.EpubNav) ) + return tuple(ordered_items) + # Preserve support for malformed EPUBs whose spine has no usable HTML entries. + if self.epub.spine: + log.warning("Could not resolve any EPUB spine documents as HTML; using manifest order") return items def get_epub_html_item_by_href(self, href): @@ -456,7 +470,7 @@ def add_toc_entry(self, entries, parent): @cached_property def html_content(self): - cache_key = f"preprocessed-html-v1:{self.uri.to_uri_string()}" + cache_key = f"preprocessed-html-v2:{self.uri.to_uri_string()}" document_path = self.get_file_system_path() try: with Cache( diff --git a/tests/assets/bookworm-order-bug-sample.epub b/tests/assets/bookworm-order-bug-sample.epub new file mode 100644 index 0000000000000000000000000000000000000000..11f1801e15132b040ffa3493a06aed27bcd32713 GIT binary patch literal 3454 zcmcJScT`i^8pRVr4HzQD#Bu0NkSY#JQ7M8_C4@2qA_Rz1k`NIPB(y|{@<5al4G2KFnvw=>WXrH>EIueHZ8%oZ1e) z!%-}A#um=7w{$ksvUaf2@eRg>q5^Q}5bX#o#`8iQxLy}}VCr+?J-2{@-6B|lx>BMS zv=h5YwI= zuCmu7DXG8gu(Is&b#B=XIV6+SqNH-)J5VOR9GWjnMFz_>8DdLD)ka!#J4@roh6y#y zsw>AeAbW*Vvb)iXf*4bB|D6tXxzwZ-=*hO)nYbDZf0h5AO-Safg~u{r{qv<33Up9` z004m>0NBiVzQdO0j>xYMhsK3!2jh=rr!#RRU8v>U0+`tgwsWEtL7^&3nu=i=*(4z! zf&GDYvaw!lVJQ8!goG&kU6jc~+08}4`KB;k6}O#Dw)-Co`_bW-Ba?PyMS4YKYbx4z z>bC(6?`!%JN?gfjjxgM=>1Qvr{^=*pf9=2rG4aZ|pWLihknvBq9%AZ4nobxtwD9E} zio22I-Vi8LQXQEU>7_p zMbB0Xj!39)9CyciGKk$|7$LH5+E}>$z2SlD<-h1%I_QA)cb*or>8-00=(xK74Q9Ew zC6TWnEqE2EZWc_`g(pm{ zB;GW(jW;*2ry*XbvRP;|+V5u&d@eKZrV29-7+$C%O5a zt8tPa3TEJQA$ih))b{ZKJwXMd(em4cR@ZU;S`P+j2{d2wY@3$5lwK+cbbZCtU>odw zbTeU6v5S1iRXcV5Vq?9f6Aa~do4DD3CbGgl20COD-eW+h&_W}GufUcT+LzR>U$p*Q zI=}nmvYQl2HZtqf6*xS27Ge?qDSUq|iut5}%Oex%-GY5SjI4$@kMXc=RxLZF`fk5! zLp@G@Xi~!YWt6EOMCI?3_&Q-Q>?9m-TZ?NLzGnx<(YVvJW~&XwDsMdM%jq+>8jqFf zlZ{U_tJv&QrbfTB6-zEM2BL^Xi~ptg=w? zfU07`sP4-rPe$j`9Xg%uTn5Y!{(-9~7&)sT(i0RL7<18eVW^eTI{^xdQT72nPNvv= z^!H^nrd`^{0wi@R_<{_nehQ z7QPG;G(og^zmudvsu7zk4I5yl159p*DU2Fyxw`13pHXpKgMrrgqK*uUy2*0C$(qtX zk(ntOwRwNsYXaYTs*LBO?=^m;>h8V)RbQVPmn`A5JmlP@v;mC7`ss{y_VQ9rvil7S zX>});^YKvx@rY6AQfHL79NcOwAyaxG0SGDT3?B^)Pa@AM^)fSbFX@_-i;t;VJsKip z-XzdNU`9tV6&^8}TPe9Ri@t19S&!V7@;;1gC0i+N2LR<5pq`(+w^glx@C0z9-x^Bz zEvKsK1KWhE)u>|alKMcZzzy`Pr#{1ope34U{_1sO>C@}K0K2!5`cF%g1-*jhgI$ZF zy1l*hOF|YnRCjTy7Uovx*2XK6M?hEKTp|dUH7t8WH<@SQOCYurMAeg*+E)I?isoW9 z;$ZdOcCzj>gWB4wpYdvE#+P(j^#-(FvrIR@YG(Fd zUJ)W_yTF@wzqf=`6^&`5$f_`R>;7aO0R9j$8LGBH76tx57AXovd2t)`MKP zdeo)K&pUD5%H+EB7wbahD0M4_;JxZ&N5kezw6=piedTmy(_ZpBGsK8v*6nvaz0Pa= zdVkV|6y4U`I$T<;)IM*d31Sdg#A(6ApK~tYPpC~Rv_se=vt7d2`W7S2AlNSIzrAig(&C%Kh^D5s}i9v*aDZLN=MxUCfv8za|mvC#6mSOHzEn%nf;M*pu(hG#8S z4*W%E3^f(&I1zXbQa7$pfX7*C$} zhL#bv2mEDpg2q%bnPPM?mY8fwB|?)xm537xdl}VzmWY@Oy2)Cx>n^hE-jAkFy>Ncl z>ii{I)e3piM`uu|zwof5Es#&@YhdQ23BZpLn)@yH;D(O1K)t5gI6iada*sHm5$C*d znC86)Z$$ligRl4aiL>?(ft6Rzn~c`vgM6#UKA%)3*siBmX%`Pstale cached content") + cache_utils.set_document_modified_time(old_cache_key, epub_path, cache) + + document = EpubDocument(uri) + document.read() + + assert [item.id for item in document.epub_html_items] == [ + item_id for item_id, _linear in document.epub.spine + ] + assert document.get_content().startswith("Chapter 1") + assert "Contents" not in document.get_content() + assert "stale cached content" not in document.get_content() + + +def test_epub_html_items_tolerate_malformed_spine(): + book = epub.EpubBook() + first = epub.EpubHtml(uid="first", file_name="first.xhtml") + second = epub.EpubHtml(uid="second", file_name="second.xhtml") + notes = epub.EpubHtml(uid="notes", file_name="notes.xhtml") + nav = epub.EpubNav() + for item in (first, second, notes, nav): + book.add_item(item) + book.spine = [("OPS/second", "yes"), ("missing", "yes"), ("first", "yes")] + document = EpubDocument(None) + document.epub = book + + assert [item.id for item in document.epub_html_items] == [ + "second", + "first", + "notes", + ] + + book.spine = [("missing", "yes")] + fallback_document = EpubDocument(None) + fallback_document.epub = book + assert [item.id for item in fallback_document.epub_html_items] == [ + "first", + "second", + "notes", + "nav", + ] + + def test_modified_epub_modifies_cache(asset): book = temp_book() epub.write_epub(asset("test.epub"), book, {})