From 5b07fb0a6a544aadfc71d053ee7a6fb16b754fd9 Mon Sep 17 00:00:00 2001 From: vito Date: Tue, 18 Aug 2026 16:40:23 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(reader):=20=F0=9F=90=9B=20keep=20the=20?= =?UTF-8?q?reading=20position=20when=20the=20app=20is=20refocused=20(#95)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Readium's EPUB navigator pads its own page container by the system-bar insets (Configuration.shouldApplyInsetsPadding, on by default). Quire already owns the reader's insets — full-screen reading is deliberately edge-to-edge, and with it off the whole reader subtree is padded by WindowInsets.systemBars — so that padding is either for bars that are hidden or a second helping of the same inset. It also arrived late. The page rendered at full height, then the first time the window regained focus the WebView finally re-measured against the padding and lost ~350px (measured: CoordinatorLayout pad=247/105, R2WebView 1080x2424 -> 1080x2072@247). That re-paginated the chapter, so the reader came back to a different page, and because the resize happened inside Readium's own view tree nothing here saw it coming: the post-re-pagination locator was published and written over the saved position, making the wrong page permanent. Turn that padding off, and harden the re-anchor while we're at it. It used to be armed only for resizes the reader could name — rotation and the full-screen-reading toggle — which is why an unnamed one did this much damage. ReaderViewModel.onViewportChanged now arms on the measured viewport instead, covering causes we haven't thought of; seek/goTo clear a pending anchor so an explicit jump isn't undone by one. Verified on an emulator with a 5-chapter fixture: full-screen reading on, five screenshots across four background/resume cycles are byte-identical (before: drifted, with a 247px gap that stuck); off, three cycles hold position with a single correct inset. --- .../ereader/ui/reader/ReaderScreen.kt | 52 +++++- .../ereader/ui/reader/ReaderViewModel.kt | 33 ++++ .../ui/reader/ReaderViewportResizeTest.kt | 154 ++++++++++++++++++ 3 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt diff --git a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt index b9325e1..cff45bc 100644 --- a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt +++ b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt @@ -31,8 +31,10 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.onSizeChanged import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalView +import androidx.compose.ui.unit.IntSize import androidx.compose.ui.viewinterop.AndroidView import androidx.core.view.WindowCompat import androidx.core.view.WindowInsetsCompat @@ -137,6 +139,20 @@ fun ReaderScreen(viewModel: ReaderViewModel, onClose: () -> Unit) { onExit = viewModel::clearPendingResize, ) + // Last measured size of the reader content. Every change is reported to the + // view model, which re-anchors the reading position across it whatever caused + // it — see ReaderViewModel.onViewportChanged. The settle valve is keyed on the + // size so a resize that arrives in several steps (an inset animation, say) + // restarts the wait instead of completing mid-flight; Readium's onPageChanged + // normally beats it to the re-anchor and makes it a no-op. + var viewport by remember { mutableStateOf(IntSize.Zero) } + LaunchedEffect(viewport) { + if (viewport != IntSize.Zero) { + delay(RESIZE_SETTLE_MS) + viewModel.completeViewportResize() + } + } + // The app is edge-to-edge (MainActivity). Immersive reading uses that full // bleed: content draws behind the (hidden) bars and the chrome self-insets. // With immersive off, the reader behaves like any normal screen — inset the @@ -162,6 +178,10 @@ fun ReaderScreen(viewModel: ReaderViewModel, onClose: () -> Unit) { onNext = viewModel::pageForward, onToggleChrome = viewModel::toggleChrome, onPageLoaded = viewModel::completeViewportResize, + onViewportChanged = { size -> + viewport = size + viewModel.onViewportChanged(size.width, size.height) + }, ) ReaderTopBar( @@ -228,6 +248,7 @@ private fun ReaderContent( onNext: () -> Unit, onToggleChrome: () -> Unit, onPageLoaded: () -> Unit, + onViewportChanged: (IntSize) -> Unit, ) { val activity = LocalContext.current as FragmentActivity val containerId = rememberSaveable { View.generateViewId() } @@ -235,7 +256,11 @@ private fun ReaderContent( var fragment by remember { mutableStateOf(null) } AndroidView( - modifier = Modifier.fillMaxSize(), + // This is the node whose height decides how much text fits in a Readium column, so + // it — not the window, and not the insets — is the authoritative viewport. + modifier = Modifier + .fillMaxSize() + .onSizeChanged(onViewportChanged), factory = { ctx -> ReaderTapDispatcher(ctx).apply { layoutParams = ViewGroup.LayoutParams( @@ -280,6 +305,24 @@ private fun ReaderContent( initialLocator = initialLocator, initialPreferences = preferences.toEpubPreferences(), paginationListener = paginationListener, + configuration = EpubNavigatorFragment.Configuration( + // Quire owns the reader's insets, so Readium must not also apply them. + // + // Left at its default (on), Readium pads its own page container by the + // system-bar insets whenever they are dispatched to its view. In full-screen + // reading the bars are hidden and the reader is deliberately full-bleed, so + // that padding is for bars that aren't there — and it arrived late: the page + // rendered edge to edge, then the first time the window regained focus the + // WebView finally re-measured against it and lost ~350px of height. That + // re-paginated the chapter under the reader, and because the resize happened + // inside Readium's own view tree, nothing here saw it coming: the post- + // re-pagination locator was published and written over the saved position, so + // the reader came back to the wrong page and stayed there (issue #95). + // + // With full-screen reading off it was simply double-inset: the whole reader + // subtree is already padded by WindowInsets.systemBars in ReaderScreen. + shouldApplyInsetsPadding = false, + ), ) val nav = (fm.fragmentFactory.instantiate( activity.classLoader, @@ -310,6 +353,13 @@ private fun ReaderContent( } } +/** + * How long to wait for Readium to re-paginate after the viewport changed before honouring the + * anchor anyway. onPageChanged normally gets there first; this only has to cover a resize that + * didn't re-paginate at all, so that locator publishing is never left suppressed. + */ +private const val RESIZE_SETTLE_MS = 600L + /** * Drives immersive full-screen: binds the OS status + navigation bars to the reader * chrome. Every window mutation is snapshotted once and reverted on exit so the rest of diff --git a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt index d1d1776..cca79df 100644 --- a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt +++ b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt @@ -87,6 +87,7 @@ class ReaderViewModel( } fun goTo(locator: Locator) { + clearPendingResize() viewModelScope.launch { navigator?.go(locator, false) } } @@ -132,6 +133,35 @@ class ReaderViewModel( private var pendingRotationAnchor: Locator? = null private var suppressLocatorPublishing: Boolean = false + private var viewportSize: Pair? = null + + /** + * Reports the reader viewport's measured size, on every layout pass. + * + * Any change in that size re-paginates Readium's WebView, after which Readium reports + * whatever text happened to land on screen. Nothing marks that locator as junk, so left + * alone it is published to the HUD and written over the reader's saved position — the + * reader comes back to the wrong page and stays there. + * + * Everything else here arms the re-anchor by naming a cause: [beginViewportResize] is + * called from `MainActivity.onConfigurationChanged` for rotation, and from the + * full-screen-reading toggle. Issue #95 was a resize nobody had named, so this arms on + * the effect instead — the measured size — and covers causes we haven't thought of. + * (It only sees resizes of the Compose node; one that happens further down, inside + * Readium's own view tree, is invisible here. See `shouldApplyInsetsPadding` in + * ReaderScreen for the one that bit us.) + * + * The first report just establishes the baseline; the WebView is created at that size, so + * there is nothing to re-anchor. Zero sizes (pre-layout, or a detached view) are ignored + * so they never read as a resize in either direction. + */ + fun onViewportChanged(width: Int, height: Int) { + if (width <= 0 || height <= 0) return + val previous = viewportSize + viewportSize = width to height + if (previous == null || previous == viewportSize) return + beginViewportResize() + } // Called from MainActivity.onBeforeReaderConfigChange — runs BEFORE the // Activity dispatches the configuration change down to fragments. Snapshots @@ -174,6 +204,9 @@ class ReaderViewModel( fun seek(percent: Double) { val target = previewLocator(percent) ?: return val nav = navigator ?: return + // An explicit jump supersedes any armed re-anchor: the anchor predates the seek, so + // honouring it afterwards would yank the reader back out of the page they just chose. + clearPendingResize() // Surface the target on the HUD synchronously, before the suspending nav.go() // call dispatches. This avoids a one-frame window where the slider thumb // would snap back to the pre-seek liveLocator after the UI clears its drag diff --git a/app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt b/app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt new file mode 100644 index 0000000..42dbfba --- /dev/null +++ b/app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt @@ -0,0 +1,154 @@ +package io.theficos.ereader.ui.reader + +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import io.theficos.ereader.data.local.DocumentRepository +import io.theficos.ereader.data.local.ProgressRepository +import io.theficos.ereader.data.local.db.EReaderDatabase +import io.theficos.ereader.reader.ReaderPreferencesStore +import io.theficos.ereader.reader.ReadiumFactory +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.readium.r2.shared.publication.Locator +import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.mediatype.MediaType +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * The reader keeps its place across a viewport resize by snapshotting the current locator, + * ignoring everything Readium emits while the WebView re-paginates, and then going back to the + * snapshot. These tests cover which resizes arm that. + * + * Issue #95 was a resize nobody had armed, and what it cost is the shape of every test here: a + * post-re-pagination locator reached the HUD and the progress row, so the reader came back to + * the wrong page and its saved place was gone. That particular resize is now prevented at + * source (ReaderScreen turns off Readium's own inset padding); arming on the measured viewport + * is the backstop for the next one. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +// The stock Application: EReaderApp builds the whole DI container on create, keystore and +// all, which this test has no use for. +@Config(sdk = [33], application = android.app.Application::class) +class ReaderViewportResizeTest { + + private lateinit var db: EReaderDatabase + private lateinit var vm: ReaderViewModel + + @Before fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, EReaderDatabase::class.java) + .allowMainThreadQueries() + .build() + vm = ReaderViewModel( + documentId = 1L, + docs = DocumentRepository(db.documentDao()), + progress = ProgressRepository(db.progressDao()), + readium = ReadiumFactory(context), + preferencesStore = ReaderPreferencesStore(context), + ) + } + + @After fun tearDown() { + db.close() + Dispatchers.resetMain() + } + + private fun locatorAt(progression: Double): Locator = Locator( + href = Url("ch1.xhtml")!!, + mediaType = MediaType.XHTML, + locations = Locator.Locations(progression = progression, totalProgression = progression), + ) + + @Test fun `first viewport report only establishes the baseline`() = runTest { + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + + // Nothing to re-anchor: the WebView was created at this size, so Readium is still free + // to report where it actually is. + vm.publishLocator(locatorAt(0.6)) + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.6) + } + + @Test fun `an unchanged viewport does not arm a re-anchor`() = runTest { + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + vm.onViewportChanged(1080, 2424) + + vm.publishLocator(locatorAt(0.6)) + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.6) + } + + @Test fun `a shorter viewport suppresses the drifted locator and restores the anchor`() = runTest { + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + + // Something outside the reader's control shortens the viewport. + vm.onViewportChanged(1080, 2219) + + // Readium re-paginates and reports wherever the text landed. That must not reach the + // HUD or, through the locator flow, the saved progress row. + vm.publishLocator(locatorAt(0.42)) + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.5) + + vm.completeViewportResize() + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.5) + + // Publishing is live again once the anchor has been honoured. + vm.publishLocator(locatorAt(0.55)) + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.55) + } + + @Test fun `a resize arriving in steps keeps the original anchor`() = runTest { + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + + // An inset animation resizes the viewport over several frames, with Readium emitting + // drifted locators in between. The anchor must stay the pre-resize one throughout. + vm.onViewportChanged(1080, 2380) + vm.publishLocator(locatorAt(0.41)) + vm.onViewportChanged(1080, 2300) + vm.publishLocator(locatorAt(0.40)) + vm.onViewportChanged(1080, 2219) + + vm.completeViewportResize() + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.5) + } + + @Test fun `a zero-sized viewport is not a resize`() = runTest { + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + // A detached or not-yet-measured view reports zero; treating that as a resize would arm + // the re-anchor against a size the reader never actually had. + vm.onViewportChanged(0, 0) + vm.onViewportChanged(1080, 2424) + + vm.publishLocator(locatorAt(0.6)) + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.6) + } + + @Test fun `seeking supersedes an armed re-anchor`() = runTest { + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + vm.onViewportChanged(1080, 2219) + + // A seek can land while a re-anchor is armed. The reader must stay where the seek put + // it rather than being yanked back to the anchor, which predates the seek. + vm.goTo(locatorAt(0.9)) + vm.publishLocator(locatorAt(0.9)) + vm.completeViewportResize() + + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.9) + } +} From 95853c69151dbebf2031b42b5713f018e86f8d04 Mon Sep 17 00:00:00 2001 From: vito Date: Wed, 19 Aug 2026 01:26:42 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(reader):=20=F0=9F=90=9B=20return=20to?= =?UTF-8?q?=20the=20same=20page=20after=20rotating=20the=20device=20(#95)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rotate to landscape and back and the reader did not come back to the page you left. It moved by a few paragraphs, and rotating again moved it again, always further, so a few round trips could leave you a screen or two from where you were reading. A measured trace on a marker book went from paragraph 18 to 31 to 38 over three rotate-and-return trips. This is separate from the refocus bug in the previous commit and predates it. The reader put itself back by handing Readium the locator Readium had last published. That locator carries only `locations.progression`, a fraction of the way through the chapter, because Readium builds it from the EPUB positions service, which emits nothing but a progression and a position index. Readium restores such a locator arithmetically: `round(progression * numPages)`, then jump to that page. Rotating rebuilds the page grid the arithmetic is denominated in (12 portrait pages against 23 landscape ones in the test book) and the sum runs while the grid is still being rebuilt, so the page it picks is only loosely related to the page you left. Readium then publishes wherever it landed, that becomes the anchor for the next rotation, and the error compounds. So anchor on the document instead of on a fraction. Readium restores a locator precisely, by looking an element up in the DOM and scrolling to it, whenever the locator carries text to match, and it will tell you which element is on screen. Three things had to be true for that to actually hold still: The anchor has to be an element that *begins* on the page. Readium's own `findFirstVisibleLocator` returns the first element merely *overlapping* the page, which is usually one that began earlier, and restoring scrolls to where an element begins. Both round the same way, so they compound: anchoring that way still walked backwards, 22 to 21 to 20 to 18, down to the chapter heading. PAGE_START_ANCHOR_JS mirrors Readium's walk and changes the test to "begins here", which makes capture and restore inverses of each other. The re-anchor has to happen when the layout is final. Re-pagination arrives in steps and Readium snaps to page boundaries using a page width it caches in JavaScript, so an early re-anchor measures against the old grid. Anchoring only on the first step landed wrong, and inconsistently. It now re-anchors on every step and once more when the size stops changing, which is safe to repeat only because a DOM anchor lands in the same place every time. A rotation must not rewrite the anchor. Which element begins a page depends on the page's shape, so re-reading the anchor in the orientation you are only passing through replaces it with an earlier one and the return leg honours that. Readium reports the restored position about half a second after the jump, which arrives as an ordinary position update, so the anchor is held across that window. Verified on an emulator against a book whose every sentence carries its own paragraph marker, comparing screenshots. With full-screen reading off, ten starting positions over two round trips each, plus eight consecutive round trips from one position: every portrait screenshot byte-identical to the one before rotating, and no accumulation. With full-screen reading on, four positions over two round trips each, likewise identical. The already-fixed refocus behaviour still holds: three background-and-resume cycles in each mode leave the page untouched. The saved reading position now carries the DOM anchor too, so reopening a book lands on the exact page rather than near it. --- .../ereader/ui/reader/ReaderScreen.kt | 11 +- .../ereader/ui/reader/ReaderViewModel.kt | 133 ++++++++++++- .../ui/reader/ReaderViewportResizeTest.kt | 176 ++++++++++++++++++ .../ereader/reader/PageStartAnchor.kt | 114 ++++++++++++ .../theficos/ereader/reader/ResizeAnchor.kt | 46 +++++ .../ereader/reader/PageStartAnchorTest.kt | 64 +++++++ .../ereader/reader/ResizeAnchorTest.kt | 91 +++++++++ 7 files changed, 620 insertions(+), 15 deletions(-) create mode 100644 reader/src/main/java/io/theficos/ereader/reader/PageStartAnchor.kt create mode 100644 reader/src/main/java/io/theficos/ereader/reader/ResizeAnchor.kt create mode 100644 reader/src/test/java/io/theficos/ereader/reader/PageStartAnchorTest.kt create mode 100644 reader/src/test/java/io/theficos/ereader/reader/ResizeAnchorTest.kt diff --git a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt index cff45bc..560aeef 100644 --- a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt +++ b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt @@ -141,10 +141,11 @@ fun ReaderScreen(viewModel: ReaderViewModel, onClose: () -> Unit) { // Last measured size of the reader content. Every change is reported to the // view model, which re-anchors the reading position across it whatever caused - // it — see ReaderViewModel.onViewportChanged. The settle valve is keyed on the - // size so a resize that arrives in several steps (an inset animation, say) - // restarts the wait instead of completing mid-flight; Readium's onPageChanged - // normally beats it to the re-anchor and makes it a no-op. + // it — see ReaderViewModel.onViewportChanged. This is keyed on the size so a + // resize arriving in several steps (an inset animation, say) restarts the wait + // instead of completing mid-flight. Readium's onPageChanged re-anchors as it + // re-paginates; this has the last word, once the size has stopped moving and + // the WebView's page grid is final. var viewport by remember { mutableStateOf(IntSize.Zero) } LaunchedEffect(viewport) { if (viewport != IntSize.Zero) { @@ -177,7 +178,7 @@ fun ReaderScreen(viewModel: ReaderViewModel, onClose: () -> Unit) { onPrev = viewModel::pageBackward, onNext = viewModel::pageForward, onToggleChrome = viewModel::toggleChrome, - onPageLoaded = viewModel::completeViewportResize, + onPageLoaded = viewModel::reanchorViewport, onViewportChanged = { size -> viewport = size viewModel.onViewportChanged(size.width, size.height) diff --git a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt index cca79df..2050892 100644 --- a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt +++ b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderViewModel.kt @@ -10,8 +10,11 @@ import io.theficos.ereader.reader.EpubAsset import io.theficos.ereader.reader.ProgressTracker import io.theficos.ereader.reader.ReaderPreferences import io.theficos.ereader.reader.ReaderPreferencesStore +import io.theficos.ereader.reader.PAGE_START_ANCHOR_JS import io.theficos.ereader.reader.ReadiumFactory import io.theficos.ereader.reader.locatorAtPercent +import io.theficos.ereader.reader.parsePageStartAnchor +import io.theficos.ereader.reader.resizeAnchor import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableSharedFlow @@ -34,6 +37,11 @@ class ReaderViewModel( private val progress: ProgressRepository, private val readium: ReadiumFactory, private val preferencesStore: ReaderPreferencesStore, + // How the reader asks Readium whereabouts in the DOM the current page starts. A seam so + // the re-anchor can be tested without a live WebView; see ReaderViewportResizeTest. + private val readDomAnchor: suspend (EpubNavigatorFragment?) -> Locator? = + ::readPageStartAnchor, + private val nowMs: () -> Long = System::currentTimeMillis, ) : ViewModel() { private val _state = MutableStateFlow(ReaderUiState.Loading) @@ -88,6 +96,7 @@ class ReaderViewModel( fun goTo(locator: Locator) { clearPendingResize() + invalidateDomAnchor() viewModelScope.launch { navigator?.go(locator, false) } } @@ -129,11 +138,40 @@ class ReaderViewModel( if (suppressLocatorPublishing) return _currentLocator.value = locator _locatorUpdates.tryEmit(locator) + refreshDomAnchor() + } + + /** + * Keeps [domAnchor] tracking the page on screen. + * + * Asking Readium costs a round trip into the WebView's JavaScript, which is far too slow to + * do at the moment a rotation starts — the answer would come back describing the page after + * re-pagination, which is the one thing it must not be. So it is kept warm instead, refreshed + * whenever Readium reports a settled position, and read synchronously when the resize arms. + * + * Both guards here exist to keep a rotation from rewriting the anchor, which is the failure + * that made rotation drift compound. Which element anchors a page depends on the page's + * shape: a tall portrait column starts several paragraphs where a short landscape one starts + * a single paragraph, so reading the anchor in the orientation the reader is only passing + * through replaces it with an earlier one, and the return leg dutifully honours that. Turning + * a page moves the reader; rotating the device does not. + */ + private fun refreshDomAnchor() { + if (nowMs() < anchorPinnedUntil) return + viewModelScope.launch { + val nav = navigator + val dom = runCatching { readDomAnchor(nav) }.getOrNull() + // A resize may have armed while that round trip was in flight, in which case the + // answer describes the re-paginated page. Drop it and keep the pre-resize anchor. + if (!suppressLocatorPublishing && nowMs() >= anchorPinnedUntil) domAnchor = dom + } } private var pendingRotationAnchor: Locator? = null private var suppressLocatorPublishing: Boolean = false private var viewportSize: Pair? = null + private var domAnchor: Locator? = null + private var anchorPinnedUntil: Long = 0L /** * Reports the reader viewport's measured size, on every layout pass. @@ -163,23 +201,66 @@ class ReaderViewModel( beginViewportResize() } - // Called from MainActivity.onBeforeReaderConfigChange — runs BEFORE the - // Activity dispatches the configuration change down to fragments. Snapshots - // the current locator into pendingRotationAnchor and gates publishLocator so - // Readium's post-resize drifted emissions cannot overwrite the anchor. + /** + * Forgets where the page on screen begins, because the reader is being sent elsewhere. + * + * [domAnchor] describes the page being left. Until Readium reports the one being arrived at, + * a resize landing in between would otherwise fold that stale element into the new locator + * and send the reader straight back to the page they just jumped away from. + */ + private fun invalidateDomAnchor() { + domAnchor = null + } + + // Called from MainActivity.onBeforeReaderConfigChange — runs BEFORE the Activity dispatches + // the configuration change down to fragments. Snapshots where the reader is into + // pendingRotationAnchor and gates publishLocator so Readium's post-resize drifted emissions + // cannot overwrite it. fun beginViewportResize() { - val anchor = _currentLocator.value ?: return - pendingRotationAnchor = anchor + // A single rotation arms this several times over — MainActivity as the configuration + // change is dispatched, then onViewportChanged as the measured size follows. Only the + // first of those still sees the page the reader is leaving, so it wins. + if (pendingRotationAnchor != null) return + val live = _currentLocator.value ?: return + pendingRotationAnchor = resizeAnchor(live, domAnchor) suppressLocatorPublishing = true } - // Called from the Readium PaginationListener when re-pagination completes - // after a viewport resize. No-op if no resize is pending (so it's safe to - // call on every onPageLoaded). Re-anchors via navigator.go(anchor, false), - // re-seeds the current-locator flow, and re-enables publishing. + /** + * Puts the reader back on the anchored page, and stays armed. Called from the Readium + * pagination listener on every re-pagination while a resize is in flight; a no-op when no + * resize is pending, so it is safe on every page turn. + * + * Re-pagination arrives in steps, and anchoring on the first one is not enough: wired that + * way, and with everything else here unchanged, rotation still landed on the wrong page and + * did so inconsistently, the same start position coming back two different ways. Readium + * scrolls to an anchor by snapping the element's offset to a page boundary, using a page + * width it caches in JavaScript and only recomputes when it is told the viewport moved, so + * an early re-anchor is measuring against a grid that is still the old one. Re-anchoring on + * every step and once more when the size stops changing ([completeViewportResize]) gives the + * settled layout the last word. + * + * Repeating this is only safe because the anchor is a DOM element: it lands in the same place + * however many times it is used. The progression fraction this used to carry was consumed a + * little by every application, which is why it could only ever be applied once. + */ + fun reanchorViewport() { + val anchor = pendingRotationAnchor ?: return + viewModelScope.launch { navigator?.go(anchor, false) } + } + + // Called when the viewport has stopped changing (and from the immersive transition's own + // settle timer). No-op if no resize is pending. Re-anchors one last time, now that the + // layout is final, re-seeds the current-locator flow and re-enables publishing. fun completeViewportResize() { val anchor = pendingRotationAnchor ?: return pendingRotationAnchor = null + // Readium reports the restored position of its own accord a moment after the jump — + // measured at around half a second. That report is the resize finishing, not the reader + // moving, so it must not be allowed to re-read the anchor from the page it just landed + // on. Hold the anchor over that window; the next page the reader actually turns to + // refreshes it normally. + anchorPinnedUntil = nowMs() + ANCHOR_PIN_MS viewModelScope.launch { navigator?.go(anchor, false) _currentLocator.value = anchor @@ -207,6 +288,7 @@ class ReaderViewModel( // An explicit jump supersedes any armed re-anchor: the anchor predates the seek, so // honouring it afterwards would yank the reader back out of the page they just chose. clearPendingResize() + invalidateDomAnchor() // Surface the target on the HUD synchronously, before the suspending nav.go() // call dispatches. This avoids a one-frame window where the slider thumb // would snap back to the pre-seek liveLocator after the UI clears its drag @@ -229,6 +311,13 @@ class ReaderViewModel( } } +/** + * How long the reading anchor is held after a viewport resize completes, covering Readium's own + * delayed report of where it landed. Long enough for that report (about half a second in + * practice), short enough that a page the reader turns to just after a rotation still registers. + */ +private const val ANCHOR_PIN_MS = 1_500L + sealed interface ReaderUiState { data object Loading : ReaderUiState data class Error(val message: String) : ReaderUiState @@ -239,3 +328,27 @@ sealed interface ReaderUiState { val savedProgress: Progress?, ) : ReaderUiState } + +/** + * Asks the navigator where the page on screen begins in the DOM. + * + * Runs [PAGE_START_ANCHOR_JS] in the current resource's web view and stamps the answer with that + * resource's href, so a stale anchor can be rejected later if the reader has moved on to another + * chapter. Falls back to Readium's own `firstVisibleElementLocator()` whenever the script + * declines to answer: a page with nothing starting on it, or a layout the script bows out of. + * That fallback anchors slightly earlier than the reader actually is — the very thing the script + * exists to improve on — but it is still an exact DOM anchor, so it costs at most a one-off + * shift rather than the compounding walk the progression fraction caused. + */ +private suspend fun readPageStartAnchor(navigator: EpubNavigatorFragment?): Locator? { + val nav = navigator ?: return null + val current = nav.currentLocator.value + val fromScript = runCatching { + parsePageStartAnchor( + json = nav.evaluateJavascript(PAGE_START_ANCHOR_JS), + href = current.href, + mediaType = current.mediaType, + ) + }.getOrNull() + return fromScript ?: runCatching { nav.firstVisibleElementLocator() }.getOrNull() +} diff --git a/app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt b/app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt index 42dbfba..918f7a6 100644 --- a/app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt +++ b/app/src/test/java/io/theficos/ereader/ui/reader/ReaderViewportResizeTest.kt @@ -8,6 +8,7 @@ import io.theficos.ereader.data.local.ProgressRepository import io.theficos.ereader.data.local.db.EReaderDatabase import io.theficos.ereader.reader.ReaderPreferencesStore import io.theficos.ereader.reader.ReadiumFactory +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -18,6 +19,7 @@ import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.readium.r2.navigator.epub.EpubNavigatorFragment import org.readium.r2.shared.publication.Locator import org.readium.r2.shared.util.Url import org.readium.r2.shared.util.mediatype.MediaType @@ -138,6 +140,180 @@ class ReaderViewportResizeTest { assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.6) } + /** + * A [ReaderViewModel] whose DOM anchors come from [anchors] in order, the last one repeating. + * Standing in for a JavaScript round trip into Readium's WebView, which a unit test has no + * WebView for. + */ + /** Test clock, in milliseconds; advance it to step past the post-resize anchor hold. */ + private var clock = 0L + + private fun vmServing( + vararg anchors: Locator, + /** Held open from the second read onwards, to park one in flight across a rotation. */ + gate: CompletableDeferred? = null, + ): ReaderViewModel { + val context = ApplicationProvider.getApplicationContext() + var call = 0 + return ReaderViewModel( + documentId = 1L, + docs = DocumentRepository(db.documentDao()), + progress = ProgressRepository(db.progressDao()), + readium = ReadiumFactory(context), + preferencesStore = ReaderPreferencesStore(context), + readDomAnchor = { _: EpubNavigatorFragment? -> + if (call > 0) gate?.await() + anchors[minOf(call++, anchors.size - 1)] + }, + nowMs = { clock }, + ) + } + + private fun domAnchor(nth: Int) = Locator( + href = Url("ch1.xhtml")!!, + mediaType = MediaType.XHTML, + locations = Locator.Locations( + otherLocations = mapOf("cssSelector" to ":root > :nth-child(2) > :nth-child($nth)"), + ), + text = Locator.Text(highlight = "Paragraph $nth."), + ) + + private val Locator?.selector: String? + get() = this?.locations?.otherLocations?.get("cssSelector") as? String + + @Test fun `the anchor carries a DOM location, not just a progression`() = runTest { + val vm = vmServing(domAnchor(23)) + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + vm.onViewportChanged(2424, 1080) + vm.completeViewportResize() + + // Readium restores a locator precisely only when it carries text to match; without it, + // it maps the progression onto the new page grid and lands wherever that arithmetic + // happens to point. This is the whole fix: rotation must hand it a DOM location. + assertThat(vm.currentLocator.value?.text?.highlight).isEqualTo("Paragraph 23.") + assertThat(vm.currentLocator.value.selector) + .isEqualTo(":root > :nth-child(2) > :nth-child(23)") + // ...while still carrying the position it left, for Readium's own fallback and for the + // percentage the HUD and the progress row read. + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.5) + } + + @Test fun `a rotation does not move the anchor, so returning is exact`() = runTest { + // Which element "starts the page" depends on the shape of the page: a tall portrait + // column starts several paragraphs where a short landscape one starts a single + // paragraph. Re-reading the anchor while the reader is rotated therefore replaces it + // with an earlier one, and honouring that on the way back walks the reader backwards a + // page at a time. A rotation is not a change of reading position: the anchor must be + // whatever the reader was last actually on. + val vm = vmServing(domAnchor(23), domAnchor(22), domAnchor(18)) + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + + repeat(3) { + vm.onViewportChanged(2424, 1080) + vm.completeViewportResize() + vm.onViewportChanged(1080, 2424) + vm.completeViewportResize() + assertThat(vm.currentLocator.value.selector) + .isEqualTo(":root > :nth-child(2) > :nth-child(23)") + } + } + + @Test fun `Readium's own report of the restored page does not move the anchor`() = runTest { + // Readium reports where it landed of its own accord, about half a second after the + // jump. That arrives as an ordinary publish, so without a hold it re-reads the anchor + // from the page the reader is only rotating through — which is how the drift crept back + // in intermittently even once the anchor was no longer refreshed explicitly. + val vm = vmServing(domAnchor(23), domAnchor(31)) + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + + vm.onViewportChanged(2424, 1080) + vm.completeViewportResize() + clock += 500 + vm.publishLocator(locatorAt(0.39)) + + vm.onViewportChanged(1080, 2424) + vm.completeViewportResize() + assertThat(vm.currentLocator.value.selector) + .isEqualTo(":root > :nth-child(2) > :nth-child(23)") + } + + @Test fun `moving the reader does move the anchor`() = runTest { + // The other half of the same rule: the anchor has to follow the reader when the reader + // is the one moving, page turns and swipes alike, both of which arrive as a publish. + val vm = vmServing(domAnchor(23), domAnchor(31)) + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + // A rotation, and then — once its dust has settled — a page turn. + vm.onViewportChanged(2424, 1080) + vm.completeViewportResize() + clock += 5_000 + vm.publishLocator(locatorAt(0.6)) + + vm.onViewportChanged(1080, 2424) + vm.completeViewportResize() + assertThat(vm.currentLocator.value.selector) + .isEqualTo(":root > :nth-child(2) > :nth-child(31)") + } + + @Test fun `an anchor read while the reader is resizing is discarded`() = runTest { + // The read is a round trip into the WebView's JavaScript. If a rotation starts while one + // is in flight, the answer that comes back describes the re-paginated page — exactly the + // page the anchor exists to avoid. Keeping the older, pre-rotation one is right even + // though it is a page turn behind. + val gate = CompletableDeferred() + val vm = vmServing(domAnchor(23), domAnchor(31), gate = gate) + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + + // A page turn, whose read parks on the gate, and then a rotation on top of it. + vm.publishLocator(locatorAt(0.6)) + vm.onViewportChanged(2424, 1080) + gate.complete(Unit) + vm.completeViewportResize() + + // The next rotation is where a swallowed answer would show up. + vm.onViewportChanged(1080, 2424) + vm.completeViewportResize() + assertThat(vm.currentLocator.value.selector) + .isEqualTo(":root > :nth-child(2) > :nth-child(23)") + } + + @Test fun `re-anchoring mid-resize does not end the resize`() = runTest { + // Readium re-paginates in steps and the reader is put back on the anchor at each one, + // because an early step measures against a page grid that is still the old one. Those + // steps must not be mistaken for the end of the resize. + val vm = vmServing(domAnchor(23)) + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + vm.onViewportChanged(2424, 1080) + + vm.reanchorViewport() + vm.publishLocator(locatorAt(0.31)) + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.5) + + vm.completeViewportResize() + vm.publishLocator(locatorAt(0.55)) + assertThat(vm.currentLocator.value?.locations?.progression).isEqualTo(0.55) + } + + @Test fun `a jump forgets the anchor rather than dragging the reader back`() = runTest { + // The cached anchor describes the page being left. A rotation landing between the jump + // and Readium's report of where it arrived would otherwise fold that element into the + // new locator, and Readium would honour the element over the progression. + val vm = vmServing(domAnchor(23)) + vm.publishLocator(locatorAt(0.5)) + vm.onViewportChanged(1080, 2424) + + vm.goTo(locatorAt(0.9)) + vm.onViewportChanged(2424, 1080) + vm.completeViewportResize() + + assertThat(vm.currentLocator.value.selector).isNull() + } + @Test fun `seeking supersedes an armed re-anchor`() = runTest { vm.publishLocator(locatorAt(0.5)) vm.onViewportChanged(1080, 2424) diff --git a/reader/src/main/java/io/theficos/ereader/reader/PageStartAnchor.kt b/reader/src/main/java/io/theficos/ereader/reader/PageStartAnchor.kt new file mode 100644 index 0000000..a6406d8 --- /dev/null +++ b/reader/src/main/java/io/theficos/ereader/reader/PageStartAnchor.kt @@ -0,0 +1,114 @@ +package io.theficos.ereader.reader + +import org.json.JSONObject +import org.readium.r2.shared.publication.Locator +import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.mediatype.MediaType + +/** + * Finds the element that *begins* on the page currently on screen. + * + * Readium ships `readium.findFirstVisibleLocator()`, which returns the first element that is + * visible — anything whose box overlaps the page at all, including a paragraph that started + * two pages back and merely spills onto this one. That is the wrong end of the paragraph to + * anchor to, and it is wrong in a way that compounds: + * + * - going back to a locator scrolls to where its element *starts* (`scrollToRect` floors the + * element's offset to a page boundary), so restoring the first visible element lands on the + * page where that element started, one page or more before where the reader was; + * - the reader is then sitting on a page whose own first visible element is the one before + * that, so the next rotation anchors one element earlier again. + * + * Measured on the marker fixture, that walks backwards forever: anchor paragraph 11 landed on + * 10, then 7, 6, 4, 2, and finally the chapter heading, one rotation at a time. Capturing and + * restoring have to be inverses of each other, and they only are if the anchor is an element + * that starts on this page: restoring puts its start at the page's leading edge, which is this + * same page, and capturing there returns the same element. A fixed point. + * + * The script mirrors Readium's own walk — same `display: block` / `opacity: 0` skips, same + * descent to the deepest match — and changes one thing: an element qualifies when its box + * *begins* within the page (`left >= 0`) rather than merely reaching it (`right > 0`). + * Containers that only overlap are still descended into, because the element that opens the + * page is usually inside one. + * + * Returns `null`, and so leaves the caller on Readium's own answer, when nothing starts on this + * page (one paragraph filling it end to end), or when the layout is one this reasoning does not + * hold for: scrolled rather than paginated, or right-to-left, where the leading edge is the + * other side. + */ +const val PAGE_START_ANCHOR_JS = """ +(function () { + try { + if (typeof readium !== 'undefined' && readium.isFixedLayout) return null; + var de = document.scrollingElement || document.documentElement; + if (de.scrollHeight > de.clientHeight) return null; + if (getComputedStyle(document.documentElement).direction === 'rtl') return null; + var width = window.innerWidth; + function skip(el) { + var s = getComputedStyle(el); + if (!s) return false; + return s.getPropertyValue('display') !== 'block' || s.getPropertyValue('opacity') === '0'; + } + function opens(r) { return r.left >= -1 && r.left < width; } + function touches(r) { return r.right > 0 && r.left < width; } + function deepest(el) { + for (var i = 0; i < el.children.length; i++) { + var c = el.children[i]; + if (!skip(c) && opens(c.getBoundingClientRect())) return deepest(c); + } + return el; + } + function search(el) { + for (var i = 0; i < el.children.length; i++) { + var c = el.children[i]; + if (skip(c)) continue; + var r = c.getBoundingClientRect(); + if (opens(r)) return deepest(c); + if (touches(r)) { var f = search(c); if (f) return f; } + } + return null; + } + var el = search(document.body); + if (!el || !el.textContent) return null; + var parts = []; + for (var n = el; n && n.parentElement; n = n.parentElement) { + var i = Array.prototype.indexOf.call(n.parentElement.children, n) + 1; + parts.unshift(':nth-child(' + i + ')'); + } + return { cssSelector: [':root'].concat(parts).join(' > '), text: el.textContent }; + } catch (e) { + return null; + } +})(); +""" + +/** + * Turns what [PAGE_START_ANCHOR_JS] returned into the shape Readium restores from: a + * `cssSelector` under `locations`, and the element's text as `text.highlight`. + * + * Both fields matter and they do different jobs. Readium resolves this pair by looking the + * selector up to get a root and then matching the text inside it, so the selector is what makes + * it exact and the text is what makes `R2EpubPageFragment.loadLocator` choose that path at all — + * it only calls `scrollToLocator` when `text.highlight` is set. + * + * [href] and [mediaType] are the caller's, matching how Readium stamps its own + * `firstVisibleElementLocator()`: the script has no idea which resource it is running in. + * Returns `null` for the script's own `null`, for anything unparseable, and for a blank + * selector or text, none of which Readium could resolve. + */ +fun parsePageStartAnchor( + json: String?, + href: Url, + mediaType: MediaType, +): Locator? { + val raw = json?.trim()?.takeUnless { it.isEmpty() || it == "null" } ?: return null + val obj = runCatching { JSONObject(raw) }.getOrNull() ?: return null + val selector = obj.optString("cssSelector").takeUnless { it.isBlank() } ?: return null + val text = obj.optString("text").takeUnless { it.isBlank() } ?: return null + return Locator( + href = href, + mediaType = mediaType, + locations = Locator.Locations(otherLocations = mapOf("cssSelector" to selector)), + text = Locator.Text(highlight = text), + ) +} diff --git a/reader/src/main/java/io/theficos/ereader/reader/ResizeAnchor.kt b/reader/src/main/java/io/theficos/ereader/reader/ResizeAnchor.kt new file mode 100644 index 0000000..c421f12 --- /dev/null +++ b/reader/src/main/java/io/theficos/ereader/reader/ResizeAnchor.kt @@ -0,0 +1,46 @@ +package io.theficos.ereader.reader + +import org.readium.r2.shared.publication.Locator + +/** + * Builds the locator the reader goes back to after its viewport changes. + * + * Readium restores a locator in `R2EpubPageFragment.loadLocator()`, and what it does there + * depends entirely on what the locator carries: + * + * 1. `text.highlight != null` — `scrollToLocator()`: look the element up in the DOM by its + * CSS selector and scroll its box into view. Independent of how the text is paginated. + * 2. else `locations.htmlId` — `scrollToId()`: the same idea, by element id. + * 3. else — `item = round(progression * numPages)`, then `setCurrentItem(item)`. + * + * What Readium publishes on `currentLocator` never has the first two. It builds that locator + * from the EPUB positions service, which emits `Locations(progression, position)` and nothing + * else, so re-anchoring on it always took the third path: a fraction mapped onto a page grid. + * Rotation rebuilds that grid (in the fixture book, 12 portrait pages against 23 landscape + * ones) and the arithmetic runs while it is still being rebuilt, so the page it lands on is + * only loosely related to the page it left. Worse, Readium then publishes wherever it landed, + * that becomes the anchor for the next rotation, and the error walks: a real trace went + * paragraph 18 to 31 to 38 over three rotate-and-return trips. + * + * [dom] is where the page begins in the document, from [PAGE_START_ANCHOR_JS]: a `cssSelector` + * and the element's text, and no progression whatsoever. Handing that to Readium on its own + * would trade one bug for a worse one, because if the selector ever failed to resolve Readium + * would fall back to `progression ?: 0.0` and jump to the top of the chapter, and because the + * progress row and the HUD percentage both read their numbers off this same locator. + * + * So keep both. The DOM fields decide where to land; [live] keeps the fallback honest and the + * percentages unchanged. Falls back to [live] untouched whenever [dom] cannot be trusted: + * absent, textless (nothing for `scrollToLocator` to match), or pointing at another resource, + * which would be the dangerous case — selectors here are positional (`:nth-child(19)`), so one + * from a different chapter resolves happily against the wrong paragraph. + */ +fun resizeAnchor(live: Locator, dom: Locator?): Locator { + if (dom == null || dom.text.highlight.isNullOrBlank()) return live + if (dom.href != live.href) return live + return live.copy( + locations = live.locations.copy( + otherLocations = live.locations.otherLocations + dom.locations.otherLocations, + ), + text = dom.text, + ) +} diff --git a/reader/src/test/java/io/theficos/ereader/reader/PageStartAnchorTest.kt b/reader/src/test/java/io/theficos/ereader/reader/PageStartAnchorTest.kt new file mode 100644 index 0000000..8d56e84 --- /dev/null +++ b/reader/src/test/java/io/theficos/ereader/reader/PageStartAnchorTest.kt @@ -0,0 +1,64 @@ +package io.theficos.ereader.reader + +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.mediatype.MediaType +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Reading back what [PAGE_START_ANCHOR_JS] found. + * + * The script itself runs in Readium's WebView and is verified on a device; this covers the + * Kotlin side of the boundary, which has to survive the script declining to answer — it returns + * `null` for a page nothing starts on, and for layouts it bows out of — without turning that + * into an anchor Readium cannot resolve. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [33]) +class PageStartAnchorTest { + + private val chapter = Url("ch1.xhtml")!! + + private fun parse(json: String?) = parsePageStartAnchor(json, chapter, MediaType.XHTML) + + @Test fun `builds the locator shape Readium restores from`() { + val anchor = parse( + """{"cssSelector":":root > :nth-child(2) > :nth-child(23)","text":"Paragraph 22."}""" + ) + + assertThat(anchor).isNotNull() + assertThat(anchor!!.locations.otherLocations["cssSelector"]) + .isEqualTo(":root > :nth-child(2) > :nth-child(23)") + assertThat(anchor.text.highlight).isEqualTo("Paragraph 22.") + // The script has no idea which resource it is running in; the caller supplies that, and + // it is what later lets a stale anchor from another chapter be rejected. + assertThat(anchor.href).isEqualTo(chapter) + assertThat(anchor.mediaType).isEqualTo(MediaType.XHTML) + } + + @Test fun `declines the script's own null`() { + // What the script returns for a page with nothing starting on it, and for a scrolled or + // right-to-left layout. The caller falls back to Readium's own answer. + assertThat(parse("null")).isNull() + assertThat(parse(null)).isNull() + assertThat(parse("")).isNull() + assertThat(parse(" ")).isNull() + } + + @Test fun `declines anything unparseable rather than throwing`() { + assertThat(parse("{oops")).isNull() + assertThat(parse("\"a string\"")).isNull() + } + + @Test fun `declines a half-built anchor`() { + // Readium needs both halves: no selector means it would search the whole document for + // the text, no text means it never takes the precise path at all. + assertThat(parse("""{"text":"Paragraph 22."}""")).isNull() + assertThat(parse("""{"cssSelector":":root > :nth-child(2)"}""")).isNull() + assertThat(parse("""{"cssSelector":"","text":"Paragraph 22."}""")).isNull() + assertThat(parse("""{"cssSelector":":root","text":" "}""")).isNull() + } +} diff --git a/reader/src/test/java/io/theficos/ereader/reader/ResizeAnchorTest.kt b/reader/src/test/java/io/theficos/ereader/reader/ResizeAnchorTest.kt new file mode 100644 index 0000000..a983a7e --- /dev/null +++ b/reader/src/test/java/io/theficos/ereader/reader/ResizeAnchorTest.kt @@ -0,0 +1,91 @@ +package io.theficos.ereader.reader + +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.readium.r2.shared.publication.Locator +import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.mediatype.MediaType +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * What the reader goes back to after its viewport changes. + * + * The stakes are in [resizeAnchor]'s own documentation: Readium only restores a locator + * precisely when it carries `text.highlight`, and the locator Readium itself publishes never + * does, so a locator that reaches Readium without those fields is restored by mapping a + * fraction onto a page grid — the behaviour that walked the reader 20 paragraphs down the + * chapter over three rotations. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [33]) +class ResizeAnchorTest { + + private val chapter = Url("ch1.xhtml")!! + + private fun live(progression: Double = 0.42) = Locator( + href = chapter, + mediaType = MediaType.XHTML, + title = "Chapter 1", + locations = Locator.Locations( + progression = progression, + position = 9, + totalProgression = 0.084, + ), + ) + + private fun dom( + href: Url = chapter, + selector: String? = ":root > :nth-child(2) > :nth-child(23)", + highlight: String? = "Chapter 1 paragraph 22 sentence 1", + ) = Locator( + href = href, + mediaType = MediaType.XHTML, + locations = Locator.Locations( + otherLocations = selector?.let { mapOf("cssSelector" to it) } ?: emptyMap(), + ), + text = Locator.Text(highlight = highlight), + ) + + @Test fun `carries the DOM fields Readium needs to restore precisely`() { + val anchor = resizeAnchor(live(), dom()) + + // Both are load-bearing: the highlight is what makes Readium choose scrollToLocator at + // all, and the selector is what makes that landing exact. + assertThat(anchor.text.highlight).isEqualTo("Chapter 1 paragraph 22 sentence 1") + assertThat(anchor.locations.otherLocations["cssSelector"]) + .isEqualTo(":root > :nth-child(2) > :nth-child(23)") + } + + @Test fun `keeps the live progression, so the fallback and the percentages still work`() { + val anchor = resizeAnchor(live(progression = 0.42), dom()) + + // If the selector ever fails to resolve, Readium falls back to `progression ?: 0.0` and + // would otherwise jump to the top of the chapter. These same fields are what the HUD + // percentage and the saved progress row are read from. + assertThat(anchor.locations.progression).isEqualTo(0.42) + assertThat(anchor.locations.totalProgression).isEqualTo(0.084) + assertThat(anchor.locations.position).isEqualTo(9) + assertThat(anchor.href).isEqualTo(chapter) + assertThat(anchor.title).isEqualTo("Chapter 1") + } + + @Test fun `falls back to the live locator when there is no DOM anchor`() { + assertThat(resizeAnchor(live(), null)).isEqualTo(live()) + } + + @Test fun `falls back when the DOM anchor has no text for Readium to match`() { + // Without a highlight Readium never reaches scrollToLocator, so a selector alone would + // be dead weight on the locator. + assertThat(resizeAnchor(live(), dom(highlight = null))).isEqualTo(live()) + assertThat(resizeAnchor(live(), dom(highlight = " "))).isEqualTo(live()) + } + + @Test fun `refuses a DOM anchor from another resource`() { + // The dangerous case. These selectors are positional, so one captured in a different + // chapter resolves perfectly happily against whatever paragraph sits at that index here. + val strayChapter = resizeAnchor(live(), dom(href = Url("ch2.xhtml")!!)) + assertThat(strayChapter).isEqualTo(live()) + } +}