From 996f550783b6cccfa52f315fe178113cbcbf287e Mon Sep 17 00:00:00 2001 From: Jiminy Panoz Date: Thu, 16 Jul 2026 17:03:44 +0200 Subject: [PATCH 1/4] Resolve tocEntry for itemless resources --- .../publication/services/timeline/Timeline.ts | 43 ++++++++++++++++--- shared/test/Timeline.test.ts | 43 +++++++++++++++++-- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/shared/src/publication/services/timeline/Timeline.ts b/shared/src/publication/services/timeline/Timeline.ts index eba70dc6..f992e0e3 100644 --- a/shared/src/publication/services/timeline/Timeline.ts +++ b/shared/src/publication/services/timeline/Timeline.ts @@ -267,10 +267,12 @@ export class Timeline { } /** - * Maps a `TimelineItem` (typically from `locate()`) to its `ContextualizedTocEntry`: - * a direct match first, falling back to the nearest preceding toc entry - * for that resource when there's no exact match (e.g. a mid-resource - * audio position between chapter markers). + * Maps a `TimelineItem` (typically from `locate()`) to its `ContextualizedTocEntry`, + * trying three fallbacks in order: + * 1. A direct match on `current`'s own link. + * 2. The nearest preceding toc entry within the same resource. + * 3. When no toc entry references the resource at all, the nearest + * preceding resource's toc entry. */ tocEntryFor(current: TimelineItem): ContextualizedTocEntry | undefined { const link = this.linkFor(current); @@ -279,7 +281,10 @@ export class Timeline { const direct = this.findTocEntryByLink(this.contextualizedToc, link); if (direct) return direct; - return this.nearestTocEntryForResource(link.href, current); + const nearest = this.nearestTocEntryForResource(link.href, current); + if (nearest) return nearest; + + return this.previousResolvedTocEntry(link.href); } private get flat(): TimelineItem[] { @@ -470,6 +475,11 @@ export class Timeline { return undefined; } + /** + * Tier-2 fallback for `tocEntryFor`: among the toc entries that reference + * `href`'s own resource (e.g. chapter markers within one audio file), + * return the nearest one at or before `current`'s position/scroll. + */ private nearestTocEntryForResource(href: string, current: TimelineItem): ContextualizedTocEntry | undefined { const bare = Timeline.bareHref(href); // Re-walk with raw items (not the display-formatted TocEntry tree) so we can compare @@ -491,6 +501,29 @@ export class Timeline { return this.findTocEntryByLink(this.contextualizedToc, chosen.link); } + /** + * Tier-3 fallback for `tocEntryFor`: when no toc entry references + * `href`'s resource at all, walk backward through the reading order and + * return the nearest preceding resource's toc entry. + */ + private previousResolvedTocEntry(href: string): ContextualizedTocEntry | undefined { + const bare = Timeline.bareHref(href); + const index = this._allItems.findIndex(item => this.itemMatchesHref(item, bare)); + if (index === -1) return undefined; + + for (let i = index - 1; i >= 0; i--) { + const precedingLink = this.linkFor(this._allItems[i]); + if (!precedingLink) continue; + const precedingBare = Timeline.bareHref(precedingLink.href); + const { atStart, fragments } = Timeline.collectTocCandidates(this.tocLinks, precedingBare, this._tocDepth, 1); + const chosen = atStart[0] ?? (fragments.length === 1 ? fragments[0] : undefined); + if (!chosen) continue; + const entry = this.findTocEntryByLink(this.contextualizedToc, chosen); + if (entry) return entry; + } + return undefined; + } + // ------------------------------------------------------------------------- // Shared utilities // ------------------------------------------------------------------------- diff --git a/shared/test/Timeline.test.ts b/shared/test/Timeline.test.ts index 788ab971..ab1cb0a7 100644 --- a/shared/test/Timeline.test.ts +++ b/shared/test/Timeline.test.ts @@ -777,7 +777,7 @@ describe('Timeline – contextualizedToc', () => { // --------------------------------------------------------------------------- describe('Timeline – tocEntryFor', () => { - it('10.1 direct match: a TOC-derived child item resolves to its own toc entry', () => { + it('10.1 tier-1 direct match: a TOC-derived child item resolves to its own toc entry', () => { const t = build( ro({ href: 'chapter1.html', title: 'Chapter 1' }), toc({ href: 'chapter1.html#intro', title: 'Introduction' }), @@ -786,7 +786,7 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(child)?.link.title).toBe('Introduction'); }); - it('10.2 fallback match: a plain reading-order item resolves to the nearest preceding toc entry by scroll', () => { + it('10.2 tier-2 fallback: a plain reading-order item resolves to the nearest preceding toc entry by scroll', () => { const t = build( ro({ href: 'chapter1.html', title: 'Chapter 1' }), toc( @@ -802,7 +802,42 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(t.items[0])?.link.title).toBe('Section 2'); }); - it('10.3 no match: an item unknown to the timeline resolves to undefined', () => { + it('10.3 tier-3 fallback: a resource with no TOC entries of its own resolves to the preceding resource\'s toc entry', () => { + // One TOC entry spans three consecutive RO resources — only the first is referenced. + const t = build( + ro( + { href: 'chapter1.html' }, + { href: 'chapter1-2.html' }, + { href: 'chapter1-3.html' }, + ), + toc({ href: 'chapter1.html', title: 'Chapter One' }), + ); + expect(t.tocEntryFor(t.items[1])?.link.title).toBe('Chapter One'); + }); + + it('10.4 tier-3 fallback: walks back past multiple untitled resources to find the nearest preceding entry', () => { + const t = build( + ro( + { href: 'chapter1.html' }, + { href: 'chapter1-2.html' }, + { href: 'chapter1-3.html' }, + ), + toc({ href: 'chapter1.html', title: 'Chapter One' }), + ); + // chapter1-3.html has no toc entry of its own, and neither does chapter1-2.html + // immediately preceding it — must skip past it to chapter1.html's entry. + expect(t.tocEntryFor(t.items[2])?.link.title).toBe('Chapter One'); + }); + + it('10.5 tier-3 fallback: no preceding entry exists → undefined', () => { + const t = build( + ro({ href: 'chapter1.html' }, { href: 'chapter2.html', title: 'Chapter 2' }), + toc({ href: 'chapter2.html', title: 'Chapter Two' }), + ); + expect(t.tocEntryFor(t.items[0])).toBeUndefined(); + }); + + it('10.6 no match: an item unknown to the timeline resolves to undefined', () => { const t = build( ro({ href: 'chapter1.html', title: 'Chapter 1' }), toc({ href: 'chapter1.html#intro', title: 'Introduction' }), @@ -811,7 +846,7 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(untracked)).toBeUndefined(); }); - it('10.4 no manifest toc: a reading-order item direct-matches its own fallback toc entry', () => { + it('10.7 no manifest toc: a reading-order item direct-matches its own fallback toc entry', () => { const t = build(ro({ href: 'chapter1.html', title: 'Chapter 1' })); expect(t.tocEntryFor(t.items[0])?.link.href).toBe('chapter1.html'); }); From 1de9c00085bc36a70d800571d0de034b9636d209 Mon Sep 17 00:00:00 2001 From: Jiminy Panoz Date: Thu, 16 Jul 2026 17:15:09 +0200 Subject: [PATCH 2/4] Prepare packages --- navigator/CHANGELOG.MD | 6 ++++++ navigator/package.json | 2 +- shared/CHANGELOG.MD | 6 ++++++ shared/package.json | 2 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/navigator/CHANGELOG.MD b/navigator/CHANGELOG.MD index e521fb38..8c2360d2 100644 --- a/navigator/CHANGELOG.MD +++ b/navigator/CHANGELOG.MD @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.7.2] – 2026-07-16 + +### Fixed + +- `Timeline.tocEntryFor()` (via `@readium/shared`) now falls back to the nearest preceding resource's toc entry when a resource has no toc entry of its own — the case where one toc entry spans several consecutive reading-order resources ([#242](https://github.com/readium/ts-toolkit/pull/242)) + ## [2.7.1] – 2026-07-16 ### Fixed diff --git a/navigator/package.json b/navigator/package.json index 1a82be01..dab8ad1a 100644 --- a/navigator/package.json +++ b/navigator/package.json @@ -1,6 +1,6 @@ { "name": "@readium/navigator", - "version": "2.7.1", + "version": "2.7.2", "type": "module", "description": "Next generation SDK for publications in Web Apps", "author": "readium", diff --git a/shared/CHANGELOG.MD b/shared/CHANGELOG.MD index 11d7d2d7..97add596 100644 --- a/shared/CHANGELOG.MD +++ b/shared/CHANGELOG.MD @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.3.1] – 2026-07-16 + +### Fixed + +- `Timeline.tocEntryFor()` now falls back to the nearest preceding resource's toc entry when a resource has no toc entry of its own — the case where one toc entry spans several consecutive reading-order resources ([#242](https://github.com/readium/ts-toolkit/pull/242)) + ## [2.3.0] – 2026-07-16 ### Added diff --git a/shared/package.json b/shared/package.json index 6db3f519..1a6a9a67 100644 --- a/shared/package.json +++ b/shared/package.json @@ -1,6 +1,6 @@ { "name": "@readium/shared", - "version": "2.3.0", + "version": "2.3.1", "type": "module", "description": "Shared models to be used across other Readium projects and implementations in Typescript", "author": "readium", From 35162fa6cdd2ee057269267559e56c7430db884c Mon Sep 17 00:00:00 2001 From: Jiminy Panoz Date: Thu, 16 Jul 2026 17:38:43 +0200 Subject: [PATCH 3/4] Fix tier-3 tocEntryFor edge cases Ambiguous fragment-only coverage on a preceding resource was skipped instead of resolved (now picks the last fragment), and resource lookup matched by href instead of Link identity, which could hit the wrong occurrence when an href repeats in the reading order. --- .../publication/services/timeline/Timeline.ts | 9 ++--- shared/test/Timeline.test.ts | 38 +++++++++++++++++-- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/shared/src/publication/services/timeline/Timeline.ts b/shared/src/publication/services/timeline/Timeline.ts index f992e0e3..b84c9d89 100644 --- a/shared/src/publication/services/timeline/Timeline.ts +++ b/shared/src/publication/services/timeline/Timeline.ts @@ -284,7 +284,7 @@ export class Timeline { const nearest = this.nearestTocEntryForResource(link.href, current); if (nearest) return nearest; - return this.previousResolvedTocEntry(link.href); + return this.previousResolvedTocEntry(link); } private get flat(): TimelineItem[] { @@ -506,9 +506,8 @@ export class Timeline { * `href`'s resource at all, walk backward through the reading order and * return the nearest preceding resource's toc entry. */ - private previousResolvedTocEntry(href: string): ContextualizedTocEntry | undefined { - const bare = Timeline.bareHref(href); - const index = this._allItems.findIndex(item => this.itemMatchesHref(item, bare)); + private previousResolvedTocEntry(link: Link): ContextualizedTocEntry | undefined { + const index = this._allItems.findIndex(item => this.linkFor(item) === link); if (index === -1) return undefined; for (let i = index - 1; i >= 0; i--) { @@ -516,7 +515,7 @@ export class Timeline { if (!precedingLink) continue; const precedingBare = Timeline.bareHref(precedingLink.href); const { atStart, fragments } = Timeline.collectTocCandidates(this.tocLinks, precedingBare, this._tocDepth, 1); - const chosen = atStart[0] ?? (fragments.length === 1 ? fragments[0] : undefined); + const chosen = atStart[0] ?? (fragments.length > 0 ? fragments[fragments.length - 1] : undefined); if (!chosen) continue; const entry = this.findTocEntryByLink(this.contextualizedToc, chosen); if (entry) return entry; diff --git a/shared/test/Timeline.test.ts b/shared/test/Timeline.test.ts index ab1cb0a7..858af220 100644 --- a/shared/test/Timeline.test.ts +++ b/shared/test/Timeline.test.ts @@ -829,7 +829,24 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(t.items[2])?.link.title).toBe('Chapter One'); }); - it('10.5 tier-3 fallback: no preceding entry exists → undefined', () => { + it('10.5 tier-3 fallback: preceding resource with only ambiguous fragment entries still resolves, to the last one', () => { + // chapter1.html has no start-of-resource toc entry, and two fragment entries — + // ambiguous for title resolution (undefined title), but tier-3 must still + // pick one deterministically rather than skipping past it to nothing. + const t = build( + ro( + { href: 'chapter1.html' }, + { href: 'chapter1-2.html' }, + ), + toc( + { href: 'chapter1.html#s1', title: 'Section 1' }, + { href: 'chapter1.html#s2', title: 'Section 2' }, + ), + ); + expect(t.tocEntryFor(t.items[1])?.link.title).toBe('Section 2'); + }); + + it('10.6 tier-3 fallback: no preceding entry exists → undefined', () => { const t = build( ro({ href: 'chapter1.html' }, { href: 'chapter2.html', title: 'Chapter 2' }), toc({ href: 'chapter2.html', title: 'Chapter Two' }), @@ -837,7 +854,22 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(t.items[0])).toBeUndefined(); }); - it('10.6 no match: an item unknown to the timeline resolves to undefined', () => { + it('10.7 tier-3 fallback: does not misresolve when the reading order repeats the same href', () => { + // Two RO entries share an href; the second (untitled, no toc entries of + // its own) must resolve against its own preceding neighbor by identity, + // not the first occurrence of a matching href. + const t = build( + ro( + { href: 'shared.html', title: 'First' }, + { href: 'other.html' }, + { href: 'shared.html' }, + ), + toc({ href: 'other.html', title: 'Other' }), + ); + expect(t.tocEntryFor(t.items[2])?.link.title).toBe('Other'); + }); + + it('10.8 no match: an item unknown to the timeline resolves to undefined', () => { const t = build( ro({ href: 'chapter1.html', title: 'Chapter 1' }), toc({ href: 'chapter1.html#intro', title: 'Introduction' }), @@ -846,7 +878,7 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(untracked)).toBeUndefined(); }); - it('10.7 no manifest toc: a reading-order item direct-matches its own fallback toc entry', () => { + it('10.9 no manifest toc: a reading-order item direct-matches its own fallback toc entry', () => { const t = build(ro({ href: 'chapter1.html', title: 'Chapter 1' })); expect(t.tocEntryFor(t.items[0])?.link.href).toBe('chapter1.html'); }); From 88c47bbeae191ad0e83f4f68a66c174a0cef8146 Mon Sep 17 00:00:00 2001 From: Jiminy Panoz Date: Thu, 16 Jul 2026 17:47:49 +0200 Subject: [PATCH 4/4] Avoid tier 3 cross-resource misresolution --- .../publication/services/timeline/Timeline.ts | 7 +++++- shared/test/Timeline.test.ts | 22 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/shared/src/publication/services/timeline/Timeline.ts b/shared/src/publication/services/timeline/Timeline.ts index b84c9d89..4ad73d4b 100644 --- a/shared/src/publication/services/timeline/Timeline.ts +++ b/shared/src/publication/services/timeline/Timeline.ts @@ -515,7 +515,12 @@ export class Timeline { if (!precedingLink) continue; const precedingBare = Timeline.bareHref(precedingLink.href); const { atStart, fragments } = Timeline.collectTocCandidates(this.tocLinks, precedingBare, this._tocDepth, 1); - const chosen = atStart[0] ?? (fragments.length > 0 ? fragments[fragments.length - 1] : undefined); + // Exclude fragment-only entries (bareHref "") — collectTocCandidates + // treats them as matching any bare href for single-track audio, but + // here they must belong to this specific preceding resource. + const ownAtStart = atStart.filter(l => Timeline.bareHref(l.href) === precedingBare); + const ownFragments = fragments.filter(l => Timeline.bareHref(l.href) === precedingBare); + const chosen = ownAtStart[0] ?? (ownFragments.length > 0 ? ownFragments[ownFragments.length - 1] : undefined); if (!chosen) continue; const entry = this.findTocEntryByLink(this.contextualizedToc, chosen); if (entry) return entry; diff --git a/shared/test/Timeline.test.ts b/shared/test/Timeline.test.ts index 858af220..40ad2639 100644 --- a/shared/test/Timeline.test.ts +++ b/shared/test/Timeline.test.ts @@ -846,7 +846,21 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(t.items[1])?.link.title).toBe('Section 2'); }); - it('10.6 tier-3 fallback: no preceding entry exists → undefined', () => { + it('10.6 tier-3 fallback: excludes fragment-only toc entries when resolving the preceding resource', () => { + // "#t=30" is fragment-only (single-track audio convention) and loosely + // matches any resource's own title resolution in build(), but it does + // not belong to track1.mp3 specifically — must not be picked as its entry. + const t = build( + ro( + { href: 'track1.mp3' }, + { href: 'track2.mp3' }, + ), + toc({ href: '#t=30', title: 'Marker' }), + ); + expect(t.tocEntryFor(t.items[1])).toBeUndefined(); + }); + + it('10.7 tier-3 fallback: no preceding entry exists → undefined', () => { const t = build( ro({ href: 'chapter1.html' }, { href: 'chapter2.html', title: 'Chapter 2' }), toc({ href: 'chapter2.html', title: 'Chapter Two' }), @@ -854,7 +868,7 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(t.items[0])).toBeUndefined(); }); - it('10.7 tier-3 fallback: does not misresolve when the reading order repeats the same href', () => { + it('10.8 tier-3 fallback: does not misresolve when the reading order repeats the same href', () => { // Two RO entries share an href; the second (untitled, no toc entries of // its own) must resolve against its own preceding neighbor by identity, // not the first occurrence of a matching href. @@ -869,7 +883,7 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(t.items[2])?.link.title).toBe('Other'); }); - it('10.8 no match: an item unknown to the timeline resolves to undefined', () => { + it('10.9 no match: an item unknown to the timeline resolves to undefined', () => { const t = build( ro({ href: 'chapter1.html', title: 'Chapter 1' }), toc({ href: 'chapter1.html#intro', title: 'Introduction' }), @@ -878,7 +892,7 @@ describe('Timeline – tocEntryFor', () => { expect(t.tocEntryFor(untracked)).toBeUndefined(); }); - it('10.9 no manifest toc: a reading-order item direct-matches its own fallback toc entry', () => { + it('10.10 no manifest toc: a reading-order item direct-matches its own fallback toc entry', () => { const t = build(ro({ href: 'chapter1.html', title: 'Chapter 1' })); expect(t.tocEntryFor(t.items[0])?.link.href).toBe('chapter1.html'); });