diff --git a/flutter_readium/CHANGELOG.md b/flutter_readium/CHANGELOG.md index ce6753f5..7a4e6cd6 100644 --- a/flutter_readium/CHANGELOG.md +++ b/flutter_readium/CHANGELOG.md @@ -5,6 +5,13 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased +### Fixed + +- **Jumping to a saved position in a different audiobook track landed at the wrong + time (iOS).** The offset was scaled by the length of the track already playing + instead of the target track. Restoring in a freshly opened book was the worst + case: it started that track from the beginning. + ## [0.4.1] - 2026-08-27 ### Added diff --git a/flutter_readium/example/integration_test/groups/timebased_playback_test.dart b/flutter_readium/example/integration_test/groups/timebased_playback_test.dart index e51e4e5d..cbef6021 100644 --- a/flutter_readium/example/integration_test/groups/timebased_playback_test.dart +++ b/flutter_readium/example/integration_test/groups/timebased_playback_test.dart @@ -214,6 +214,82 @@ void main() { await harness.readium.pause(); }); + + // Regression: the progression -> time-offset helper resolved the track + // duration from the *currently playing* locator, so a cross-track jump + // scaled progression by the wrong track's length. + test('goToLocator scales progression by the target track, not the playing one', () async { + final path = harness.fixturePath( + FixtureKeys.audiobook, + reason: 'Fixture ${FixtureKeys.audiobook} missing from asset bundle', + ); + + final pub = await harness.readium.openPublication(path); + + final firstDuration = pub.readingOrder.first.duration; + final targetIndex = pub.readingOrder.length - 1; + final targetLink = pub.readingOrder[targetIndex]; + final targetDuration = targetLink.duration; + expect( + firstDuration != null && targetDuration != null && targetDuration > firstDuration * 2, + isTrue, + reason: + 'Test needs the last track to be far longer than the first so the two ' + 'candidate offsets are distinguishable (got $firstDuration vs $targetDuration)', + ); + + final states = []; + final sub = harness.readium.onTimebasedPlayerStateChanged.listen(states.add); + addTearDown(sub.cancel); + + await harness.readium.audioEnable(prefs: AudioPreferences(speed: 1.0)); + + // Start on the first (short) track so it becomes the current locator. + await harness.readium.play(null); + await waitUntil( + () => states.any((s) => s.state == TimebasedState.playing && s.currentOffset != null), + timeout: const Duration(seconds: 20), + reason: 'Never reached playing state on the first track', + ); + await harness.readium.pause(); + + // No time fragment: progression is the only offset signal, which is what + // Publication.resolveLocator now produces for a relocated audiobook locator. + const progression = 0.5; + final navigated = await harness.readium.goToLocator( + Locator( + href: targetLink.href, + type: targetLink.type ?? 'audio/mpeg', + locations: Locations(position: targetIndex + 1, progression: progression), + ), + ); + expect(navigated, isTrue, reason: 'goToLocator to the last track should succeed'); + + await waitUntil( + () => states.last.currentLocator?.href == targetLink.href && states.last.currentOffset != null, + timeout: const Duration(seconds: 20), + reason: 'Never landed on the target track with an offset', + ); + + final expectedSeconds = targetDuration! * progression; + final wrongSeconds = firstDuration! * progression; + final landedSeconds = states.last.currentOffset!.inMilliseconds / 1000.0; + + expect( + (landedSeconds - expectedSeconds).abs(), + lessThan((landedSeconds - wrongSeconds).abs()), + reason: + 'Landed at ${landedSeconds}s: closer to the current-track answer ' + '(${wrongSeconds}s) than to the target-track answer (${expectedSeconds}s)', + ); + expect( + landedSeconds, + greaterThan(wrongSeconds + 1), + reason: 'Offset was scaled by the first track duration, not the target track', + ); + + await harness.readium.pause(); + }); }, ); }); diff --git a/flutter_readium/example/pubspec.lock b/flutter_readium/example/pubspec.lock index 3eda00b3..36346ab6 100644 --- a/flutter_readium/example/pubspec.lock +++ b/flutter_readium/example/pubspec.lock @@ -177,14 +177,14 @@ packages: path: ".." relative: true source: path - version: "0.4.0" + version: "0.4.1" flutter_readium_platform_interface: dependency: "direct overridden" description: path: "../../flutter_readium_platform_interface" relative: true source: path - version: "0.4.0" + version: "0.4.1" flutter_test: dependency: "direct dev" description: flutter diff --git a/flutter_readium/ios/flutter_readium/Sources/flutter_readium/navigator/FlutterAudioNavigator.swift b/flutter_readium/ios/flutter_readium/Sources/flutter_readium/navigator/FlutterAudioNavigator.swift index 145eb308..ef319374 100644 --- a/flutter_readium/ios/flutter_readium/Sources/flutter_readium/navigator/FlutterAudioNavigator.swift +++ b/flutter_readium/ios/flutter_readium/Sources/flutter_readium/navigator/FlutterAudioNavigator.swift @@ -192,9 +192,12 @@ public class FlutterAudioNavigator: FlutterTimebasedNavigator, AudioNavigatorDel return false } + /// Progression is relative to the track the locator points at, so the duration must + /// come from that track. This used to shadow the parameter with `audioLocator`, which + /// scaled by the *currently playing* track instead — wrong for any cross-track jump, + /// and nil at cold open, which then fell back to a start-of-track seek. private func getTimeOffsetForLocatorWithProgression(locator: Locator, progression: Double) -> Double? { - guard let locator = audioLocator, - let link = publication.readingOrder.firstWithHREF(locator.href), + guard let link = publication.readingOrder.firstWithHREF(locator.href), let duration = link.duration, duration.isFinite else { return nil } @@ -812,7 +815,7 @@ public class FlutterAudioNavigator: FlutterTimebasedNavigator, AudioNavigatorDel /// Progression is resolved to a time fragment here, as this resolution is unique to AudioNavigator. // TODO: This should really be handled by the Readium Navigator (upstream issue). if let progression = locator.locations.progression, progression.isFinite, - let preciseTimeOffset = getTimeOffsetForLocatorWithProgression(locator: locator, progression: progression) { + let preciseTimeOffset = getTimeOffsetForLocatorWithProgression(locator: resolvedLocator, progression: progression) { timeOffset = preciseTimeOffset } resolvedLocator = resolvedLocator.copyWithOffset(timeOffset ?? 0.0) diff --git a/flutter_readium_platform_interface/CHANGELOG.md b/flutter_readium_platform_interface/CHANGELOG.md index ae9f9800..957147f9 100644 --- a/flutter_readium_platform_interface/CHANGELOG.md +++ b/flutter_readium_platform_interface/CHANGELOG.md @@ -5,6 +5,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased +### Added + +- `Publication.resolveLocator(Locator)` — resolves a stored locator whose href has + left the reading order after a publication was re-issued with different resource + granularity. Prefers the segmentation-independent `totalProgression` mapped onto + `Link.duration`, falls back to a bounds-safe `position` lookup, and returns `null` + rather than guessing. Drops the stale `cssSelector`/`fragments` and updates `type`. + ## [0.4.1] - 2026-08-27 ### Added diff --git a/flutter_readium_platform_interface/lib/src/shared/publication/publication.dart b/flutter_readium_platform_interface/lib/src/shared/publication/publication.dart index b948a6ad..4941ab05 100644 --- a/flutter_readium_platform_interface/lib/src/shared/publication/publication.dart +++ b/flutter_readium_platform_interface/lib/src/shared/publication/publication.dart @@ -240,6 +240,105 @@ class Publication with Equatable implements JSONable { return split == -1 ? null : find(href.substring(0, split)); } + /// Resolves [locator] against this publication, for when the stored href is + /// no longer part of the reading order — e.g. the publication was re-issued + /// with different resource granularity (chapters split/merged, or a + /// text-with-overlay reading order replaced by an audio-only one). + /// + /// Call this before navigating to any persisted [Locator] (last position, + /// bookmark, highlight) once the publication it was recorded against might + /// have changed shape. It's safe and cheap to call unconditionally: a + /// locator whose href is still valid is returned unchanged, without + /// touching [readingOrder]. + /// + /// Resolution order: + /// 1. [linkWithHref] on [Locator.hrefPath] — still valid, returned as-is. + /// 2. [Locations.totalProgression] mapped onto accumulated [Link.duration] + /// across [readingOrder]. Segmentation-independent, so preferred + /// whenever every entry has a duration. + /// 3. [Locations.position] as a 1-based index into [readingOrder]. Exact + /// only when the new segmentation happens to line up with the old one, + /// so tried last; an out-of-range position resolves to `null` instead + /// of throwing. + /// + /// Returns `null` when none of the above yields a position — callers + /// should decide what to do next (e.g. fall back to the first reading-order + /// link) rather than have this guess. + Locator? resolveLocator(final Locator locator) { + if (linkWithHref(locator.hrefPath) != null) { + return locator; + } + + final locations = locator.locations; + final totalProgression = locations?.totalProgression; + if (totalProgression != null && totalProgression > 0) { + final resolved = _resolveLocatorByDuration(locator, totalProgression); + if (resolved != null) { + return resolved; + } + } + + final position = locations?.position; + if (position == null || position <= 0) { + return null; + } + + final index = position - 1; + return readingOrder.elementAtOrNull(index) == null ? null : _relocate(locator, index, progression: null); + } + + /// Maps [totalProgression] onto the accumulated [Link.duration] of + /// [readingOrder], returning the containing link and the offset within it. + /// Returns `null` if any entry is missing a duration or the total duration + /// is zero — the caller falls back to [Locations.position] in that case. + Locator? _resolveLocatorByDuration(final Locator locator, final double totalProgression) { + final durations = readingOrder.map((final link) => link.duration).toList(); + if (durations.isEmpty || durations.any((final duration) => duration == null)) { + return null; + } + + final total = durations.cast().fold(0.0, (final sum, final duration) => sum + duration); + if (total <= 0) { + return null; + } + + final target = totalProgression * total; + var accumulated = 0.0; + for (var i = 0; i < readingOrder.length; i++) { + final duration = durations[i]!; + final isLastLink = i == readingOrder.length - 1; + if (target <= accumulated + duration || isLastLink) { + final progression = duration > 0 ? ((target - accumulated) / duration).clamp(0.0, 1.0) : 0.0; + return _relocate(locator, i, progression: progression); + } + accumulated += duration; + } + return null; // Unreachable: the loop always returns on the last link. + } + + /// Rebuilds [locator] to point at `readingOrder[index]`: [Locator.href] and + /// [Locator.type] become the resolved link's, [Locations.position] becomes + /// `index + 1`, and [progression] (when known) replaces the old one. + /// [Locations.cssSelector] and [Locations.fragments] described the old + /// resource, so they are dropped rather than carried over. + /// + /// Takes the index rather than the [Link] because [Link] compares by value: + /// a reading order containing two equal entries would resolve to the first + /// one's index, reporting a position the locator was never at. + Locator _relocate(final Locator locator, final int index, {required final double? progression}) { + final link = readingOrder[index]; + + return locator.copyWith( + href: link.href, + type: link.type ?? locator.type, + locations: Locations( + position: index + 1, + progression: progression, + totalProgression: locator.locations?.totalProgression, + ), + ); + } + /// The cover [Link], found by `rel=cover` or by href/type heuristics. `null` if not present. Link? get coverLink => resources.firstWhereOrNull( (final r) => diff --git a/flutter_readium_platform_interface/test/publication_resolve_locator_test.dart b/flutter_readium_platform_interface/test/publication_resolve_locator_test.dart new file mode 100644 index 00000000..a2efb4d1 --- /dev/null +++ b/flutter_readium_platform_interface/test/publication_resolve_locator_test.dart @@ -0,0 +1,213 @@ +import 'package:flutter_readium_platform_interface/flutter_readium_platform_interface.dart'; +import 'package:flutter_test/flutter_test.dart'; + +// Ground-truth fixtures: real Lastmark locators stored under the old +// WebPubAudio-shaped reading order (xhtml hrefs with a media-overlay behind +// them), now resolved against a republished audio-only reading order. +final _fixture26786 = { + 'href': '/26786-0029-generic.xhtml', + 'type': 'audio/mpeg', + 'title': '25', + 'locations': { + 'cssSelector': '#hix00029', + 'fragments': ['t=732.3'], + 'position': 29, + 'progression': 0.9332, + 'totalProgression': 0.4252, + }, + 'x-timestamp': '2026-05-11T14:08:00+00:00', + 'x-type': 'Lastmark', +}; + +final _fixture27651 = { + 'href': '/27651-0004-generic.xhtml', + 'type': 'audio/mpeg', + 'title': 'Section', + 'locations': { + 'cssSelector': '#text_0029', + 'fragments': ['t=46.8'], + 'position': 4, + 'progression': 0.0631, + 'totalProgression': 0.0036, + }, + 'x-timestamp': '2025-09-15T11:43:39+00:00', + 'x-type': 'Lastmark', +}; + +final _fixture23308 = { + 'href': '23308-0002-generic.xhtml', + 'type': 'application/xhtml+xml', + 'title': 'Lydbokavtalen', + 'locations': { + 'cssSelector': '#dtb4', + 'fragments': ['t=18.078651722'], + 'position': 2, + 'progression': 0.2949, + 'totalProgression': 0.0003, + }, + 'x-timestamp': '2026-05-28T16:59:29+00:00', + 'x-type': 'Lastmark', +}; + +Publication _publicationWithReadingOrder(List readingOrder) => Publication( + metadata: Metadata(localizedTitle: LocalizedString.fromStrings({'en': 'Test Book'})), + readingOrder: readingOrder, +); + +void main() { + group('Publication.resolveLocator', () { + test('href still present returns the locator unchanged', () { + final link = Link(href: '/ch1.xhtml', type: 'application/xhtml+xml'); + final pub = _publicationWithReadingOrder([link]); + final locator = Locator( + href: '/ch1.xhtml', + type: 'application/xhtml+xml', + locations: Locations(position: 1), + ); + + expect(pub.resolveLocator(locator), same(locator)); + }); + + test('href absent, totalProgression usable, durations present resolves via duration', () { + final readingOrder = [ + Link(href: '/26786-0001.mp3', type: 'audio/mpeg', duration: 100), + Link(href: '/26786-0002.mp3', type: 'audio/mpeg', duration: 100), + Link(href: '/26786-0003.mp3', type: 'audio/mpeg', duration: 100), + ]; + final pub = _publicationWithReadingOrder(readingOrder); + final locator = Locator.fromJson(_fixture26786)!; + + final resolved = pub.resolveLocator(locator); + + expect(resolved, isNotNull); + expect(resolved!.href, '/26786-0002.mp3'); + expect(resolved.type, 'audio/mpeg'); + expect(resolved.locations?.position, 2); + expect(resolved.locations?.progression, closeTo(0.2756, 1e-3)); + expect(resolved.locations?.totalProgression, closeTo(0.4252, 1e-6)); + }); + + test('href absent, durations absent falls through to position without crashing', () { + final readingOrder = [ + Link(href: '/27651-0001.mp3', type: 'audio/mpeg'), + Link(href: '/27651-0002.mp3', type: 'audio/mpeg', duration: 50), + Link(href: '/27651-0003.mp3', type: 'audio/mpeg'), + Link(href: '/27651-0004.mp3', type: 'audio/mpeg', duration: 40), + ]; + final pub = _publicationWithReadingOrder(readingOrder); + final locator = Locator.fromJson(_fixture27651)!; + + final resolved = pub.resolveLocator(locator); + + expect(resolved, isNotNull); + expect(resolved!.href, '/27651-0004.mp3'); + expect(resolved.locations?.position, 4); + expect(resolved.locations?.progression, isNull); + }); + + test('position beyond readingOrder length returns null instead of throwing', () { + final readingOrder = [ + Link(href: '/a.mp3', type: 'audio/mpeg'), + Link(href: '/b.mp3', type: 'audio/mpeg'), + ]; + final pub = _publicationWithReadingOrder(readingOrder); + final locator = Locator( + href: '/missing.xhtml', + type: 'application/xhtml+xml', + locations: Locations(position: 5), + ); + + expect(() => pub.resolveLocator(locator), returnsNormally); + expect(pub.resolveLocator(locator), isNull); + }); + + test('totalProgression == 0 with position > 1 prefers position over duration', () { + final readingOrder = [ + Link(href: '/a.mp3', type: 'audio/mpeg', duration: 10), + Link(href: '/b.mp3', type: 'audio/mpeg', duration: 10), + Link(href: '/c.mp3', type: 'audio/mpeg', duration: 10), + ]; + final pub = _publicationWithReadingOrder(readingOrder); + final locator = Locator( + href: '/missing.xhtml', + type: 'application/xhtml+xml', + locations: Locations(position: 3, totalProgression: 0), + ); + + final resolved = pub.resolveLocator(locator); + + expect(resolved, isNotNull); + expect(resolved!.href, '/c.mp3'); + expect(resolved.locations?.position, 3); + expect(resolved.locations?.progression, isNull); + }); + + test('totalProgression == 1.0 lands in the last link, not out of range', () { + final readingOrder = [ + Link(href: '/a.mp3', type: 'audio/mpeg', duration: 10), + Link(href: '/b.mp3', type: 'audio/mpeg', duration: 20), + ]; + final pub = _publicationWithReadingOrder(readingOrder); + final locator = Locator( + href: '/missing.xhtml', + type: 'application/xhtml+xml', + locations: Locations(totalProgression: 1), + ); + + final resolved = pub.resolveLocator(locator); + + expect(resolved, isNotNull); + expect(resolved!.href, '/b.mp3'); + expect(resolved.locations?.position, 2); + expect(resolved.locations?.progression, closeTo(1.0, 1e-9)); + }); + + test('empty reading order returns null', () { + final pub = _publicationWithReadingOrder([]); + final locator = Locator( + href: '/missing.xhtml', + type: 'application/xhtml+xml', + locations: Locations(position: 1, totalProgression: 0.5), + ); + + expect(pub.resolveLocator(locator), isNull); + }); + + test('resolved locator drops stale cssSelector/fragments and updates type', () { + final readingOrder = [ + Link(href: '/23308-0001.mp3', type: 'audio/mpeg', duration: 10), + Link(href: '/23308-0002.mp3', type: 'audio/mpeg', duration: 10), + ]; + final pub = _publicationWithReadingOrder(readingOrder); + final locator = Locator.fromJson(_fixture23308)!; + // Sanity: the fixture really does carry stale HTML-fragment locations. + expect(locator.locations?.cssSelector, isNotNull); + expect(locator.type, 'application/xhtml+xml'); + + final resolved = pub.resolveLocator(locator); + + expect(resolved, isNotNull); + expect(resolved!.type, 'audio/mpeg'); + expect(resolved.locations?.cssSelector, isNull); + expect(resolved.locations?.fragments, isEmpty); + }); + + test('duplicate reading-order entries still report the real position', () { + // Link compares by value, so resolving through the Link rather than its + // index would report position 1 for every one of these. + final duplicate = Link(href: '/dup.mp3', type: 'audio/mpeg', duration: 100); + final pub = _publicationWithReadingOrder([duplicate, duplicate, duplicate]); + final locator = Locator( + href: '/gone.xhtml', + type: 'application/xhtml+xml', + locations: Locations(position: 3, totalProgression: 0.9), + ); + + final resolved = pub.resolveLocator(locator); + + expect(resolved, isNotNull); + // 0.9 * 300 = 270, which falls inside the third entry. + expect(resolved!.locations?.position, 3); + }); + }); +}