Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent reporting the current position while loading a locator #487

Merged
merged 5 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -362,7 +364,9 @@ internal class R2EpubPageFragment : Fragment() {
}
.also { pendingLocator = null }

webView.listener?.onPageLoaded()
link?.let {
webView.listener?.onPageLoaded(webView, it)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading