From 94843902acb3a134f40965015bab1c933af8dd79 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 31 Mar 2026 23:56:23 +0800 Subject: [PATCH] fix(annotations): fix all annotations grouped under last chapter for fragment-href TOCs, closes #3688 (#3706) When TOC entries use fragment-suffixed hrefs (e.g. ch01.xhtml#ch01), the sectionsMap lookup matched subitems with cfi: undefined instead of parent sections with valid CFIs. This caused findTocItemBS to map every annotation to the last TOC entry. Now skip subitems lacking a CFI and fall back to the base section. Co-authored-by: Claude Opus 4.6 (1M context) --- .../__tests__/fixtures/data/repro-3688.epub | Bin 0 -> 2520 bytes .../__tests__/utils/toc-cfi-mapping.test.ts | 107 ++++++++++++++++++ apps/readest-app/src/utils/toc.ts | 4 +- 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 apps/readest-app/src/__tests__/fixtures/data/repro-3688.epub create mode 100644 apps/readest-app/src/__tests__/utils/toc-cfi-mapping.test.ts diff --git a/apps/readest-app/src/__tests__/fixtures/data/repro-3688.epub b/apps/readest-app/src/__tests__/fixtures/data/repro-3688.epub new file mode 100644 index 0000000000000000000000000000000000000000..bc44d41b4b564be74fdce966fae18cc82bb46be4 GIT binary patch literal 2520 zcmZ`*2{@G78y{w5$uchKBGFznuzM3n`kW2r1M3e{+@bzmI#q=RDu}p7Z|B`<>r;-}iUy5MT%&h}94cVXl52 zca=qfF{|?coq!{FkuUjsdHDO|aaa#B&X1_(#hzE*Xp8G{*Lfi4}& zjxLVOM{})#&(%G{)JbMJePSRlE4(6Mrt3=|F!~b?t`BM8oYl}WROJ)N?6{v-@ZyQWKC*tPhshb7BuezpcurHZ_|CL&4+ole^ zwqex%GWStLjEub08E3cxf8vqGCmBEGdAXCmu8$F7qCMYw&TNDY`_&kf?cc&sGF+9G z&MSeYEML^s9>`-ZGgHR`B?E(kCpO#}YM5ZRl?MJU`-3hXnarv*#UhyDZ=&9(Pauz> zVuWMb;X&~#YGd@iS#j{ExU_mB#Y-HzOWB`lXLX#X;)n3kPr|03UZOvq9^=`pfKz_C zKB7Kd;W-}uU?iGD%5<7^Q-lkXn1hHg-iH##78N$VF~XM$jhHAZfu(%FQIZ5+)u<8S zjw8_l-4?|d4Nj!Fo`{dZ>=NPgYPZY&OUysq*iIb}Zs9yaU4_@Ucm4>=uk5;?({cLu32+;$-{b!8X!d3 zE#DpiiktA@%mw~g$&PkkH%$fZoWTz2;?dVn4-~8}ZT8czTM;{z{hN~_3J>x46pW6} zQ0`Y_H}&+4BYS>#$vrr4^iG#NaPpx!^Y19z&LanaAjm}xQw9(@=zm6##*YXx42V+a zzqYZ(_v|w1k!q1o7ghIn{Xtp*2bTfIL}hx`r_oVJ#`Ig;5_F1DJ5hrBGhDh_N4BR* z1*~00uYQYI|5f^y`*`^aO3B7toy+9a?^3>TLc;-K=Poe`AtrI5O;# zjrcr6N`E2>UaN4n#PwL87olx*VRI$Vm>oSxfSgK@yowt@j4l`i5@x3tP+`1?WK}YOsJp;Hs|O?WYRIp>bIN?_HU=Jg#|BLn?G5AF zGHsF?)2n*oJ&Z%>DluYHd$;QIRL)V-_^V6 z=xvdLSsSZ%2}5|%ub9bpSyfGU8g}1*Q?HYLdu7!%J0^y>l7?^oL&NUC7);D|RPa%L z1oNpK=1!o}&oxG-nvluHkoH`ZU@GJIbF+_;5(6&>o$r0UshbgICTrldV(e6BS`=os zDH_zW&>;2`Rp`7R(dV^5{Y>3fpUlIW`5R~I9@$!a6QVB5D+uk%WE5}~H%^HKCQJG}qKj-qnuWpB z&EI<4(g zUYchToO!gD!}`6h^}WR535)c0?gwdSbB3F8J4QZVQ{JjP zx3b?3!NGYL{O?i?aQk;Fj9|Tf2={K(UNXInfr998ePFmtv{}ynUA9>;;7~z@5gxl? z|0m+RUGF7~tTkBjM~9mNOZ|{Zw%@&kl7-mz>$8Jl%jUMn?Z4hjMc8Nx5RKa&0rYoG z*^S<-#In()Kwa5Ee=oSZ(R)['CSS'] = { + escape: (s: string) => s.replace(/([^\w-])/g, '\\$1'), + }; +} + +// Register a stub paginator custom element so View.open() doesn't fail in jsdom +if (!customElements.get('foliate-paginator')) { + customElements.define( + 'foliate-paginator', + class extends HTMLElement { + override setAttribute() {} + override addEventListener() {} + open() {} + }, + ); +} + +// Mock the paginator module import so View doesn't try to load the real one +vi.mock('foliate-js/paginator.js', () => ({})); + +/** + * Regression test for https://github.com/readest/readest/issues/3688 + * + * When TOC entries use fragment-suffixed hrefs (e.g. ch01.xhtml#ch01), + * all annotations were incorrectly grouped under the last chapter. + * This happened because section subitems created during EPUB init had + * cfi: undefined, and the sectionsMap lookup found these subitems + * instead of the parent sections with valid CFIs. + */ +describe('TOC-to-CFI mapping with fragment hrefs (#3688)', () => { + let book: BookDoc; + + beforeAll(async () => { + const epubPath = resolve(__dirname, '../fixtures/data/repro-3688.epub'); + const buffer = readFileSync(epubPath); + const file = new File([buffer], 'repro-3688.epub', { + type: 'application/epub+zip', + }); + const loader = new DocumentLoader(file); + const result = await loader.open(); + book = result.book; + await updateToc(book, false, 'none'); + }, 30000); + + it('should assign valid CFIs to all TOC items', () => { + const toc = book.toc ?? []; + expect(toc.length).toBeGreaterThan(0); + + const collectItems = (items: TOCItem[]): TOCItem[] => + items.flatMap((item) => [item, ...(item.subitems ? collectItems(item.subitems) : [])]); + + const allItems = collectItems(toc); + const itemsWithHref = allItems.filter((item) => item.href); + expect(itemsWithHref.length).toBeGreaterThan(1); + + // Every TOC item with an href should have a valid (non-empty) CFI + for (const item of itemsWithHref) { + expect( + item.cfi, + `TOC item "${item.label}" (href: ${item.href}) should have a valid CFI`, + ).toBeTruthy(); + } + }); + + it('should map CFIs from different sections to different TOC items', () => { + const toc = book.toc ?? []; + const sections = book.sections ?? []; + expect(sections.length).toBeGreaterThan(1); + + const linearSections = sections.filter((s) => s.linear !== 'no' && s.cfi); + expect(linearSections.length).toBeGreaterThan(2); + + const firstSection = linearSections[0]!; + const lastSection = linearSections[linearSections.length - 1]!; + + const firstTocItem = findTocItemBS(toc, firstSection.cfi); + const lastTocItem = findTocItemBS(toc, lastSection.cfi); + + expect(firstTocItem).not.toBeNull(); + expect(lastTocItem).not.toBeNull(); + + // Annotations from different sections must NOT all map to the same chapter + expect(firstTocItem!.label).toBe('Chapter One'); + expect(lastTocItem!.label).toBe('Chapter Three'); + }); + + it('should map a mid-book CFI to a mid-book TOC item, not the last one', () => { + const toc = book.toc ?? []; + const sections = book.sections ?? []; + const linearSections = sections.filter((s) => s.linear !== 'no' && s.cfi); + + const midSection = linearSections[1]!; + const midTocItem = findTocItemBS(toc, midSection.cfi); + + expect(midTocItem).not.toBeNull(); + expect(midTocItem!.label).toBe('Chapter Two'); + }); +}); diff --git a/apps/readest-app/src/utils/toc.ts b/apps/readest-app/src/utils/toc.ts index 00886be5..84108f88 100644 --- a/apps/readest-app/src/utils/toc.ts +++ b/apps/readest-app/src/utils/toc.ts @@ -193,7 +193,9 @@ const updateTocLocation = ( item.id ??= index++; if (item.href) { const id = bookDoc.splitTOCHref(item.href)[0]!; - const section = sectionsMap[item.href] || sectionsMap[id]; + const exactMatch = sectionsMap[item.href]; + const baseMatch = sectionsMap[id]; + const section = (exactMatch?.cfi ? exactMatch : null) || baseMatch || exactMatch; if (section) { item.cfi = section.cfi; if (