From 4d7874a4d509a8448b0a63609a02e6b6f35283a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Mon, 2 Oct 2023 10:04:24 +0200 Subject: [PATCH 1/5] Helper to migrate legacy HREFs (#399) --- CHANGELOG.md | 4 +- docs/migration-guide.md | 49 +++++++++++++++++++ .../readium/r2/shared/publication/Locator.kt | 35 ++++++++++++- .../java/org/readium/r2/shared/util/Url.kt | 15 +++++- .../r2/shared/publication/LocatorTest.kt | 21 ++++++++ .../org/readium/r2/shared/util/UrlTest.kt | 42 +++++++++++++++- 6 files changed, 161 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35c58b7539..3a4282358f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,8 +25,8 @@ All notable changes to this project will be documented in this file. Take a look ### Changed * Readium resources are now prefixed with `readium_`. Take care of updating any overridden resource by following [the migration guide](docs/migration-guide.md#300). -* `Link` and `Locator`'s `href` do not start with a `/` for packaged publications anymore. - * To ensure backward-compatibility, `href` starting with a `/` are still supported. But you may want to update the locators persisted in your database to drop the `/` prefix for packaged publications. +* `Link` and `Locator`'s `href` are normalized as valid URLs to improve interoperability with the Readium Web toolkits. + * **You MUST migrate your database if you were persisting HREFs and Locators**. Take a look at [the migration guide](docs/migration-guide.md) for guidance. #### Shared diff --git a/docs/migration-guide.md b/docs/migration-guide.md index a6fb823fd3..ec4d56261c 100644 --- a/docs/migration-guide.md +++ b/docs/migration-guide.md @@ -29,6 +29,55 @@ dependencies { } ``` +### Migration of HREFs and Locators (bookmarks, annotations, etc.) + +:warning: This requires a database migration in your application, if you were persisting `Locator` objects. + +In Readium v2.x, a `Link` or `Locator`'s `href` could be either: + +* a valid absolute URL for a streamed publication, e.g. `https://domain.com/isbn/dir/my%20chapter.html`, +* a percent-decoded path for a local archive such as an EPUB, e.g. `/dir/my chapter.html`. + * Note that it was relative to the root of the archive (`/`). + +To improve the interoperability with other Readium toolkits (in particular the Readium Web Toolkits, which only work in a streaming context) **Readium v3 now generates and expects valid URLs** for `Locator` and `Link`'s `href`. + +* `https://domain.com/isbn/dir/my%20chapter.html` is left unchanged, as it was already a valid URL. +* `/dir/my chapter.html` becomes the relative URL path `dir/my%20chapter.html` + * We dropped the `/` prefix to avoid issues when resolving to a base URL. + * Special characters are percent-encoded. + +**You must migrate the HREFs or Locators stored in your database** when upgrading to Readium 3. To assist you, two helpers are provided: `Url.fromLegacyHref()` and `Locator.fromLegacyJSON()`. + +Here's an example of a Jetpack Room migration that can serve as inspiration: + +```kotlin +val MIGRATION_HREF = object : Migration(1, 2) { + + override fun migrate(db: SupportSQLiteDatabase) { + val normalizedHrefs: Map = buildMap { + db.query("SELECT id, href FROM bookmarks").use { cursor -> + while (cursor.moveToNext()) { + val id = cursor.getLong(0) + val href = cursor.getString(1) + + val normalizedHref = Url.fromLegacyHref(href)?.toString() + if (normalizedHref != null) { + put(id, normalizedHref) + } + } + } + } + + val stmt = db.compileStatement("UPDATE bookmarks SET href = ? WHERE id = ?") + for ((id, href) in normalizedHrefs) { + stmt.bindString(1, href) + stmt.bindLong(2, id) + stmt.executeUpdateDelete() + } + } +} +``` + ### All resources now have the prefix `readium_`. To avoid conflicts when merging your app resources, all resources declared in the Readium toolkit now have the prefix `readium_`. This means that you must rename any layouts or strings you have overridden. Here is a comprehensive list of the changes. diff --git a/readium/shared/src/main/java/org/readium/r2/shared/publication/Locator.kt b/readium/shared/src/main/java/org/readium/r2/shared/publication/Locator.kt index 556f182db1..c055dd8b6b 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/publication/Locator.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/publication/Locator.kt @@ -14,6 +14,7 @@ import kotlinx.parcelize.Parcelize import kotlinx.parcelize.WriteWith import org.json.JSONArray import org.json.JSONObject +import org.readium.r2.shared.DelicateReadiumApi import org.readium.r2.shared.JSONable import org.readium.r2.shared.extensions.* import org.readium.r2.shared.toJSON @@ -197,10 +198,36 @@ public data class Locator( public companion object { + /** + * Creates a [Locator] from its JSON representation. + */ public fun fromJSON( json: JSONObject?, mediaTypeRetriever: MediaTypeRetriever = MediaTypeRetriever(), warnings: WarningLogger? = null + ): Locator? = + fromJSON(json, mediaTypeRetriever, warnings, withLegacyHref = false) + + /** + * Creates a [Locator] from its legacy JSON representation. + * + * Only use this API when you are upgrading to Readium 3.x and migrating the [Locator] + * objects stored in your database. See the migration guide for more information. + */ + @DelicateReadiumApi + public fun fromLegacyJSON( + json: JSONObject?, + mediaTypeRetriever: MediaTypeRetriever = MediaTypeRetriever(), + warnings: WarningLogger? = null + ): Locator? = + fromJSON(json, mediaTypeRetriever, warnings, withLegacyHref = true) + + @OptIn(DelicateReadiumApi::class) + private fun fromJSON( + json: JSONObject?, + mediaTypeRetriever: MediaTypeRetriever = MediaTypeRetriever(), + warnings: WarningLogger? = null, + withLegacyHref: Boolean = false ): Locator? { val href = json?.optNullableString("href") val type = json?.optNullableString("type") @@ -209,7 +236,13 @@ public data class Locator( return null } - val url = Url(href) ?: run { + val url = ( + if (withLegacyHref) { + Url.fromLegacyHref(href) + } else { + Url(href) + } + ) ?: run { warnings?.log(Locator::class.java, "[href] is not a valid URL", json) return null } diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt b/readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt index 615d83d9b1..bd67a5960a 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt @@ -13,6 +13,7 @@ import java.io.File import java.net.URI import java.net.URL import kotlinx.parcelize.Parcelize +import org.readium.r2.shared.DelicateReadiumApi import org.readium.r2.shared.InternalReadiumApi import org.readium.r2.shared.extensions.percentEncodedPath import org.readium.r2.shared.extensions.tryOrNull @@ -42,6 +43,19 @@ public sealed class Url : Parcelable { return invoke(Uri.parse(url)) } + /** + * Creates an [Url] from a legacy HREF. + * + * For example, if it is a relative path such as `/dir/my chapter.html`, it will be + * converted to the valid relative URL `dir/my%20chapter.html`. + * + * Only use this API when you are upgrading to Readium 3.x and migrating the HREFs stored in + * your database. See the 3.0 migration guide for more information. + */ + @DelicateReadiumApi + public fun fromLegacyHref(href: String): Url? = + AbsoluteUrl(href) ?: fromDecodedPath(href.removePrefix("/")) + internal operator fun invoke(uri: Uri): Url? = if (uri.isAbsolute) { AbsoluteUrl(uri) @@ -286,7 +300,6 @@ public class RelativeUrl private constructor(override val uri: Uri) : Url() { internal operator fun invoke(uri: Uri): RelativeUrl? = tryOrNull { require(uri.isRelative) - require(uri.isHierarchical) RelativeUrl(uri) } } diff --git a/readium/shared/src/test/java/org/readium/r2/shared/publication/LocatorTest.kt b/readium/shared/src/test/java/org/readium/r2/shared/publication/LocatorTest.kt index 846905d0eb..06ddbd0b1d 100644 --- a/readium/shared/src/test/java/org/readium/r2/shared/publication/LocatorTest.kt +++ b/readium/shared/src/test/java/org/readium/r2/shared/publication/LocatorTest.kt @@ -14,6 +14,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Test import org.junit.runner.RunWith +import org.readium.r2.shared.DelicateReadiumApi import org.readium.r2.shared.assertJSONEquals import org.readium.r2.shared.util.Url import org.readium.r2.shared.util.mediatype.MediaType @@ -71,6 +72,26 @@ class LocatorTest { assertNull(Locator.fromJSON(JSONObject("{ 'invalid': 'object' }"))) } + @OptIn(DelicateReadiumApi::class) + @Test + fun `parse {Locator} with legacy HREF`() { + val json = JSONObject( + """ + { + "href": "legacy href", + "type": "text/html" + } + """ + ) + + assertNull(Locator.fromJSON(json)) + + assertEquals( + Locator(href = Url("legacy%20href")!!, mediaType = MediaType.HTML), + Locator.fromLegacyJSON(json) + ) + } + @Test fun `get {Locator} minimal JSON`() { assertJSONEquals( JSONObject( diff --git a/readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt b/readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt index c0b5cf4f17..8b377b249f 100644 --- a/readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt +++ b/readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt @@ -4,10 +4,13 @@ import android.net.Uri import java.io.File import java.net.URI import java.net.URL +import kotlin.test.assertIs +import kotlin.test.assertNotNull import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Test import org.junit.runner.RunWith +import org.readium.r2.shared.DelicateReadiumApi import org.readium.r2.shared.util.Url.Query import org.readium.r2.shared.util.Url.QueryParameter import org.robolectric.RobolectricTestRunner @@ -34,8 +37,45 @@ class UrlTest { assertEquals(RelativeUrl(Uri.parse("foo/bar")), Url("foo/bar")) assertEquals(RelativeUrl(Uri.parse("../bar")), Url("../bar")) + // Special characters valid in a path. + assertEquals("$&+,/=@", RelativeUrl("$&+,/=@")?.path) + // Used in the EPUB parser - assertEquals(RelativeUrl(Uri.parse("#")), Url("#")) + val url = Url("#") as? RelativeUrl + assertNotNull(url) + assertEquals(null, url.path) + assertEquals(null, url.fragment) + } + + @OptIn(DelicateReadiumApi::class) + @Test + fun createFromLegacyHref() { + testLegacy("dir/chapter.xhtml", "dir/chapter.xhtml") + // Starting slash is removed. + testLegacy("/dir/chapter.xhtml", "dir/chapter.xhtml") + // Special characters are percent-encoded. + testLegacy("/dir/per%cent.xhtml", "dir/per%25cent.xhtml") + testLegacy("/barré.xhtml", "barr%C3%A9.xhtml") + testLegacy("/spa ce.xhtml", "spa%20ce.xhtml") + // We assume that a relative path is percent-decoded. + testLegacy("/spa%20ce.xhtml", "spa%2520ce.xhtml") + // Some special characters are authorized in a path. + testLegacy("/$&+,/=@", "$&+,/=@") + // Valid absolute URL are left untouched. + testLegacy( + "http://domain.com/a%20book?page=3", + "http://domain.com/a%20book?page=3" + ) + // Invalid absolute URL. + assertNull(Url.fromLegacyHref("http://domain.com/a book")) + } + + @OptIn(DelicateReadiumApi::class) + private inline fun testLegacy(href: String, expected: String) { + val url = Url.fromLegacyHref(href) + assertNotNull(url) + assertIs(url) + assertEquals(expected, url.toString()) } @Test From 9580cdd8a2f816ffbb266af67c10b442c0997f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Mon, 2 Oct 2023 11:47:40 +0200 Subject: [PATCH 2/5] Fix various crashes (#401) --- .../org/readium/r2/navigator/R2WebView.kt | 24 ++++++++++++++----- .../readium/r2/shared/asset/AssetRetriever.kt | 2 +- .../iterators/HtmlResourceContentIterator.kt | 13 ++++++---- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt index 4f3b9be446..509e768786 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt @@ -715,7 +715,7 @@ internal class R2WebView(context: Context, attrs: AttributeSet) : R2BasicWebView if (!isSelecting && !mIsBeingDragged) { mInitialVelocity = getCurrentXVelocity() val pointerIndex = ev.findPointerIndex(mActivePointerId) - val x = ev.getX(pointerIndex) + val x = ev.safeGetX(pointerIndex) val xDiff = abs(x - mLastMotionX) if (xDiff > mTouchSlop) { @@ -736,8 +736,8 @@ internal class R2WebView(context: Context, attrs: AttributeSet) : R2BasicWebView mHasAbortedScroller = false val activePointerIndex = ev.findPointerIndex(mActivePointerId) - val x = ev.getX(activePointerIndex) - val y = ev.getY(activePointerIndex) + val x = ev.safeGetX(activePointerIndex) + val y = ev.safeGetY(activePointerIndex) if (scrollMode) { val totalDelta = (y - mInitialMotionY).toInt() @@ -786,13 +786,13 @@ internal class R2WebView(context: Context, attrs: AttributeSet) : R2BasicWebView } MotionEvent.ACTION_POINTER_DOWN -> { val index = ev.actionIndex - val x = ev.getX(index) + val x = ev.safeGetX(index) mLastMotionX = x mActivePointerId = ev.getPointerId(index) } MotionEvent.ACTION_POINTER_UP -> { onSecondaryPointerUp(ev) - mLastMotionX = ev.getX(ev.findPointerIndex(mActivePointerId)) + mLastMotionX = ev.safeGetX(ev.findPointerIndex(mActivePointerId)) } } @@ -878,7 +878,7 @@ internal class R2WebView(context: Context, attrs: AttributeSet) : R2BasicWebView // This was our active pointer going up. Choose a new // active pointer and adjust accordingly. val newPointerIndex = if (pointerIndex == 0) 1 else 0 - mLastMotionX = ev.getX(newPointerIndex) + mLastMotionX = ev.safeGetX(newPointerIndex) mActivePointerId = ev.getPointerId(newPointerIndex) mVelocityTracker?.clear() } @@ -1092,3 +1092,15 @@ internal class R2WebView(context: Context, attrs: AttributeSet) : R2BasicWebView } } } + +/** + * May crash with java.lang.IllegalArgumentException: pointerIndex out of range + */ +private fun MotionEvent.safeGetX(pointerIndex: Int): Float = + try { getX(pointerIndex) } catch (e: IllegalArgumentException) { 0F } + +/** + * May crash with java.lang.IllegalArgumentException: pointerIndex out of range + */ +private fun MotionEvent.safeGetY(pointerIndex: Int): Float = + try { getY(pointerIndex) } catch (e: IllegalArgumentException) { 0F } diff --git a/readium/shared/src/main/java/org/readium/r2/shared/asset/AssetRetriever.kt b/readium/shared/src/main/java/org/readium/r2/shared/asset/AssetRetriever.kt index ffa426bfc1..2fbfafafd5 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/asset/AssetRetriever.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/asset/AssetRetriever.kt @@ -303,7 +303,7 @@ public class AssetRetriever( if (url.isContent) { val contentHints = MediaTypeHints( mediaType = contentResolver.getType(url.uri) - ?.let { MediaType(it)!! } + ?.let { MediaType(it) } ?.takeUnless { it.matches(MediaType.BINARY) }, fileExtension = contentResolver .queryProjection(url.uri, MediaStore.MediaColumns.DISPLAY_NAME) diff --git a/readium/shared/src/main/java/org/readium/r2/shared/publication/services/content/iterators/HtmlResourceContentIterator.kt b/readium/shared/src/main/java/org/readium/r2/shared/publication/services/content/iterators/HtmlResourceContentIterator.kt index 0e328ad298..f3ae6937fc 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/publication/services/content/iterators/HtmlResourceContentIterator.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/publication/services/content/iterators/HtmlResourceContentIterator.kt @@ -32,9 +32,10 @@ import org.readium.r2.shared.resource.Resource import org.readium.r2.shared.resource.readAsString import org.readium.r2.shared.util.Language import org.readium.r2.shared.util.Url -import org.readium.r2.shared.util.getOrThrow +import org.readium.r2.shared.util.getOrElse import org.readium.r2.shared.util.mediatype.MediaType import org.readium.r2.shared.util.use +import timber.log.Timber /** * Iterates an HTML [resource], starting from the given [locator]. @@ -147,7 +148,11 @@ public class HtmlResourceContentIterator internal constructor( private suspend fun parseElements(): ParsedElements { val document = resource.use { res -> - val html = res.readAsString().getOrThrow() + val html = res.readAsString().getOrElse { + Timber.w(it, "Failed to read HTML resource") + return ParsedElements() + } + Jsoup.parse(html) } @@ -206,8 +211,8 @@ public class HtmlResourceContentIterator internal constructor( * possible. Defaults to 0. */ public data class ParsedElements( - val elements: List, - val startIndex: Int + val elements: List = emptyList(), + val startIndex: Int = 0 ) private class ContentParser( From e66cf65eddbad9b6b6f724f6371e5856686f938b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Mon, 2 Oct 2023 12:57:15 +0200 Subject: [PATCH 3/5] Prevent out of memory errors when sniffing a media type (#400) --- .../r2/shared/resource/DefaultArchiveFactory.kt | 5 ++++- .../readium/r2/shared/resource/MediaTypeExt.kt | 17 +++++++++++++++-- .../util/archive/channel/ChannelZipContainer.kt | 3 ++- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/readium/shared/src/main/java/org/readium/r2/shared/resource/DefaultArchiveFactory.kt b/readium/shared/src/main/java/org/readium/r2/shared/resource/DefaultArchiveFactory.kt index 326cc428e4..5d38cf3b48 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/resource/DefaultArchiveFactory.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/resource/DefaultArchiveFactory.kt @@ -15,6 +15,9 @@ import org.readium.r2.shared.util.MessageError import org.readium.r2.shared.util.Try import org.readium.r2.shared.util.mediatype.MediaTypeRetriever +/** + * An [ArchiveFactory] to open local ZIP files with Java's [ZipFile]. + */ public class DefaultArchiveFactory( private val mediaTypeRetriever: MediaTypeRetriever ) : ArchiveFactory { @@ -28,7 +31,7 @@ public class DefaultArchiveFactory( ?.let { open(it) } ?: Try.Failure( ArchiveFactory.Error.FormatNotSupported( - MessageError("Resource not supported because file cannot be directly access.") + MessageError("Resource not supported because file cannot be directly accessed.") ) ) } diff --git a/readium/shared/src/main/java/org/readium/r2/shared/resource/MediaTypeExt.kt b/readium/shared/src/main/java/org/readium/r2/shared/resource/MediaTypeExt.kt index 96fb66bb7b..3d21057b9d 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/resource/MediaTypeExt.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/resource/MediaTypeExt.kt @@ -1,6 +1,7 @@ package org.readium.r2.shared.resource import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.getOrDefault import org.readium.r2.shared.util.mediatype.ContainerMediaTypeSnifferContent import org.readium.r2.shared.util.mediatype.ResourceMediaTypeSnifferContent @@ -9,7 +10,7 @@ public class ResourceMediaTypeSnifferContent( ) : ResourceMediaTypeSnifferContent { override suspend fun read(range: LongRange?): ByteArray? = - resource.read(range).getOrNull() + resource.safeRead(range) } public class ContainerMediaTypeSnifferContent( @@ -21,6 +22,18 @@ public class ContainerMediaTypeSnifferContent( override suspend fun read(path: String, range: LongRange?): ByteArray? = Url.fromDecodedPath(path)?.let { url -> - container.get(url).read(range).getOrNull() + container.get(url).safeRead(range) } } + +private suspend fun Resource.safeRead(range: LongRange?): ByteArray? { + try { + // We only read files smaller than 5MB to avoid an [OutOfMemoryError]. + if (range == null && length().getOrDefault(0) > 5 * 1000 * 1000) { + return null + } + return read(range).getOrNull() + } catch (e: OutOfMemoryError) { + return null + } +} diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/ChannelZipContainer.kt b/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/ChannelZipContainer.kt index bff4ac323a..d1b32efaae 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/ChannelZipContainer.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/ChannelZipContainer.kt @@ -170,7 +170,8 @@ internal class ChannelZipContainer( } /** - * An [ArchiveFactory] able to open a ZIP archive served through an HTTP server. + * An [ArchiveFactory] able to open a ZIP archive served through a stream (e.g. HTTP server, + * content URI, etc.). */ public class ChannelZipArchiveFactory( private val mediaTypeRetriever: MediaTypeRetriever From 4850dd33f19c345ac93ad6d5df0c130360492093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Mon, 2 Oct 2023 14:17:48 +0200 Subject: [PATCH 4/5] Fix crash with `ChannelZipContainer` before API 26 (#403) --- .../util/archive/channel/compress/utils/IOUtils.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/compress/utils/IOUtils.java b/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/compress/utils/IOUtils.java index 2911408782..fb4e243e11 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/compress/utils/IOUtils.java +++ b/readium/shared/src/main/java/org/readium/r2/shared/util/archive/channel/compress/utils/IOUtils.java @@ -27,7 +27,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.ByteBuffer; -import java.nio.file.LinkOption; /** * Utility functions @@ -38,13 +37,6 @@ public final class IOUtils { private static final int COPY_BUF_SIZE = 8024; private static final int SKIP_BUF_SIZE = 4096; - /** - * Empty array of type {@link LinkOption}. - * - * @since 1.21 - */ - public static final LinkOption[] EMPTY_LINK_OPTIONS = {}; - // This buffer does not need to be synchronized because it is write only; the contents are ignored // Does not affect Immutability private static final byte[] SKIP_BUF = new byte[SKIP_BUF_SIZE]; From e4f1cc4dfb6517b098920f4bce294b9fc5c42499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Tue, 3 Oct 2023 12:17:40 +0200 Subject: [PATCH 5/5] Fix configuration change crash with `PdfNavigatorFragment` (#398) --- .../pdfium/document/PdfiumDocument.kt | 21 +- .../navigator/PdfiumDocumentFragment.kt | 170 +++++++-------- .../pdfium/navigator/PdfiumEngineProvider.kt | 49 ++++- .../pdfium/navigator/PdfiumNavigator.kt | 17 ++ .../pspdfkit/document/PsPdfKitDocument.kt | 25 ++- .../navigator/PsPdfKitDocumentFragment.kt | 198 +++++++++++++----- .../navigator/PsPdfKitEngineProvider.kt | 55 +++-- .../pspdfkit/navigator/PsPdfKitNavigator.kt | 17 ++ .../org/readium/r2/navigator/Navigator.kt | 7 + .../navigator/epub/EpubNavigatorFragment.kt | 7 +- .../navigator/image/ImageNavigatorFragment.kt | 3 +- .../r2/navigator/input/InputListener.kt | 19 +- .../r2/navigator/input/KeyInterceptorView.kt | 5 +- .../r2/navigator/pdf/PdfEngineProvider.kt | 53 ++--- .../r2/navigator/pdf/PdfNavigatorFragment.kt | 162 ++++++-------- .../r2/navigator/pdf/PdfNavigatorViewModel.kt | 50 ++--- .../util/DirectionalNavigationAdapter.kt | 5 +- .../r2/navigator/util/FragmentFactory.kt | 23 +- .../org/readium/r2/shared/util/SingleJob.kt | 49 +++++ .../org/readium/r2/shared/util/cache/Cache.kt | 26 ++- .../readium/r2/shared/util/pdf/PdfDocument.kt | 21 +- .../r2/streamer/parser/pdf/PdfParser.kt | 4 + .../parser/readium/LcpdfPositionsService.kt | 15 +- .../org/readium/r2/testapp/Application.kt | 40 +++- .../r2/testapp/reader/EpubReaderFragment.kt | 4 +- .../r2/testapp/reader/ImageReaderFragment.kt | 8 +- .../r2/testapp/reader/PdfReaderFragment.kt | 24 +-- .../r2/testapp/reader/ReaderActivity.kt | 4 +- .../r2/testapp/reader/ReaderInitData.kt | 6 +- .../r2/testapp/reader/ReaderViewModel.kt | 25 ++- .../r2/testapp/reader/VisualReaderFragment.kt | 6 +- 31 files changed, 662 insertions(+), 456 deletions(-) create mode 100644 readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumNavigator.kt create mode 100644 readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitNavigator.kt create mode 100644 readium/shared/src/main/java/org/readium/r2/shared/util/SingleJob.kt diff --git a/readium/adapters/pdfium/pdfium-document/src/main/java/org/readium/adapters/pdfium/document/PdfiumDocument.kt b/readium/adapters/pdfium/pdfium-document/src/main/java/org/readium/adapters/pdfium/document/PdfiumDocument.kt index db93b01aaa..2474a98deb 100644 --- a/readium/adapters/pdfium/pdfium-document/src/main/java/org/readium/adapters/pdfium/document/PdfiumDocument.kt +++ b/readium/adapters/pdfium/pdfium-document/src/main/java/org/readium/adapters/pdfium/document/PdfiumDocument.kt @@ -19,7 +19,9 @@ import org.readium.r2.shared.InternalReadiumApi import org.readium.r2.shared.extensions.md5 import org.readium.r2.shared.extensions.tryOrNull import org.readium.r2.shared.resource.Resource -import org.readium.r2.shared.util.getOrThrow +import org.readium.r2.shared.resource.ResourceTry +import org.readium.r2.shared.resource.mapCatching +import org.readium.r2.shared.util.Try import org.readium.r2.shared.util.pdf.PdfDocument import org.readium.r2.shared.util.pdf.PdfDocumentFactory import org.readium.r2.shared.util.use @@ -84,24 +86,25 @@ public class PdfiumDocumentFactory(context: Context) : PdfDocumentFactory { // First try to open the resource as a file on the FS for performance improvement, as // PDFium requires the whole PDF document to be loaded in memory when using raw bytes. return resource.openAsFile(password) ?: resource.openBytes(password) } - private suspend fun Resource.openAsFile(password: String?): PdfiumDocument? = + private suspend fun Resource.openAsFile(password: String?): ResourceTry? = tryOrNull { - source?.toFile()?.let { open(it, password) } + source?.toFile()?.let { file -> + withContext(Dispatchers.IO) { + Try.success(core.fromFile(file, password)) + } + } } - private suspend fun Resource.openBytes(password: String?): PdfiumDocument = + private suspend fun Resource.openBytes(password: String?): ResourceTry = use { - core.fromBytes(read().getOrThrow(), password) + read().mapCatching { core.fromBytes(it, password) } } private fun PdfiumCore.fromFile(file: File, password: String?): PdfiumDocument = diff --git a/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumDocumentFragment.kt b/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumDocumentFragment.kt index 01c8dd3bde..2052830ae0 100644 --- a/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumDocumentFragment.kt +++ b/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumDocumentFragment.kt @@ -14,47 +14,39 @@ import android.view.ViewGroup import androidx.lifecycle.lifecycleScope import com.github.barteksc.pdfviewer.PDFView import kotlin.math.roundToInt -import kotlinx.coroutines.launch +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import org.readium.adapters.pdfium.document.PdfiumDocumentFactory import org.readium.r2.navigator.pdf.PdfDocumentFragment import org.readium.r2.navigator.preferences.Axis import org.readium.r2.navigator.preferences.Fit import org.readium.r2.navigator.preferences.ReadingProgression import org.readium.r2.shared.ExperimentalReadiumApi -import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Publication import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.util.SingleJob +import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.getOrElse import timber.log.Timber @ExperimentalReadiumApi public class PdfiumDocumentFragment internal constructor( private val publication: Publication, - private val link: Link, + private val href: Url, private val initialPageIndex: Int, - settings: PdfiumSettings, - private val appListener: Listener?, - private val navigatorListener: PdfDocumentFragment.Listener? + initialSettings: PdfiumSettings, + private val listener: Listener? ) : PdfDocumentFragment() { - public interface Listener { - /** Called when configuring [PDFView]. */ - public fun onConfigurePdfView(configurator: PDFView.Configurator) {} + internal interface Listener { + fun onResourceLoadFailed(href: Url, error: Resource.Exception) + fun onConfigurePdfView(configurator: PDFView.Configurator) + fun onTap(point: PointF): Boolean } - override var settings: PdfiumSettings = settings - set(value) { - if (field == value) return - - val page = pageIndex - field = value - reloadDocumentAtPage(page) - } - private lateinit var pdfView: PDFView - private var isReloading: Boolean = false - private var hasToReload: Int? = null - override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -65,86 +57,70 @@ public class PdfiumDocumentFragment internal constructor( override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - reloadDocumentAtPage(pageIndex) - } - private fun reloadDocumentAtPage(pageIndex: Int) { - if (isReloading) { - hasToReload = pageIndex - return - } + resetJob = SingleJob(viewLifecycleOwner.lifecycleScope) + reset(pageIndex = initialPageIndex) + } - isReloading = true + private lateinit var resetJob: SingleJob + private fun reset(pageIndex: Int = _pageIndex.value) { + if (view == null) return val context = context?.applicationContext ?: return - viewLifecycleOwner.lifecycleScope.launch { - try { - val document = PdfiumDocumentFactory(context) - // PDFium crashes when reusing the same PdfDocument, so we must not cache it. + resetJob.launch { + val document = PdfiumDocumentFactory(context) + // PDFium crashes when reusing the same PdfDocument, so we must not cache it. // .cachedIn(publication) - .open(publication.get(link), null) - - pageCount = document.pageCount - val page = convertPageIndexToView(pageIndex) - - pdfView.recycle() - pdfView - .fromSource { _, _, _ -> document.document } - .apply { - if (isPagesOrderReversed) { - // AndroidPdfViewer doesn't support RTL. A workaround is to provide - // the explicit page list in the right order. - pages(*((pageCount - 1) downTo 0).toList().toIntArray()) - } - } - .swipeHorizontal(settings.scrollAxis == Axis.HORIZONTAL) - .spacing(settings.pageSpacing.roundToInt()) - // Customization of [PDFView] is done before setting the listeners, - // to avoid overriding them in reading apps, which would break the - // navigator. - .apply { appListener?.onConfigurePdfView(this) } - .defaultPage(page) - .onRender { _, _, _ -> - if (settings.fit == Fit.WIDTH) { - pdfView.fitToWidth() - // Using `fitToWidth` often breaks the use of `defaultPage`, so we - // need to jump manually to the target page. - pdfView.jumpTo(page, false) - } - } - .onLoad { - val hasToReloadNow = hasToReload - if (hasToReloadNow != null) { - reloadDocumentAtPage(pageIndex) - } else { - isReloading = false - } - } - .onPageChange { index, _ -> - navigatorListener?.onPageChanged(convertPageIndexFromView(index)) + .open(publication.get(href), null) + .getOrElse { error -> + Timber.e(error) + listener?.onResourceLoadFailed(href, error) + return@launch + } + + pageCount = document.pageCount + val page = convertPageIndexToView(pageIndex) + + pdfView.recycle() + pdfView + .fromSource { _, _, _ -> document.document } + .apply { + if (isPagesOrderReversed) { + // AndroidPdfViewer doesn't support RTL. A workaround is to provide + // the explicit page list in the right order. + pages(*((pageCount - 1) downTo 0).toList().toIntArray()) } - .onTap { event -> - navigatorListener?.onTap(PointF(event.x, event.y)) - ?: false + } + .swipeHorizontal(settings.scrollAxis == Axis.HORIZONTAL) + .spacing(settings.pageSpacing.roundToInt()) + // Customization of [PDFView] is done before setting the listeners, + // to avoid overriding them in reading apps, which would break the + // navigator. + .apply { listener?.onConfigurePdfView(this) } + .defaultPage(page) + .onRender { _, _, _ -> + if (settings.fit == Fit.WIDTH) { + pdfView.fitToWidth() + // Using `fitToWidth` often breaks the use of `defaultPage`, so we + // need to jump manually to the target page. + pdfView.jumpTo(page, false) } - .load() - } catch (e: Exception) { - val error = Resource.Exception.wrap(e) - Timber.e(error) - navigatorListener?.onResourceLoadFailed(link, error) - } + } + .onPageChange { index, _ -> + _pageIndex.value = convertPageIndexFromView(index) + } + .onTap { event -> + listener?.onTap(PointF(event.x, event.y)) ?: false + } + .load() } } - override val pageIndex: Int get() = viewPageIndex ?: initialPageIndex + private var pageCount = 0 - private val viewPageIndex: Int? get() = - if (pdfView.isRecycled) { - null - } else { - convertPageIndexFromView(pdfView.currentPage) - } + private val _pageIndex = MutableStateFlow(initialPageIndex) + override val pageIndex: StateFlow = _pageIndex.asStateFlow() override fun goToPageIndex(index: Int, animated: Boolean): Boolean { if (!isValidPageIndex(index)) { @@ -154,8 +130,6 @@ public class PdfiumDocumentFragment internal constructor( return true } - private var pageCount = 0 - private fun isValidPageIndex(pageIndex: Int): Boolean { val validRange = 0 until pageCount return validRange.contains(pageIndex) @@ -182,6 +156,16 @@ public class PdfiumDocumentFragment internal constructor( * right-to-left reading progressions. */ private val isPagesOrderReversed: Boolean get() = - settings.scrollAxis == Axis.HORIZONTAL && - settings.readingProgression == ReadingProgression.RTL + settings.scrollAxis == Axis.HORIZONTAL && settings.readingProgression == ReadingProgression.RTL + + private var settings: PdfiumSettings = initialSettings + + override fun applySettings(settings: PdfiumSettings) { + if (this.settings == settings) { + return + } + + this.settings = settings + reset() + } } diff --git a/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumEngineProvider.kt b/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumEngineProvider.kt index 1aa57bc84a..b3e85ec8f1 100644 --- a/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumEngineProvider.kt +++ b/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumEngineProvider.kt @@ -6,13 +6,20 @@ package org.readium.adapters.pdfium.navigator +import android.graphics.PointF +import com.github.barteksc.pdfviewer.PDFView import org.readium.r2.navigator.SimplePresentation import org.readium.r2.navigator.VisualNavigator +import org.readium.r2.navigator.input.TapEvent import org.readium.r2.navigator.pdf.PdfDocumentFragmentInput import org.readium.r2.navigator.pdf.PdfEngineProvider +import org.readium.r2.navigator.util.SingleFragmentFactory +import org.readium.r2.navigator.util.createFragmentFactory import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.Metadata import org.readium.r2.shared.publication.Publication +import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.util.Url /** * Main component to use the PDF navigator with the PDFium adapter. @@ -22,19 +29,39 @@ import org.readium.r2.shared.publication.Publication */ @ExperimentalReadiumApi public class PdfiumEngineProvider( - private val listener: PdfiumDocumentFragment.Listener? = null, - private val defaults: PdfiumDefaults = PdfiumDefaults() + private val defaults: PdfiumDefaults = PdfiumDefaults(), + private val listener: Listener? = null ) : PdfEngineProvider { - override suspend fun createDocumentFragment(input: PdfDocumentFragmentInput): PdfiumDocumentFragment = - PdfiumDocumentFragment( - publication = input.publication, - link = input.link, - initialPageIndex = input.initialPageIndex, - settings = input.settings, - appListener = listener, - navigatorListener = input.listener - ) + public interface Listener : PdfEngineProvider.Listener { + + /** Called when configuring [PDFView]. */ + public fun onConfigurePdfView(configurator: PDFView.Configurator) {} + } + + override fun createDocumentFragmentFactory( + input: PdfDocumentFragmentInput + ): SingleFragmentFactory = + createFragmentFactory { + PdfiumDocumentFragment( + publication = input.publication, + href = input.href, + initialPageIndex = input.pageIndex, + initialSettings = input.settings, + listener = object : PdfiumDocumentFragment.Listener { + override fun onResourceLoadFailed(href: Url, error: Resource.Exception) { + input.navigatorListener?.onResourceLoadFailed(href, error) + } + + override fun onConfigurePdfView(configurator: PDFView.Configurator) { + listener?.onConfigurePdfView(configurator) + } + + override fun onTap(point: PointF): Boolean = + input.inputListener?.onTap(TapEvent(point)) ?: false + } + ) + } override fun computeSettings(metadata: Metadata, preferences: PdfiumPreferences): PdfiumSettings { val settingsPolicy = PdfiumSettingsResolver(metadata, defaults) diff --git a/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumNavigator.kt b/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumNavigator.kt new file mode 100644 index 0000000000..436d7767c7 --- /dev/null +++ b/readium/adapters/pdfium/pdfium-navigator/src/main/java/org/readium/adapters/pdfium/navigator/PdfiumNavigator.kt @@ -0,0 +1,17 @@ +/* + * Copyright 2023 Readium Foundation. All rights reserved. + * Use of this source code is governed by the BSD-style license + * available in the top-level LICENSE file of the project. + */ + +package org.readium.adapters.pdfium.navigator + +import org.readium.r2.navigator.pdf.PdfNavigatorFactory +import org.readium.r2.navigator.pdf.PdfNavigatorFragment +import org.readium.r2.shared.ExperimentalReadiumApi + +@ExperimentalReadiumApi +public typealias PdfiumNavigatorFragment = PdfNavigatorFragment + +@ExperimentalReadiumApi +public typealias PdfiumNavigatorFactory = PdfNavigatorFactory diff --git a/readium/adapters/pspdfkit/pspdfkit-document/src/main/java/org/readium/adapters/pspdfkit/document/PsPdfKitDocument.kt b/readium/adapters/pspdfkit/pspdfkit-document/src/main/java/org/readium/adapters/pspdfkit/document/PsPdfKitDocument.kt index 58dd950bf4..80f7a2d4d4 100644 --- a/readium/adapters/pspdfkit/pspdfkit-document/src/main/java/org/readium/adapters/pspdfkit/document/PsPdfKitDocument.kt +++ b/readium/adapters/pspdfkit/pspdfkit-document/src/main/java/org/readium/adapters/pspdfkit/document/PsPdfKitDocument.kt @@ -8,19 +8,21 @@ package org.readium.adapters.pspdfkit.document import android.content.Context import android.graphics.Bitmap -import androidx.core.net.toUri import com.pspdfkit.annotations.actions.GoToAction import com.pspdfkit.document.DocumentSource import com.pspdfkit.document.OutlineElement import com.pspdfkit.document.PageBinding import com.pspdfkit.document.PdfDocument as _PsPdfKitDocument import com.pspdfkit.document.PdfDocumentLoader -import java.io.File +import com.pspdfkit.exceptions.InvalidPasswordException import kotlin.reflect.KClass +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import org.readium.r2.shared.publication.ReadingProgression import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.resource.ResourceTry +import org.readium.r2.shared.util.Try import org.readium.r2.shared.util.pdf.PdfDocument import org.readium.r2.shared.util.pdf.PdfDocumentFactory import timber.log.Timber @@ -30,15 +32,22 @@ public class PsPdfKitDocumentFactory(context: Context) : PdfDocumentFactory = PsPdfKitDocument::class - override suspend fun open(file: File, password: String?): PsPdfKitDocument = - open(context, DocumentSource(file.toUri(), password)) - - override suspend fun open(resource: Resource, password: String?): PsPdfKitDocument = + override suspend fun open(resource: Resource, password: String?): ResourceTry = open(context, DocumentSource(ResourceDataProvider(resource), password)) - private suspend fun open(context: Context, documentSource: DocumentSource): PsPdfKitDocument = + private suspend fun open(context: Context, documentSource: DocumentSource): ResourceTry = withContext(Dispatchers.IO) { - PsPdfKitDocument(PdfDocumentLoader.openDocument(context, documentSource)) + try { + Try.success( + PsPdfKitDocument(PdfDocumentLoader.openDocument(context, documentSource)) + ) + } catch (e: InvalidPasswordException) { + Try.failure(Resource.Exception.Forbidden(e)) + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + Try.failure(Resource.Exception.wrap(e)) + } } } diff --git a/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitDocumentFragment.kt b/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitDocumentFragment.kt index 3a9e9cc8f2..3a78ed0b7d 100644 --- a/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitDocumentFragment.kt +++ b/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitDocumentFragment.kt @@ -14,6 +14,10 @@ import android.view.View import android.view.ViewGroup import androidx.fragment.app.FragmentContainerView import androidx.fragment.app.commitNow +import androidx.fragment.app.viewModels +import androidx.lifecycle.ViewModel +import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.viewModelScope import com.pspdfkit.annotations.Annotation import com.pspdfkit.annotations.LinkAnnotation import com.pspdfkit.annotations.SoundAnnotation @@ -31,36 +35,105 @@ import com.pspdfkit.listeners.OnPreparePopupToolbarListener import com.pspdfkit.ui.PdfFragment import com.pspdfkit.ui.toolbar.popup.PdfTextSelectionPopupToolbar import kotlin.math.roundToInt +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch import org.readium.adapters.pspdfkit.document.PsPdfKitDocument +import org.readium.adapters.pspdfkit.document.PsPdfKitDocumentFactory import org.readium.r2.navigator.pdf.PdfDocumentFragment import org.readium.r2.navigator.preferences.Axis import org.readium.r2.navigator.preferences.Fit import org.readium.r2.navigator.preferences.ReadingProgression import org.readium.r2.navigator.preferences.Spread +import org.readium.r2.navigator.util.createViewModelFactory import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.Publication import org.readium.r2.shared.publication.services.isProtected +import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.resource.ResourceTry +import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.pdf.cachedIn @ExperimentalReadiumApi -internal class PsPdfKitDocumentFragment( +public class PsPdfKitDocumentFragment internal constructor( private val publication: Publication, - private val document: PsPdfKitDocument, - private val initialPageIndex: Int, - settings: PsPdfKitSettings, + private val href: Url, + initialPageIndex: Int, + initialSettings: PsPdfKitSettings, private val listener: Listener? ) : PdfDocumentFragment() { - override var settings: PsPdfKitSettings = settings - set(value) { - if (field == value) return + internal interface Listener { + fun onResourceLoadFailed(href: Url, error: Resource.Exception) + fun onConfigurePdfView(builder: PdfConfiguration.Builder): PdfConfiguration.Builder + fun onTap(point: PointF): Boolean + } + + private companion object { + private const val pdfFragmentTag = "com.pspdfkit.ui.PdfFragment" + } + private var pdfFragment: PdfFragment? = null + set(value) { field = value - reloadDocumentAtPage() + value?.apply { + setOnPreparePopupToolbarListener(psPdfKitListener) + addDocumentListener(psPdfKitListener) + } } - private lateinit var pdfFragment: PdfFragment private val psPdfKitListener = PsPdfKitListener() + private class DocumentViewModel( + document: suspend () -> ResourceTry + ) : ViewModel() { + + private val _document: Deferred> = + viewModelScope.async { document() } + + suspend fun loadDocument(): ResourceTry = + _document.await() + + @OptIn(ExperimentalCoroutinesApi::class) + val document: PsPdfKitDocument? get() = + _document.run { + if (isCompleted) { + getCompleted().getOrNull() + } else { + null + } + } + } + + private val viewModel: DocumentViewModel by viewModels { + createViewModelFactory { + DocumentViewModel( + document = { + PsPdfKitDocumentFactory(requireContext()) + .cachedIn(publication) + .open(publication.get(href), null) + } + ) + } + } + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + // Restores the PdfFragment after a configuration change. + pdfFragment = (childFragmentManager.findFragmentByTag(pdfFragmentTag) as? PdfFragment) + ?.apply { + val document = checkNotNull(viewModel.document) { + "Should have a document when restoring the PdfFragment." + } + setCustomPdfSources(document.document.documentSources) + } + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -73,52 +146,67 @@ internal class PsPdfKitDocumentFragment( override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - reloadDocumentAtPage() - } - - private fun reloadDocumentAtPage() { - pdfFragment = createPdfFragment() - childFragmentManager.commitNow { - replace(R.id.readium_pspdfkit_fragment, pdfFragment, "com.pspdfkit.ui.PdfFragment") + if (pdfFragment == null) { + viewLifecycleOwner.lifecycleScope.launch { + viewModel.loadDocument() + .onFailure { error -> + listener?.onResourceLoadFailed(href, error) + } + .onSuccess { resetPdfFragment() } + } } } - private fun createPdfFragment(): PdfFragment { - document.document.pageBinding = settings.readingProgression.pageBinding - val config = configForSettings(settings) + /** + * Recreates the [PdfFragment] with the current settings. + */ + private fun resetPdfFragment() { + if (view == null) return + val doc = viewModel.document ?: return - val newFragment = if (::pdfFragment.isInitialized) { - PdfFragment.newInstance(pdfFragment, config) - } else { - PdfFragment.newInstance(document.document, config) - } + doc.document.pageBinding = settings.readingProgression.pageBinding - newFragment.apply { - setOnPreparePopupToolbarListener(psPdfKitListener) - addDocumentListener(psPdfKitListener) - } + val fragment = PdfFragment.newInstance(doc.document, configForSettings(settings)) + .also { pdfFragment = it } - return newFragment + childFragmentManager.commitNow { + replace(R.id.readium_pspdfkit_fragment, fragment, pdfFragmentTag) + } } private fun configForSettings(settings: PsPdfKitSettings): PdfConfiguration { - val config = PdfConfiguration.Builder() + var config = PdfConfiguration.Builder() .animateScrollOnEdgeTaps(false) .annotationReplyFeatures(AnnotationReplyFeatures.READ_ONLY) .automaticallyGenerateLinks(true) .autosaveEnabled(false) -// .backgroundColor(Color.TRANSPARENT) .disableAnnotationEditing() .disableAnnotationRotation() .disableAutoSelectNextFormElement() .disableFormEditing() .enableMagnifier(true) .excludedAnnotationTypes(emptyList()) + .scrollOnEdgeTapEnabled(false) + .scrollOnEdgeTapMargin(50) + .scrollbarsEnabled(true) + .setAnnotationInspectorEnabled(false) + .setJavaScriptEnabled(false) + .textSelectionEnabled(true) + .textSelectionPopupToolbarEnabled(true) + .themeMode(ThemeMode.DEFAULT) + .videoPlaybackEnabled(true) + .zoomOutBounce(true) + + // Customization point for integrators. + listener?.let { + config = it.onConfigurePdfView(config) + } + + // Settings-specific configuration + config = config .fitMode(settings.fit.fitMode) .layoutMode(settings.spread.pageLayout) -// .loadingProgressDrawable(null) -// .maxZoomScale() .firstPageAlwaysSingle(settings.offsetFirstPage) .pagePadding(settings.pageSpacing.roundToInt()) .restoreLastViewedPage(false) @@ -130,52 +218,50 @@ internal class PsPdfKitDocumentFragment( } ) .scrollMode(settings.scroll.scrollMode) - .scrollOnEdgeTapEnabled(false) - .scrollOnEdgeTapMargin(50) - .scrollbarsEnabled(true) - .setAnnotationInspectorEnabled(false) - .setJavaScriptEnabled(false) - .textSelectionEnabled(true) - .textSelectionPopupToolbarEnabled(true) - .themeMode(ThemeMode.DEFAULT) - .videoPlaybackEnabled(true) - .zoomOutBounce(true) if (publication.isProtected) { - config.disableCopyPaste() + config = config.disableCopyPaste() } return config.build() } - override var pageIndex: Int = initialPageIndex - private set + private val _pageIndex = MutableStateFlow(initialPageIndex) + override val pageIndex: StateFlow = _pageIndex.asStateFlow() override fun goToPageIndex(index: Int, animated: Boolean): Boolean { + val fragment = pdfFragment ?: return false if (!isValidPageIndex(index)) { return false } - pageIndex = index - pdfFragment.setPageIndex(index, animated) + fragment.setPageIndex(index, animated) return true } private fun isValidPageIndex(pageIndex: Int): Boolean { - val validRange = 0 until pdfFragment.pageCount + val validRange = 0 until (pdfFragment?.pageCount ?: 0) return validRange.contains(pageIndex) } + private var settings: PsPdfKitSettings = initialSettings + + override fun applySettings(settings: PsPdfKitSettings) { + if (this.settings == settings) { + return + } + + this.settings = settings + resetPdfFragment() + } + private inner class PsPdfKitListener : DocumentListener, OnPreparePopupToolbarListener { override fun onPageChanged(document: PdfDocument, pageIndex: Int) { - this@PsPdfKitDocumentFragment.pageIndex = pageIndex - listener?.onPageChanged(pageIndex) + _pageIndex.value = pageIndex } override fun onDocumentClick(): Boolean { - val listener = listener ?: return false - val center = view?.run { PointF(width.toFloat() / 2, height.toFloat() / 2) } - return center?.let { listener.onTap(it) } ?: false + return center?.let { listener?.onTap(it) } ?: false } override fun onPageClick( @@ -192,7 +278,7 @@ internal class PsPdfKitDocumentFragment( return false } - pdfFragment.viewProjection.toViewPoint(pagePosition, pageIndex) + checkNotNull(pdfFragment).viewProjection.toViewPoint(pagePosition, pageIndex) return listener?.onTap(pagePosition) ?: false } @@ -211,7 +297,7 @@ internal class PsPdfKitDocumentFragment( override fun onDocumentLoaded(document: PdfDocument) { super.onDocumentLoaded(document) - pdfFragment.setPageIndex(pageIndex, false) + checkNotNull(pdfFragment).setPageIndex(pageIndex.value, false) } } diff --git a/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitEngineProvider.kt b/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitEngineProvider.kt index 33414ca505..e094987276 100644 --- a/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitEngineProvider.kt +++ b/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitEngineProvider.kt @@ -6,18 +6,21 @@ package org.readium.adapters.pspdfkit.navigator -import android.content.Context -import org.readium.adapters.pspdfkit.document.PsPdfKitDocumentFactory +import android.graphics.PointF +import com.pspdfkit.configuration.PdfConfiguration import org.readium.r2.navigator.SimplePresentation import org.readium.r2.navigator.VisualNavigator -import org.readium.r2.navigator.pdf.PdfDocumentFragment +import org.readium.r2.navigator.input.TapEvent import org.readium.r2.navigator.pdf.PdfDocumentFragmentInput import org.readium.r2.navigator.pdf.PdfEngineProvider import org.readium.r2.navigator.preferences.Axis +import org.readium.r2.navigator.util.SingleFragmentFactory +import org.readium.r2.navigator.util.createFragmentFactory import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.Metadata import org.readium.r2.shared.publication.Publication -import org.readium.r2.shared.util.pdf.cachedIn +import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.util.Url /** * Main component to use the PDF navigator with PSPDFKit. @@ -27,27 +30,39 @@ import org.readium.r2.shared.util.pdf.cachedIn */ @ExperimentalReadiumApi public class PsPdfKitEngineProvider( - private val context: Context, - private val defaults: PsPdfKitDefaults = PsPdfKitDefaults() + private val defaults: PsPdfKitDefaults = PsPdfKitDefaults(), + private val listener: Listener? = null ) : PdfEngineProvider { - override suspend fun createDocumentFragment( - input: PdfDocumentFragmentInput - ): PdfDocumentFragment { - val publication = input.publication - val document = PsPdfKitDocumentFactory(context) - .cachedIn(publication) - .open(publication.get(input.link), null) + public interface Listener : PdfEngineProvider.Listener { - return PsPdfKitDocumentFragment( - publication = publication, - document = document, - initialPageIndex = input.initialPageIndex, - settings = input.settings, - listener = input.listener - ) + /** Called when configuring a new PDF fragment. */ + public fun onConfigurePdfView(builder: PdfConfiguration.Builder): PdfConfiguration.Builder = builder } + override fun createDocumentFragmentFactory( + input: PdfDocumentFragmentInput + ): SingleFragmentFactory = + createFragmentFactory { + PsPdfKitDocumentFragment( + publication = input.publication, + href = input.href, + initialPageIndex = input.pageIndex, + initialSettings = input.settings, + listener = object : PsPdfKitDocumentFragment.Listener { + override fun onResourceLoadFailed(href: Url, error: Resource.Exception) { + input.navigatorListener?.onResourceLoadFailed(href, error) + } + + override fun onConfigurePdfView(builder: PdfConfiguration.Builder): PdfConfiguration.Builder = + listener?.onConfigurePdfView(builder) ?: builder + + override fun onTap(point: PointF): Boolean = + input.inputListener?.onTap(TapEvent(point)) ?: false + } + ) + } + override fun computeSettings(metadata: Metadata, preferences: PsPdfKitPreferences): PsPdfKitSettings { val settingsPolicy = PsPdfKitSettingsResolver(metadata, defaults) return settingsPolicy.settings(preferences) diff --git a/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitNavigator.kt b/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitNavigator.kt new file mode 100644 index 0000000000..2ae95414f3 --- /dev/null +++ b/readium/adapters/pspdfkit/pspdfkit-navigator/src/main/java/org/readium/adapters/pspdfkit/navigator/PsPdfKitNavigator.kt @@ -0,0 +1,17 @@ +/* + * Copyright 2023 Readium Foundation. All rights reserved. + * Use of this source code is governed by the BSD-style license + * available in the top-level LICENSE file of the project. + */ + +package org.readium.adapters.pspdfkit.navigator + +import org.readium.r2.navigator.pdf.PdfNavigatorFactory +import org.readium.r2.navigator.pdf.PdfNavigatorFragment +import org.readium.r2.shared.ExperimentalReadiumApi + +@ExperimentalReadiumApi +public typealias PsPdfKitNavigatorFragment = PdfNavigatorFragment + +@ExperimentalReadiumApi +public typealias PsPdfKitNavigatorFactory = PdfNavigatorFactory diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/Navigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/Navigator.kt index d38b00fdb2..3cbaa1cda4 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/Navigator.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/Navigator.kt @@ -20,6 +20,8 @@ import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Locator import org.readium.r2.shared.publication.Publication import org.readium.r2.shared.publication.ReadingProgression as PublicationReadingProgression +import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.util.Url /** * Base interface for a navigator rendering a publication. @@ -76,6 +78,11 @@ public interface Navigator { public interface Listener { + /** + * Called when a publication resource failed to be loaded. + */ + public fun onResourceLoadFailed(href: Url, error: Resource.Exception) {} + /** * Called when the navigator jumps to an explicit location, which might break the linear * reading progression. diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt index 61f9dc6a3e..ec631a32ee 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt @@ -419,7 +419,7 @@ public class EpubNavigatorFragment internal constructor( // Fixed layout publications cannot intercept JS events yet. if (publication.metadata.presentation.layout == EpubLayout.FIXED) { - view = KeyInterceptorView(view, this, inputListener) + view = KeyInterceptorView(view, inputListener) } return view @@ -757,7 +757,7 @@ public class EpubNavigatorFragment internal constructor( config.javascriptInterfaces.mapValues { (_, factory) -> factory(link) } override fun onTap(point: PointF): Boolean = - inputListener.onTap(this@EpubNavigatorFragment, TapEvent(point)) + inputListener.onTap(TapEvent(point)) override fun onDragStart(event: R2BasicWebView.DragEvent): Boolean = onDrag(DragEvent.Type.Start, event) @@ -770,7 +770,6 @@ public class EpubNavigatorFragment internal constructor( private fun onDrag(type: DragEvent.Type, event: R2BasicWebView.DragEvent): Boolean = inputListener.onDrag( - this@EpubNavigatorFragment, DragEvent( type = type, start = event.startPoint.adjustedToViewport(), @@ -779,7 +778,7 @@ public class EpubNavigatorFragment internal constructor( ) override fun onKey(event: KeyEvent): Boolean = - inputListener.onKey(this@EpubNavigatorFragment, event) + inputListener.onKey(event) override fun onDecorationActivated( id: DecorationId, diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt index 27584d903e..861d29f6f5 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt @@ -88,7 +88,6 @@ public class ImageNavigatorFragment private constructor( childFragmentManager.fragmentFactory = createFragmentFactory { R2CbzPageFragment(publication) { x, y -> inputListener.onTap( - this, TapEvent(PointF(x, y)) ) } @@ -142,7 +141,7 @@ public class ImageNavigatorFragment private constructor( go(initialLocator) } - return KeyInterceptorView(view, this, inputListener) + return KeyInterceptorView(view, inputListener) } override fun onStart() { diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/input/InputListener.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/input/InputListener.kt index 52a0bcad5e..12c0e0a1e7 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/input/InputListener.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/input/InputListener.kt @@ -1,6 +1,5 @@ package org.readium.r2.navigator.input -import org.readium.r2.navigator.VisualNavigator import org.readium.r2.shared.ExperimentalReadiumApi @ExperimentalReadiumApi @@ -9,17 +8,17 @@ public interface InputListener { * Called when the user tapped the content, but nothing handled the event internally (eg. * by following an internal link). */ - public fun onTap(navigator: VisualNavigator, event: TapEvent): Boolean = false + public fun onTap(event: TapEvent): Boolean = false /** * Called when the user dragged the content, but nothing handled the event internally. */ - public fun onDrag(navigator: VisualNavigator, event: DragEvent): Boolean = false + public fun onDrag(event: DragEvent): Boolean = false /** * Called when the user pressed or released a key, but nothing handled the event internally. */ - public fun onKey(navigator: VisualNavigator, event: KeyEvent): Boolean = false + public fun onKey(event: KeyEvent): Boolean = false } @OptIn(ExperimentalReadiumApi::class) @@ -34,12 +33,12 @@ internal class CompositeInputListener : InputListener { listeners.remove(listener) } - override fun onTap(navigator: VisualNavigator, event: TapEvent): Boolean = - listeners.any { it.onTap(navigator, event) } + override fun onTap(event: TapEvent): Boolean = + listeners.any { it.onTap(event) } - override fun onDrag(navigator: VisualNavigator, event: DragEvent): Boolean = - listeners.any { it.onDrag(navigator, event) } + override fun onDrag(event: DragEvent): Boolean = + listeners.any { it.onDrag(event) } - override fun onKey(navigator: VisualNavigator, event: KeyEvent): Boolean = - listeners.any { it.onKey(navigator, event) } + override fun onKey(event: KeyEvent): Boolean = + listeners.any { it.onKey(event) } } diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/input/KeyInterceptorView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/input/KeyInterceptorView.kt index de413a8dc3..f32688e282 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/input/KeyInterceptorView.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/input/KeyInterceptorView.kt @@ -14,7 +14,6 @@ import org.readium.r2.shared.ExperimentalReadiumApi @OptIn(ExperimentalReadiumApi::class) internal class KeyInterceptorView( view: View, - private val navigator: VisualNavigator, private val listener: InputListener? ) : FrameLayout(view.context) { @@ -30,14 +29,14 @@ internal class KeyInterceptorView( override fun onKeyUp(keyCode: Int, event: android.view.KeyEvent?): Boolean { event ?.let { KeyEvent(KeyEvent.Type.Up, it) } - ?.let { listener?.onKey(navigator, it) } + ?.let { listener?.onKey(it) } return super.onKeyUp(keyCode, event) } override fun onKeyDown(keyCode: Int, event: android.view.KeyEvent?): Boolean { event ?.let { KeyEvent(KeyEvent.Type.Down, it) } - ?.let { listener?.onKey(navigator, it) } + ?.let { listener?.onKey(it) } return super.onKeyDown(keyCode, event) } } diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfEngineProvider.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfEngineProvider.kt index 44eacce190..165c15df50 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfEngineProvider.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfEngineProvider.kt @@ -6,16 +6,18 @@ package org.readium.r2.navigator.pdf -import android.graphics.PointF import androidx.fragment.app.Fragment +import kotlinx.coroutines.flow.StateFlow +import org.readium.r2.navigator.Navigator import org.readium.r2.navigator.VisualNavigator +import org.readium.r2.navigator.input.InputListener import org.readium.r2.navigator.preferences.Configurable import org.readium.r2.navigator.preferences.PreferencesEditor +import org.readium.r2.navigator.util.SingleFragmentFactory import org.readium.r2.shared.ExperimentalReadiumApi -import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Metadata import org.readium.r2.shared.publication.Publication -import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.util.Url /** * To be implemented by adapters for third-party PDF engines which can be used with [PdfNavigatorFragment]. @@ -23,10 +25,12 @@ import org.readium.r2.shared.resource.Resource @ExperimentalReadiumApi public interface PdfEngineProvider, E : PreferencesEditor

> { + public interface Listener + /** - * Creates a [PdfDocumentFragment] for [input]. + * Creates a [PdfDocumentFragment] factory for [input]. */ - public suspend fun createDocumentFragment(input: PdfDocumentFragmentInput): PdfDocumentFragment + public fun createDocumentFragmentFactory(input: PdfDocumentFragmentInput): SingleFragmentFactory<*> /** * Creates settings for [metadata] and [preferences]. @@ -34,7 +38,7 @@ public interface PdfEngineProvider = suspend (PdfDocumentFragmentInput) -> PdfDocumentFragment - /** - * A [PdfDocumentFragment] renders a single PDF resource. + * A [PdfDocumentFragment] renders a single PDF document. */ @ExperimentalReadiumApi public abstract class PdfDocumentFragment : Fragment() { - public interface Listener { - /** - * Called when the fragment navigates to a different page. - */ - public fun onPageChanged(pageIndex: Int) - - /** - * Called when the user triggers a tap on the document. - */ - public fun onTap(point: PointF): Boolean - - /** - * Called when the PDF engine fails to load the PDF document. - */ - public fun onResourceLoadFailed(link: Link, error: Resource.Exception) - } - /** - * Returns the current page index in the document, from 0. + * Current page index displayed in the PDF document. */ - public abstract val pageIndex: Int + public abstract val pageIndex: StateFlow /** * Jumps to the given page [index]. @@ -89,16 +73,17 @@ public abstract class PdfDocumentFragment : Fragment( public abstract fun goToPageIndex(index: Int, animated: Boolean): Boolean /** - * Current settings for the PDF document. + * Submits a new set of settings. */ - public abstract var settings: S + public abstract fun applySettings(settings: S) } @ExperimentalReadiumApi public data class PdfDocumentFragmentInput( val publication: Publication, - val link: Link, - val initialPageIndex: Int, + val href: Url, + val pageIndex: Int, val settings: S, - val listener: PdfDocumentFragment.Listener? + val navigatorListener: Navigator.Listener?, + val inputListener: InputListener? ) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfNavigatorFragment.kt index 1f7ea7fb30..5b8af9c629 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfNavigatorFragment.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfNavigatorFragment.kt @@ -6,7 +6,6 @@ package org.readium.r2.navigator.pdf -import android.graphics.PointF import android.os.Bundle import android.view.LayoutInflater import android.view.View @@ -19,14 +18,9 @@ import androidx.fragment.app.viewModels import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import org.readium.r2.navigator.R import org.readium.r2.navigator.VisualNavigator @@ -35,10 +29,8 @@ import org.readium.r2.navigator.extensions.page import org.readium.r2.navigator.input.CompositeInputListener import org.readium.r2.navigator.input.InputListener import org.readium.r2.navigator.input.KeyInterceptorView -import org.readium.r2.navigator.input.TapEvent import org.readium.r2.navigator.preferences.Configurable -import org.readium.r2.navigator.preferences.PreferencesEditor -import org.readium.r2.navigator.preferences.ReadingProgression +import org.readium.r2.navigator.util.SingleFragmentFactory import org.readium.r2.navigator.util.createFragmentFactory import org.readium.r2.shared.DelicateReadiumApi import org.readium.r2.shared.ExperimentalReadiumApi @@ -48,16 +40,13 @@ import org.readium.r2.shared.publication.Locator import org.readium.r2.shared.publication.Publication import org.readium.r2.shared.publication.ReadingProgression as PublicationReadingProgression import org.readium.r2.shared.publication.services.isRestricted -import org.readium.r2.shared.resource.Resource import org.readium.r2.shared.util.mediatype.MediaType -import timber.log.Timber /** * Navigator for PDF publications. * * The PDF navigator delegates the actual PDF rendering to third-party engines like PDFium or - * PSPDFKit. You must use an implementation of [PdfDocumentFragmentFactory] provided by the PDF - * engine of your choice. + * PSPDFKit. * * To use this [Fragment], create a factory with [PdfNavigatorFactory.createFragmentFactory]. */ @@ -65,19 +54,13 @@ import timber.log.Timber @OptIn(DelicateReadiumApi::class) public class PdfNavigatorFragment> internal constructor( override val publication: Publication, - initialLocator: Locator? = null, - initialPreferences: P, + private val initialLocator: Locator? = null, + private val initialPreferences: P, private val listener: Listener?, private val pdfEngineProvider: PdfEngineProvider ) : Fragment(), VisualNavigator, Configurable { - public interface Listener : VisualNavigator.Listener { - - /** - * Called when a PDF resource failed to be loaded, for example because of an [OutOfMemoryError]. - */ - public fun onResourceLoadFailed(link: Link, error: Resource.Exception) {} - } + public interface Listener : VisualNavigator.Listener public companion object { @@ -92,12 +75,12 @@ public class PdfNavigatorFragment, E : PreferencesEditor

> createFactory( + public fun

> createFactory( publication: Publication, initialLocator: Locator? = null, preferences: P? = null, listener: Listener? = null, - pdfEngineProvider: PdfEngineProvider + pdfEngineProvider: PdfEngineProvider<*, P, *> ): FragmentFactory = createFragmentFactory { PdfNavigatorFragment( publication, @@ -118,22 +101,37 @@ public class PdfNavigatorFragment by viewModels { PdfNavigatorViewModel.createFactory( requireActivity().application, publication, - initialLocator?.let { publication.normalizeLocator(it) }, - initialPreferences = initialPreferences, - pdfEngineProvider = pdfEngineProvider + initialLocator?.locations, + initialPreferences, + pdfEngineProvider ) } - private lateinit var documentFragment: StateFlow?> + private lateinit var documentFragment: PdfDocumentFragment + + private val documentFragmentFactory: SingleFragmentFactory<*> by lazy { + val locator = viewModel.currentLocator.value + pdfEngineProvider.createDocumentFragmentFactory( + PdfDocumentFragmentInput( + publication = publication, + href = locator.href, + pageIndex = locator.locations.pageIndex, + settings = viewModel.settings.value, + navigatorListener = listener, + inputListener = inputListener + ) + ) + } override fun onCreate(savedInstanceState: Bundle?) { - // Clears the savedInstanceState to prevent the child fragment manager from restoring the - // pdfFragment, as the [ResourceDataProvider] is not [Parcelable]. - super.onCreate(null) + childFragmentManager.fragmentFactory = documentFragmentFactory + super.onCreate(savedInstanceState) } override fun onCreateView( @@ -143,79 +141,42 @@ public class PdfNavigatorFragment - old.href == new.href - } - .map { locator -> - createPdfDocumentFragment(locator, settings.value) + val tag = "documentFragment" + if (savedInstanceState == null) { + childFragmentManager.commitNow { + replace( + R.id.readium_pdf_container, + documentFragmentFactory(), + tag + ) } - .stateIn(viewLifecycleOwner.lifecycleScope, started = SharingStarted.Eagerly, null) + } + + @Suppress("UNCHECKED_CAST") + documentFragment = childFragmentManager.findFragmentByTag(tag) as PdfDocumentFragment viewLifecycleOwner.lifecycleScope.launch { - viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { - documentFragment - .filterNotNull() - .onEach { fragment: PdfDocumentFragment -> - childFragmentManager.commitNow { - replace(R.id.readium_pdf_container, fragment, "readium_pdf_fragment") - } - } + repeatOnLifecycle(Lifecycle.State.STARTED) { + documentFragment.pageIndex + .onEach { viewModel.onPageChanged(it) } .launchIn(this) - settings - .onEach { settings -> - documentFragment.value?.settings = settings - } + viewModel.settings + .onEach { documentFragment.applySettings(it) } .launchIn(this) } } } - private suspend fun createPdfDocumentFragment(locator: Locator, settings: S): PdfDocumentFragment? { - val link = viewModel.findLink(locator) ?: return null - - return try { - val pageIndex = (locator.locations.page ?: 1) - 1 - pdfEngineProvider.createDocumentFragment( - PdfDocumentFragmentInput( - publication = publication, - link = link, - initialPageIndex = pageIndex, - settings = settings, - listener = DocumentFragmentListener() - ) - ) - } catch (e: Exception) { - Timber.e(e, "Failed to load PDF resource ${link.href}") - listener?.onResourceLoadFailed(link, Resource.Exception.wrap(e)) - null - } - } - - private inner class DocumentFragmentListener : PdfDocumentFragment.Listener { - override fun onPageChanged(pageIndex: Int) { - viewModel.onPageChanged(pageIndex) - } - - override fun onTap(point: PointF): Boolean = - inputListener.onTap(this@PdfNavigatorFragment, TapEvent(point)) - - override fun onResourceLoadFailed(link: Link, error: Resource.Exception) { - listener?.onResourceLoadFailed(link, error) - } - } - // Configurable - @Suppress("Unchecked_cast") - override val settings: StateFlow get() = viewModel.settings as StateFlow + override val settings: StateFlow get() = viewModel.settings override fun submitPreferences(preferences: P) { viewModel.submitPreferences(preferences) @@ -223,15 +184,13 @@ public class PdfNavigatorFragment - get() = viewModel.currentLocator + override val currentLocator: StateFlow get() = viewModel.currentLocator override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean { @Suppress("NAME_SHADOWING") val locator = publication.normalizeLocator(locator) listener?.onJumpToLocator(locator) - val pageNumber = locator.locations.page ?: locator.locations.position ?: 1 - return goToPageIndex(pageNumber - 1, animated, completion) + return goToPageIndex(locator.locations.pageIndex, animated, completion) } override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean { @@ -240,18 +199,17 @@ public class PdfNavigatorFragment Unit): Boolean { - val fragment = documentFragment.value ?: return false - return goToPageIndex(fragment.pageIndex + 1, animated, completion) + val pageIndex = currentLocator.value.locations.pageIndex + 1 + return goToPageIndex(pageIndex, animated, completion) } override fun goBackward(animated: Boolean, completion: () -> Unit): Boolean { - val fragment = documentFragment.value ?: return false - return goToPageIndex(fragment.pageIndex - 1, animated, completion) + val pageIndex = currentLocator.value.locations.pageIndex - 1 + return goToPageIndex(pageIndex, animated, completion) } private fun goToPageIndex(pageIndex: Int, animated: Boolean, completion: () -> Unit): Boolean { - val fragment = documentFragment.value ?: return false - val success = fragment.goToPageIndex(pageIndex, animated = animated) + val success = documentFragment.goToPageIndex(pageIndex, animated = animated) if (success) { completion() } return success } @@ -273,12 +231,7 @@ public class PdfNavigatorFragment PublicationReadingProgression.LTR - ReadingProgression.RTL -> PublicationReadingProgression.RTL - } - - private val inputListener = CompositeInputListener() + get() = throw NotImplementedError() override fun addInputListener(listener: InputListener) { inputListener.add(listener) @@ -288,3 +241,6 @@ public class PdfNavigatorFragment>( application: Application, private val publication: Publication, - initialLocator: Locator, + initialLocations: Locator.Locations?, initialPreferences: P, private val pdfEngineProvider: PdfEngineProvider ) : AndroidViewModel(application) { private val _currentLocator: MutableStateFlow = - MutableStateFlow(initialLocator) + MutableStateFlow( + requireNotNull(publication.locatorFromLink(publication.readingOrder.first())) + .copy(locations = initialLocations ?: Locator.Locations()) + ) val currentLocator: StateFlow = _currentLocator.asStateFlow() - private val _settings: MutableStateFlow = MutableStateFlow( - pdfEngineProvider.computeSettings(publication.metadata, initialPreferences) - ) + private val _settings: MutableStateFlow = + MutableStateFlow(computeSettings(initialPreferences)) - val settings: StateFlow = _settings.asStateFlow() + val settings: StateFlow = _settings.asStateFlow() fun submitPreferences(preferences: P) = viewModelScope.launch { - _settings.value = pdfEngineProvider.computeSettings(publication.metadata, preferences) + _settings.value = computeSettings(preferences) } + private fun computeSettings(preferences: P): S = + pdfEngineProvider.computeSettings(publication.metadata, preferences) + fun onPageChanged(pageIndex: Int) = viewModelScope.launch { publication.positions().getOrNull(pageIndex)?.let { locator -> _currentLocator.value = locator } } - fun findLink(locator: Locator): Link? = - if (isPDFFile) { - publication.readingOrder.first() - } else { - publication.linkWithHref(locator.href) - } - - /** - * Historically, the reading order of a standalone PDF file contained a single link with the - * HREF "/$assetName". This was fragile if the asset named changed, or was different on other - * devices. - * - * To avoid this, we now use a single link with the HREF "publication.pdf". And to avoid - * breaking legacy locators, we match any HREF if the reading order contains a single link with - * the HREF "publication.pdf". - */ - private val isPDFFile: Boolean = - publication.readingOrder.count() == 1 && - publication.readingOrder[0].href.toString() == "publication.pdf" - companion object { - fun , E : PreferencesEditor

> createFactory( + fun > createFactory( application: Application, publication: Publication, - initialLocator: Locator?, + initialLocations: Locator.Locations?, initialPreferences: P, - pdfEngineProvider: PdfEngineProvider + pdfEngineProvider: PdfEngineProvider ) = createViewModelFactory { PdfNavigatorViewModel( application = application, publication = publication, - initialLocator = initialLocator - ?: requireNotNull(publication.locatorFromLink(publication.readingOrder.first())), + initialLocations = initialLocations, initialPreferences = initialPreferences, pdfEngineProvider = pdfEngineProvider ) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/util/DirectionalNavigationAdapter.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/util/DirectionalNavigationAdapter.kt index 0a234278d8..d4108bbb2b 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/util/DirectionalNavigationAdapter.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/util/DirectionalNavigationAdapter.kt @@ -33,6 +33,7 @@ import org.readium.r2.shared.ExperimentalReadiumApi */ @ExperimentalReadiumApi public class DirectionalNavigationAdapter( + private val navigator: VisualNavigator, private val tapEdges: Set = setOf(TapEdge.Horizontal), private val handleTapsWhileScrolling: Boolean = false, private val minimumHorizontalEdgeSize: Double = 80.0, @@ -49,7 +50,7 @@ public class DirectionalNavigationAdapter( Horizontal, Vertical; } - override fun onTap(navigator: VisualNavigator, event: TapEvent): Boolean { + override fun onTap(event: TapEvent): Boolean { if (navigator.presentation.value.scroll && !handleTapsWhileScrolling) { return false } @@ -89,7 +90,7 @@ public class DirectionalNavigationAdapter( return false } - override fun onKey(navigator: VisualNavigator, event: KeyEvent): Boolean { + override fun onKey(event: KeyEvent): Boolean { if (event.type != KeyEvent.Type.Down || event.modifiers.isNotEmpty()) { return false } diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/util/FragmentFactory.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/util/FragmentFactory.kt index 9061f3d6e5..ef8f34a1b1 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/util/FragmentFactory.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/util/FragmentFactory.kt @@ -12,20 +12,33 @@ import org.readium.r2.shared.InternalReadiumApi import org.readium.r2.shared.extensions.tryOrNull /** - * Creates a [FragmentFactory] for a single type of [Fragment] using the result of the given - * [factory] closure. + * A [FragmentFactory] which will instantiate a single type of [Fragment] using the result of the + * given [factory] closure. */ -@InternalReadiumApi -public inline fun createFragmentFactory(crossinline factory: () -> T): FragmentFactory = object : FragmentFactory() { +public class SingleFragmentFactory( + public val fragmentClass: Class, + private val factory: () -> T +) : FragmentFactory() { + + public operator fun invoke(): T = + factory() override fun instantiate(classLoader: ClassLoader, className: String): Fragment { return when (className) { - T::class.java.name -> factory() + fragmentClass.name -> factory() else -> super.instantiate(classLoader, className) } } } +/** + * Creates a [FragmentFactory] for a single type of [Fragment] using the result of the given + * [factory] closure. + */ +@InternalReadiumApi +public inline fun createFragmentFactory(noinline factory: () -> T): SingleFragmentFactory = + SingleFragmentFactory(T::class.java, factory) + /** * A [FragmentFactory] which will iterate over a provided list of [factories] until finding one * instantiating successfully the requested [Fragment]. diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/SingleJob.kt b/readium/shared/src/main/java/org/readium/r2/shared/util/SingleJob.kt new file mode 100644 index 0000000000..609ad697a5 --- /dev/null +++ b/readium/shared/src/main/java/org/readium/r2/shared/util/SingleJob.kt @@ -0,0 +1,49 @@ +/* + * Copyright 2023 Readium Foundation. All rights reserved. + * Use of this source code is governed by the BSD-style license + * available in the top-level LICENSE file of the project. + */ + +package org.readium.r2.shared.util + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import org.readium.r2.shared.InternalReadiumApi + +/** + * Runs a single coroutine job at a time. + * + * If a previous job is running, cancels it before launching the new one. + */ +@InternalReadiumApi +public class SingleJob( + private val scope: CoroutineScope +) { + private var job: Job? = null + private val mutex = Mutex() + + /** + * Launches a coroutine job. + * + * If a previous job is running, cancels it before launching the new one. + */ + public fun launch(block: suspend CoroutineScope.() -> Unit) { + scope.launch { + mutex.withLock { + job?.cancelAndJoin() + job = launch { block() } + } + } + } + + /** + * Cancels the current job, if any. + */ + public fun cancel() { + job?.cancel() + } +} diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/cache/Cache.kt b/readium/shared/src/main/java/org/readium/r2/shared/util/cache/Cache.kt index 23faaa45f0..09b96a11b6 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/util/cache/Cache.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/util/cache/Cache.kt @@ -15,6 +15,7 @@ import org.readium.r2.shared.InternalReadiumApi import org.readium.r2.shared.util.Closeable import org.readium.r2.shared.util.MemoryObserver import org.readium.r2.shared.util.SuspendingCloseable +import org.readium.r2.shared.util.Try /** * A generic cache for objects of type [V]. @@ -44,13 +45,6 @@ public interface CacheTransaction { */ public suspend fun put(key: String, value: V?) - /** - * Gets the current cached value for the given [key] or creates and caches a new one. - */ - public suspend fun CacheTransaction.getOrPut(key: String, defaultValue: suspend () -> V): V = - get(key) - ?: defaultValue().also { put(key, it) } - /** * Clears the cached value for the given [key]. * @@ -64,6 +58,24 @@ public interface CacheTransaction { public suspend fun clear() } +/** + * Gets the current cached value for the given [key] or creates and caches a new one. + */ +public suspend fun CacheTransaction.getOrPut(key: String, defaultValue: suspend () -> V): V = + get(key) + ?: defaultValue().also { put(key, it) } + +/** + * Gets the current cached value for the given [key] or creates and caches a new one. + */ +public suspend fun CacheTransaction.getOrTryPut( + key: String, + defaultValue: suspend () -> Try +): Try = + get(key)?.let { Try.success(it) } + ?: defaultValue() + .onSuccess { put(key, it) } + /** * A basic in-memory cache. */ diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/pdf/PdfDocument.kt b/readium/shared/src/main/java/org/readium/r2/shared/util/pdf/PdfDocument.kt index a41a55ee43..ae193c06ee 100644 --- a/readium/shared/src/main/java/org/readium/r2/shared/util/pdf/PdfDocument.kt +++ b/readium/shared/src/main/java/org/readium/r2/shared/util/pdf/PdfDocument.kt @@ -11,7 +11,6 @@ package org.readium.r2.shared.util.pdf import android.content.Context import android.graphics.Bitmap -import java.io.File import kotlin.reflect.KClass import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.Link @@ -20,9 +19,11 @@ import org.readium.r2.shared.publication.PublicationServicesHolder import org.readium.r2.shared.publication.ReadingProgression import org.readium.r2.shared.publication.services.cacheService import org.readium.r2.shared.resource.Resource +import org.readium.r2.shared.resource.ResourceTry import org.readium.r2.shared.util.SuspendingCloseable import org.readium.r2.shared.util.Url import org.readium.r2.shared.util.cache.Cache +import org.readium.r2.shared.util.cache.getOrTryPut import org.readium.r2.shared.util.mediatype.MediaType public interface PdfDocumentFactory { @@ -30,11 +31,8 @@ public interface PdfDocumentFactory { /** Class for the type of document this factory produces. */ public val documentType: KClass - /** Opens a PDF from a [file]. */ - public suspend fun open(file: File, password: String?): T - - /** Opens a PDF from a Fetcher resource. */ - public suspend fun open(resource: Resource, password: String?): T + /** Opens a PDF from a [resource]. */ + public suspend fun open(resource: Resource, password: String?): ResourceTry } /** @@ -58,17 +56,10 @@ private class CachingPdfDocumentFactory( private val cache: Cache ) : PdfDocumentFactory by factory { - override suspend fun open(file: File, password: String?): T = - cache.transaction { - getOrPut(file.path) { - factory.open(file, password) - } - } - - override suspend fun open(resource: Resource, password: String?): T { + override suspend fun open(resource: Resource, password: String?): ResourceTry { val key = resource.source?.toString() ?: return factory.open(resource, password) return cache.transaction { - getOrPut(key) { + getOrTryPut(key) { factory.open(resource, password) } } diff --git a/readium/streamer/src/main/java/org/readium/r2/streamer/parser/pdf/PdfParser.kt b/readium/streamer/src/main/java/org/readium/r2/streamer/parser/pdf/PdfParser.kt index f2ecea346f..dc198d80e2 100644 --- a/readium/streamer/src/main/java/org/readium/r2/streamer/parser/pdf/PdfParser.kt +++ b/readium/streamer/src/main/java/org/readium/r2/streamer/parser/pdf/PdfParser.kt @@ -13,6 +13,7 @@ import org.readium.r2.shared.publication.* import org.readium.r2.shared.publication.services.InMemoryCacheService import org.readium.r2.shared.publication.services.InMemoryCoverService import org.readium.r2.shared.util.Try +import org.readium.r2.shared.util.getOrElse import org.readium.r2.shared.util.logging.WarningLogger import org.readium.r2.shared.util.mediatype.MediaType import org.readium.r2.shared.util.pdf.PdfDocumentFactory @@ -45,6 +46,9 @@ public class PdfParser( PublicationParser.Error.ParsingFailed("No PDF found in the publication.") ) val document = pdfFactory.open(resource, password = null) + .getOrElse { + return Try.failure(PublicationParser.Error.IO(it)) + } val tableOfContents = document.outline.toLinks(resource.url) val manifest = Manifest( diff --git a/readium/streamer/src/main/java/org/readium/r2/streamer/parser/readium/LcpdfPositionsService.kt b/readium/streamer/src/main/java/org/readium/r2/streamer/parser/readium/LcpdfPositionsService.kt index fd9897cd80..af9f2b2bc4 100644 --- a/readium/streamer/src/main/java/org/readium/r2/streamer/parser/readium/LcpdfPositionsService.kt +++ b/readium/streamer/src/main/java/org/readium/r2/streamer/parser/readium/LcpdfPositionsService.kt @@ -10,15 +10,16 @@ package org.readium.r2.streamer.parser.readium import org.readium.r2.shared.ExperimentalReadiumApi -import org.readium.r2.shared.extensions.tryOrLog import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Locator import org.readium.r2.shared.publication.Publication import org.readium.r2.shared.publication.services.PositionsService +import org.readium.r2.shared.util.getOrElse import org.readium.r2.shared.util.mediatype.MediaType import org.readium.r2.shared.util.pdf.PdfDocument import org.readium.r2.shared.util.pdf.PdfDocumentFactory import org.readium.r2.shared.util.pdf.cachedIn +import timber.log.Timber /** * Creates the [positions] for an LCP protected PDF [Publication] from its reading order and @@ -95,11 +96,13 @@ internal class LcpdfPositionsService( } private suspend fun openPdfAt(link: Link): PdfDocument? = - tryOrLog { - pdfFactory - .cachedIn(context.services) - .open(context.container.get(link.url()), password = null) - } + pdfFactory + .cachedIn(context.services) + .open(context.container.get(link.url()), password = null) + .getOrElse { + Timber.e(it) + null + } companion object { diff --git a/test-app/src/main/java/org/readium/r2/testapp/Application.kt b/test-app/src/main/java/org/readium/r2/testapp/Application.kt index d500d26071..641c3366d2 100755 --- a/test-app/src/main/java/org/readium/r2/testapp/Application.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/Application.kt @@ -7,12 +7,15 @@ package org.readium.r2.testapp import android.content.Context +import android.os.Build +import android.os.StrictMode import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.Preferences import androidx.datastore.preferences.preferencesDataStore import com.google.android.material.color.DynamicColors import java.io.File import java.util.Properties +import java.util.concurrent.Executors import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Deferred import kotlinx.coroutines.MainScope @@ -54,9 +57,14 @@ class Application : android.app.Application() { by preferencesDataStore(name = "navigator-preferences") override fun onCreate() { + if (DEBUG) { +// enableStrictMode() + Timber.plant(Timber.DebugTree()) + } + super.onCreate() + DynamicColors.applyToActivitiesIfAvailable(this) - if (DEBUG) Timber.plant(Timber.DebugTree()) readium = Readium(this) @@ -138,4 +146,34 @@ class Application : android.app.Application() { } ) } + + /** + * Strict mode will log violation of VM and threading policy. + * Use it to make sure the app doesn't do too much work on the main thread. + */ + private fun enableStrictMode() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) { + return + } + + val executor = Executors.newSingleThreadExecutor() + StrictMode.setThreadPolicy( + StrictMode.ThreadPolicy.Builder() + .detectAll() + .penaltyListener(executor) { violation -> + Timber.e(violation, "Thread policy violation") + } +// .penaltyDeath() + .build() + ) + StrictMode.setVmPolicy( + StrictMode.VmPolicy.Builder() + .detectAll() + .penaltyListener(executor) { violation -> + Timber.e(violation, "VM policy violation") + } +// .penaltyDeath() + .build() + ) + } } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/EpubReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/EpubReaderFragment.kt index b25cf65eb0..828797d51d 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/EpubReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/EpubReaderFragment.kt @@ -41,7 +41,7 @@ import org.readium.r2.testapp.reader.preferences.UserPreferencesViewModel import org.readium.r2.testapp.search.SearchFragment @OptIn(ExperimentalReadiumApi::class, ExperimentalDecorator::class) -class EpubReaderFragment : VisualReaderFragment(), EpubNavigatorFragment.Listener { +class EpubReaderFragment : VisualReaderFragment() { override lateinit var navigator: EpubNavigatorFragment @@ -61,7 +61,7 @@ class EpubReaderFragment : VisualReaderFragment(), EpubNavigatorFragment.Listene readerData.navigatorFactory.createFragmentFactory( initialLocator = readerData.initialLocation, initialPreferences = readerData.preferencesManager.preferences.value, - listener = this, + listener = model, configuration = EpubNavigatorFragment.Configuration { // To customize the text selection menu. selectionActionModeCallback = customSelectionActionModeCallback diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ImageReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ImageReaderFragment.kt index 295a00de1e..9a8592ca3d 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/ImageReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ImageReaderFragment.kt @@ -15,7 +15,7 @@ import org.readium.r2.navigator.Navigator import org.readium.r2.navigator.image.ImageNavigatorFragment import org.readium.r2.testapp.R -class ImageReaderFragment : VisualReaderFragment(), ImageNavigatorFragment.Listener { +class ImageReaderFragment : VisualReaderFragment() { override lateinit var navigator: Navigator @@ -23,7 +23,11 @@ class ImageReaderFragment : VisualReaderFragment(), ImageNavigatorFragment.Liste val readerData = model.readerInitData as VisualReaderInitData childFragmentManager.fragmentFactory = - ImageNavigatorFragment.createFactory(publication, readerData.initialLocation, this) + ImageNavigatorFragment.createFactory( + publication, + readerData.initialLocation, + model + ) super.onCreate(savedInstanceState) } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/PdfReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/PdfReaderFragment.kt index d1a78e696e..80c80c82b0 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/PdfReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/PdfReaderFragment.kt @@ -10,21 +10,19 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.Toast import androidx.fragment.app.commitNow +import org.readium.adapters.pdfium.navigator.PdfiumNavigatorFragment import org.readium.adapters.pdfium.navigator.PdfiumPreferences import org.readium.adapters.pdfium.navigator.PdfiumSettings import org.readium.r2.navigator.pdf.PdfNavigatorFragment import org.readium.r2.shared.ExperimentalReadiumApi -import org.readium.r2.shared.publication.Link -import org.readium.r2.shared.resource.Resource import org.readium.r2.testapp.R import org.readium.r2.testapp.reader.preferences.UserPreferencesViewModel @OptIn(ExperimentalReadiumApi::class) -class PdfReaderFragment : VisualReaderFragment(), PdfNavigatorFragment.Listener { +class PdfReaderFragment : VisualReaderFragment() { - override lateinit var navigator: PdfNavigatorFragment + override lateinit var navigator: PdfiumNavigatorFragment override fun onCreate(savedInstanceState: Bundle?) { val readerData = model.readerInitData as PdfReaderInitData @@ -33,7 +31,7 @@ class PdfReaderFragment : VisualReaderFragment(), PdfNavigatorFragment.Listener readerData.navigatorFactory.createFragmentFactory( initialLocator = readerData.initialLocation, initialPreferences = readerData.preferencesManager.preferences.value, - listener = this + listener = model ) super.onCreate(savedInstanceState) @@ -55,9 +53,10 @@ class PdfReaderFragment : VisualReaderFragment(), PdfNavigatorFragment.Listener ) } } + @Suppress("Unchecked_cast") navigator = childFragmentManager.findFragmentByTag(NAVIGATOR_FRAGMENT_TAG)!! - as PdfNavigatorFragment + as PdfiumNavigatorFragment return view } @@ -69,17 +68,6 @@ class PdfReaderFragment : VisualReaderFragment(), PdfNavigatorFragment.Listener .bind(navigator, viewLifecycleOwner) } - override fun onResourceLoadFailed(link: Link, error: Resource.Exception) { - val message = when (error) { - is Resource.Exception.OutOfMemory -> "The PDF is too large to be rendered on this device" - else -> "Failed to render this PDF" - } - Toast.makeText(requireActivity(), message, Toast.LENGTH_LONG).show() - - // There's nothing we can do to recover, so we quit the Activity. - requireActivity().finish() - } - companion object { const val NAVIGATOR_FRAGMENT_TAG = "navigator" diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt index 58269a9bf4..ab56418f96 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderActivity.kt @@ -158,7 +158,9 @@ open class ReaderActivity : AppCompatActivity() { when (event) { is ReaderViewModel.Event.OpenOutlineRequested -> showOutlineFragment() is ReaderViewModel.Event.OpenDrmManagementRequested -> showDrmManagementFragment() - is ReaderViewModel.Event.Failure -> showError(event.error) + is ReaderViewModel.Event.Failure -> { + showError(event.error) + } else -> {} } } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderInitData.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderInitData.kt index c45c8ab13c..c8b4978f81 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderInitData.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderInitData.kt @@ -8,9 +8,8 @@ package org.readium.r2.testapp.reader +import org.readium.adapters.pdfium.navigator.PdfiumNavigatorFactory import org.readium.adapters.pdfium.navigator.PdfiumPreferences -import org.readium.adapters.pdfium.navigator.PdfiumPreferencesEditor -import org.readium.adapters.pdfium.navigator.PdfiumSettings import org.readium.r2.navigator.epub.EpubNavigatorFactory import org.readium.r2.navigator.epub.EpubPreferences import org.readium.r2.navigator.media3.exoplayer.ExoPlayerNavigator @@ -18,7 +17,6 @@ import org.readium.r2.navigator.media3.exoplayer.ExoPlayerNavigatorFactory import org.readium.r2.navigator.media3.exoplayer.ExoPlayerPreferences import org.readium.r2.navigator.media3.tts.AndroidTtsNavigatorFactory import org.readium.r2.navigator.media3.tts.android.AndroidTtsPreferences -import org.readium.r2.navigator.pdf.PdfNavigatorFactory import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.* import org.readium.r2.testapp.reader.preferences.PreferencesManager @@ -56,7 +54,7 @@ class PdfReaderInitData( publication: Publication, initialLocation: Locator?, val preferencesManager: PreferencesManager, - val navigatorFactory: PdfNavigatorFactory, + val navigatorFactory: PdfiumNavigatorFactory, ttsInitData: TtsInitData? ) : VisualReaderInitData(bookId, publication, initialLocation, ttsInitData) diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt index 8332e08adf..347744ad2d 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt @@ -19,8 +19,10 @@ import kotlinx.coroutines.flow.* import kotlinx.coroutines.launch import org.readium.r2.navigator.Decoration import org.readium.r2.navigator.ExperimentalDecorator +import org.readium.r2.navigator.epub.EpubNavigatorFragment +import org.readium.r2.navigator.image.ImageNavigatorFragment +import org.readium.r2.navigator.pdf.PdfNavigatorFragment import org.readium.r2.shared.ExperimentalReadiumApi -import org.readium.r2.shared.Search import org.readium.r2.shared.UserException import org.readium.r2.shared.publication.Locator import org.readium.r2.shared.publication.LocatorCollection @@ -28,7 +30,9 @@ import org.readium.r2.shared.publication.Publication import org.readium.r2.shared.publication.services.search.SearchIterator import org.readium.r2.shared.publication.services.search.SearchTry import org.readium.r2.shared.publication.services.search.search +import org.readium.r2.shared.resource.Resource import org.readium.r2.shared.util.Try +import org.readium.r2.shared.util.Url import org.readium.r2.testapp.Application import org.readium.r2.testapp.data.BookRepository import org.readium.r2.testapp.data.model.Highlight @@ -39,12 +43,15 @@ import org.readium.r2.testapp.utils.EventChannel import org.readium.r2.testapp.utils.createViewModelFactory import timber.log.Timber -@OptIn(Search::class, ExperimentalDecorator::class, ExperimentalCoroutinesApi::class) +@OptIn(ExperimentalDecorator::class, ExperimentalCoroutinesApi::class) class ReaderViewModel( private val bookId: Long, private val readerRepository: ReaderRepository, private val bookRepository: BookRepository -) : ViewModel() { +) : ViewModel(), + EpubNavigatorFragment.Listener, + ImageNavigatorFragment.Listener, + PdfNavigatorFragment.Listener { val readerInitData = try { @@ -238,6 +245,18 @@ class ReaderViewModel( SearchPagingSource(listener = PagingSourceListener()) } + // Navigator.Listener + + override fun onResourceLoadFailed(href: Url, error: Resource.Exception) { + val message = when (error) { + is Resource.Exception.OutOfMemory -> "The resource is too large to be rendered on this device: $href" + else -> "Failed to render the resource: $href" + } + activityChannel.send(Event.Failure(UserException(message, error))) + } + + // Search + inner class PagingSourceListener : SearchPagingSource.Listener { override suspend fun next(): SearchTry { val iterator = searchIterator ?: return Try.success(null) diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index b0bbb61446..5024720eb3 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -66,7 +66,7 @@ import org.readium.r2.testapp.utils.extensions.throttleLatest * Provides common menu items and saves last location on stop. */ @OptIn(ExperimentalDecorator::class, ExperimentalReadiumApi::class) -abstract class VisualReaderFragment : BaseReaderFragment(), VisualNavigator.Listener { +abstract class VisualReaderFragment : BaseReaderFragment() { protected var binding: FragmentReaderBinding by viewLifecycle() @@ -93,10 +93,10 @@ abstract class VisualReaderFragment : BaseReaderFragment(), VisualNavigator.List (navigator as VisualNavigator).apply { // This will automatically turn pages when tapping the screen edges or arrow keys. - addInputListener(DirectionalNavigationAdapter()) + addInputListener(DirectionalNavigationAdapter(this)) addInputListener(object : InputListener { - override fun onTap(navigator: VisualNavigator, event: TapEvent): Boolean { + override fun onTap(event: TapEvent): Boolean { requireActivity().toggleSystemUi() return true }