Prevent reporting the current position while loading a locator (#487)

Author: mickael-menu

readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt

@@ -54,8 +54,12 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV
 
     interface Listener {
         val readingProgression: ReadingProgression
-        fun onResourceLoaded(link: Link?, webView: R2BasicWebView, url: String?) {}
-        fun onPageLoaded() {}
+
+        /** Called when the resource content is loaded in the web view. */
+        fun onResourceLoaded(webView: R2BasicWebView, link: Link) {}
+
+        /** Called when the target page of the resource is loaded in the web view. */
+        fun onPageLoaded(webView: R2BasicWebView, link: Link) {}
         fun onPageChanged(pageIndex: Int, totalPages: Int, url: String) {}
         fun onPageEnded(end: Boolean) {}
         fun onTap(point: PointF): Boolean = false

readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt

@@ -265,6 +265,19 @@ public class EpubNavigatorFragment internal constructor(
 
     public interface Listener : OverflowableNavigator.Listener, HyperlinkNavigator.Listener
 
+    private sealed class State {
+        /** The navigator just started and didn't load any resource yet. */
+        object Initializing : State()
+
+        /** The navigator is jumping to the resource at `locator`. */
+        data class Loading(val locator: Locator) : State()
+
+        /** The navigator is idle and ready for interactions. */
+        object Ready : State()
+    }
+
+    private var state: State = State.Initializing
+
     // Configurable
 
     override val settings: StateFlow<EpubSettings> get() = viewModel.settings
@@ -595,6 +608,8 @@ public class EpubNavigatorFragment internal constructor(
         @Suppress("NAME_SHADOWING")
         val locator = publication.normalizeLocator(locator)
 
+        state = State.Loading(locator)
+
         listener?.onJumpToLocator(locator)
 
         val href = locator.href.removeFragment()
@@ -759,12 +774,17 @@ public class EpubNavigatorFragment internal constructor(
         override val readingProgression: ReadingProgression
             get() = viewModel.readingProgression
 
-        override fun onResourceLoaded(link: Link?, webView: R2BasicWebView, url: String?) {
-            run(viewModel.onResourceLoaded(link, webView))
+        override fun onResourceLoaded(webView: R2BasicWebView, link: Link) {
+            run(viewModel.onResourceLoaded(webView, link))
         }
 
-        override fun onPageLoaded() {
+        override fun onPageLoaded(webView: R2BasicWebView, link: Link) {
             paginationListener?.onPageLoaded()
+
+            if (state is State.Initializing || (state as? State.Loading)?.locator?.href == link.url()) {
+                state = State.Ready
+            }
+
             notifyCurrentLocation()
         }
 
@@ -1028,7 +1048,9 @@ public class EpubNavigatorFragment internal constructor(
         debounceLocationNotificationJob = viewLifecycleOwner.lifecycleScope.launch {
             delay(100L)
 
-            if (currentReflowablePageFragment?.isLoaded?.value == false) {
+            // We don't want to notify the current location if the navigator is still loading a
+            // locator, to avoid notifying intermediate locations.
+            if (currentReflowablePageFragment?.isLoaded?.value == false || state != State.Ready) {
                 return@launch
             }
 

readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorViewModel.kt

@@ -140,20 +140,18 @@ internal class EpubNavigatorViewModel(
             .launchIn(viewModelScope)
     }
 
-    fun onResourceLoaded(link: Link?, webView: R2BasicWebView): RunScriptCommand {
+    fun onResourceLoaded(webView: R2BasicWebView, link: Link): RunScriptCommand {
         val templates = decorationTemplates.toJSON().toString()
             .replace("\\n", " ")
         var script = "readium.registerDecorationTemplates($templates);\n"
 
-        if (link != null) {
-            for ((group, decorations) in decorations) {
-                val changes = decorations
-                    .filter { it.locator.href == link.url() }
-                    .map { DecorationChange.Added(it) }
+        for ((group, decorations) in decorations) {
+            val changes = decorations
+                .filter { it.locator.href == link.url() }
+                .map { DecorationChange.Added(it) }
 
-                val groupScript = changes.javascriptForGroup(group, decorationTemplates) ?: continue
-                script += "$groupScript\n"
-            }
+            val groupScript = changes.javascriptForGroup(group, decorationTemplates) ?: continue
+            script += "$groupScript\n"
         }
 
         return RunScriptCommand(script, scope = RunScriptCommand.Scope.WebView(webView))

readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2EpubPageFragment.kt

@@ -209,7 +209,9 @@ internal class R2EpubPageFragment : Fragment() {
             override fun onPageFinished(view: WebView?, url: String?) {
                 super.onPageFinished(view, url)
 
-                webView.listener?.onResourceLoaded(link, webView, url)
+                link?.let {
+                    webView.listener?.onResourceLoaded(webView, it)
+                }
 
                 // To make sure the page is properly laid out before jumping to the target locator,
                 // we execute a dummy JavaScript and wait for the callback result.
@@ -362,7 +364,9 @@ internal class R2EpubPageFragment : Fragment() {
                     }
                     .also { pendingLocator = null }
 
-                webView.listener?.onPageLoaded()
+                link?.let {
+                    webView.listener?.onPageLoaded(webView, it)
+                }
             }
         }
     }

readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2FXLPageFragment.kt

@@ -29,6 +29,7 @@ import org.readium.r2.navigator.epub.EpubNavigatorFragment
 import org.readium.r2.navigator.epub.EpubNavigatorViewModel
 import org.readium.r2.navigator.epub.fxl.R2FXLLayout
 import org.readium.r2.navigator.epub.fxl.R2FXLOnDoubleTapListener
+import org.readium.r2.shared.publication.Link
 import org.readium.r2.shared.util.Url
 
 internal class R2FXLPageFragment : Fragment() {
@@ -39,6 +40,12 @@ internal class R2FXLPageFragment : Fragment() {
     private val secondResourceUrl: Url?
         get() = BundleCompat.getParcelable(requireArguments(), "secondUrl", Url::class.java)
 
+    private val firstResourceLink: Link?
+        get() = BundleCompat.getParcelable(requireArguments(), "firstLink", Link::class.java)
+
+    private val secondResourceLink: Link?
+        get() = BundleCompat.getParcelable(requireArguments(), "secondLink", Link::class.java)
+
     private var webViews = mutableListOf<R2BasicWebView>()
 
     private var _doubleBinding: ReadiumNavigatorFragmentFxllayoutDoubleBinding? = null
@@ -75,8 +82,8 @@ internal class R2FXLPageFragment : Fragment() {
             val left = doubleBinding.firstWebView
             val right = doubleBinding.secondWebView
 
-            setupWebView(left, firstResourceUrl)
-            setupWebView(right, secondResourceUrl)
+            setupWebView(left, firstResourceLink, firstResourceUrl)
+            setupWebView(right, secondResourceLink, secondResourceUrl)
 
             r2FXLLayout.addOnDoubleTapListener(R2FXLOnDoubleTapListener(true))
             r2FXLLayout.addOnTapListener(object : R2FXLLayout.OnTapListener {
@@ -100,7 +107,7 @@ internal class R2FXLPageFragment : Fragment() {
 
             val webview = singleBinding.webViewSingle
 
-            setupWebView(webview, firstResourceUrl)
+            setupWebView(webview, firstResourceLink, firstResourceUrl)
 
             r2FXLLayout.addOnDoubleTapListener(R2FXLOnDoubleTapListener(true))
             r2FXLLayout.addOnTapListener(object : R2FXLLayout.OnTapListener {
@@ -136,7 +143,7 @@ internal class R2FXLPageFragment : Fragment() {
     }
 
     @SuppressLint("SetJavaScriptEnabled")
-    private fun setupWebView(webView: R2BasicWebView, resourceUrl: Url?) {
+    private fun setupWebView(webView: R2BasicWebView, link: Link?, resourceUrl: Url?) {
         webViews.add(webView)
         navigator?.let {
             webView.listener = it.webViewListener
@@ -167,6 +174,15 @@ internal class R2FXLPageFragment : Fragment() {
 
             override fun shouldInterceptRequest(view: WebView, request: WebResourceRequest): WebResourceResponse? =
                 (webView as? R2BasicWebView)?.shouldInterceptRequest(view, request)
+
+            override fun onPageFinished(view: WebView?, url: String?) {
+                super.onPageFinished(view, url)
+
+                if (link != null) {
+                    webView.listener?.onResourceLoaded(webView, link)
+                    webView.listener?.onPageLoaded(webView, link)
+                }
+            }
         }
         webView.isHapticFeedbackEnabled = false
         webView.isLongClickable = false
@@ -179,11 +195,13 @@ internal class R2FXLPageFragment : Fragment() {
 
     companion object {
 
-        fun newInstance(url: Url?, url2: Url? = null): R2FXLPageFragment =
+        fun newInstance(left: Pair<Link, Url>?, right: Pair<Link, Url>? = null): R2FXLPageFragment =
             R2FXLPageFragment().apply {
                 arguments = Bundle().apply {
-                    putParcelable("firstUrl", url)
-                    putParcelable("secondUrl", url2)
+                    putParcelable("firstLink", left?.first)
+                    putParcelable("firstUrl", left?.second)
+                    putParcelable("secondLink", right?.first)
+                    putParcelable("secondUrl", right?.second)
                 }
             }
     }

readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2PagerAdapter.kt

@@ -16,6 +16,7 @@ import androidx.collection.LongSparseArray
 import androidx.collection.forEach
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.FragmentManager
+import org.readium.r2.navigator.extensions.let
 import org.readium.r2.shared.publication.Link
 import org.readium.r2.shared.publication.Locator
 import org.readium.r2.shared.util.AbsoluteUrl
@@ -83,7 +84,10 @@ internal class R2PagerAdapter internal constructor(
                 )
             }
             is PageResource.EpubFxl -> {
-                R2FXLPageFragment.newInstance(resource.leftUrl, resource.rightUrl)
+                R2FXLPageFragment.newInstance(
+                    left = let(resource.leftLink, resource.leftUrl) { l, u -> Pair(l, u) },
+                    right = let(resource.rightLink, resource.rightUrl) { l, u -> Pair(l, u) }
+                )
             }
             is PageResource.Cbz -> {
                 fm.fragmentFactory

readium/shared/src/main/java/org/readium/r2/shared/publication/Publication.kt

@@ -15,7 +15,6 @@ import kotlin.reflect.KClass
 import kotlinx.parcelize.Parcelize
 import org.json.JSONObject
 import org.readium.r2.shared.*
-import org.readium.r2.shared.extensions.*
 import org.readium.r2.shared.publication.epub.listOfAudioClips
 import org.readium.r2.shared.publication.epub.listOfVideoClips
 import org.readium.r2.shared.publication.services.CacheService