Skip to content

Commit cdfe876

Browse files
mickael-menumickael-menu
authored
Various bugfixes (#405)
1 parent 31ee129 commit cdfe876

File tree

21 files changed

+179
-118
lines changed

21 files changed

+179
-118
lines changed

‎readium/adapters/pspdfkit/navigator/src/main/java/org/readium/adapter/pspdfkit/navigator/PsPdfKitDocumentFragment.kt‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import org.readium.r2.shared.util.Url
5757
import org.readium.r2.shared.util.pdf.cachedIn
5858
import org.readium.r2.shared.util.resource.Resource
5959
import org.readium.r2.shared.util.resource.ResourceTry
60+
import timber.log.Timber
6061

6162
@ExperimentalReadiumApi
6263
public class PsPdfKitDocumentFragment internal constructor(
@@ -162,7 +163,7 @@ public class PsPdfKitDocumentFragment internal constructor(
162163
* Recreates the [PdfFragment] with the current settings.
163164
*/
164165
private fun resetPdfFragment() {
165-
if (view == null) return
166+
if (isStateSaved || view == null) return
166167
val doc = viewModel.document ?: return
167168

168169
doc.document.pageBinding = settings.readingProgression.pageBinding
@@ -295,9 +296,15 @@ public class PsPdfKitDocumentFragment internal constructor(
295296
}
296297

297298
override fun onDocumentLoaded(document: PdfDocument) {
298-
super.onDocumentLoaded(document)
299+
val index = pageIndex.value
300+
if (index < 0 || index >= document.pageCount) {
301+
Timber.w(
302+
"Tried to restore page index $index, but the document has ${document.pageCount} pages"
303+
)
304+
return
305+
}
299306

300-
checkNotNull(pdfFragment).setPageIndex(pageIndex.value, false)
307+
checkNotNull(pdfFragment).setPageIndex(index, false)
301308
}
302309
}
303310

‎readium/lcp/src/main/java/org/readium/r2/lcp/LcpContentProtection.kt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,10 @@ internal class LcpContentProtection(
113113
LicenseDocument(it)
114114
} catch (e: Exception) {
115115
return Try.failure(
116-
Publication.OpenError.InvalidAsset(cause = ThrowableError(e))
116+
Publication.OpenError.InvalidAsset(
117+
"Failed to read the LCP license document",
118+
cause = ThrowableError(e)
119+
)
117120
)
118121
}
119122
}
@@ -127,9 +130,7 @@ internal class LcpContentProtection(
127130
val url = (link.url() as? AbsoluteUrl)
128131
?: return Try.failure(
129132
Publication.OpenError.InvalidAsset(
130-
cause = ThrowableError(
131-
LcpException.Parsing.Url(rel = LicenseDocument.Rel.Publication.value)
132-
)
133+
"The LCP license document does not contain a valid link to the publication"
133134
)
134135
)
135136

‎readium/lcp/src/main/java/org/readium/r2/lcp/LcpPublicationRetriever.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import kotlinx.coroutines.launch
1313
import org.readium.r2.lcp.license.container.createLicenseContainer
1414
import org.readium.r2.lcp.license.model.LicenseDocument
1515
import org.readium.r2.shared.extensions.tryOrLog
16+
import org.readium.r2.shared.util.ErrorException
1617
import org.readium.r2.shared.util.downloads.DownloadManager
1718
import org.readium.r2.shared.util.mediatype.FormatRegistry
1819
import org.readium.r2.shared.util.mediatype.MediaType
@@ -255,7 +256,7 @@ public class LcpPublicationRetriever(
255256
listenersForId.forEach {
256257
it.onAcquisitionFailed(
257258
lcpRequestId,
258-
LcpException.Network(Exception(error.message))
259+
LcpException.Network(ErrorException(error))
259260
)
260261
}
261262

‎readium/navigators/media/tts/src/main/java/org/readium/navigator/media/tts/TtsNavigator.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ public class TtsNavigator<S : TtsEngine.Settings, P : TtsEngine.Preferences<P>,
7676

7777
val contentIterator =
7878
TtsUtteranceIterator(publication, tokenizerFactory, initialLocator)
79+
if (!contentIterator.hasNext()) {
80+
return null
81+
}
7982

8083
val ttsEngine =
8184
ttsEngineProvider.createEngine(publication, actualInitialPreferences)

‎readium/navigators/media/tts/src/main/java/org/readium/navigator/media/tts/TtsNavigatorFactory.kt‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class TtsNavigatorFactory<S : TtsEngine.Settings, P : TtsEngine.Preferenc
3434
) {
3535
public companion object {
3636

37-
public suspend operator fun invoke(
37+
public operator fun invoke(
3838
application: Application,
3939
publication: Publication,
4040
tokenizerFactory: (language: Language?) -> TextTokenizer = defaultTokenizerFactory,
@@ -57,35 +57,31 @@ public class TtsNavigatorFactory<S : TtsEngine.Settings, P : TtsEngine.Preferenc
5757
)
5858
}
5959

60-
public suspend operator fun <S : TtsEngine.Settings, P : TtsEngine.Preferences<P>, E : PreferencesEditor<P>,
60+
public operator fun <S : TtsEngine.Settings, P : TtsEngine.Preferences<P>, E : PreferencesEditor<P>,
6161
F : TtsEngine.Error, V : TtsEngine.Voice> invoke(
6262
application: Application,
6363
publication: Publication,
6464
ttsEngineProvider: TtsEngineProvider<S, P, E, F, V>,
6565
tokenizerFactory: (language: Language?) -> TextTokenizer = defaultTokenizerFactory,
6666
metadataProvider: MediaMetadataProvider = defaultMediaMetadataProvider
67-
): TtsNavigatorFactory<S, P, E, F, V>? {
68-
return createNavigatorFactory(
67+
): TtsNavigatorFactory<S, P, E, F, V>? =
68+
createNavigatorFactory(
6969
application,
7070
publication,
7171
ttsEngineProvider,
7272
tokenizerFactory,
7373
metadataProvider
7474
)
75-
}
7675

77-
private suspend fun <S : TtsEngine.Settings, P : TtsEngine.Preferences<P>, E : PreferencesEditor<P>,
76+
private fun <S : TtsEngine.Settings, P : TtsEngine.Preferences<P>, E : PreferencesEditor<P>,
7877
F : TtsEngine.Error, V : TtsEngine.Voice> createNavigatorFactory(
7978
application: Application,
8079
publication: Publication,
8180
ttsEngineProvider: TtsEngineProvider<S, P, E, F, V>,
8281
tokenizerFactory: (language: Language?) -> TextTokenizer,
8382
metadataProvider: MediaMetadataProvider
8483
): TtsNavigatorFactory<S, P, E, F, V>? {
85-
publication.content()
86-
?.iterator()
87-
?.takeIf { it.hasNext() }
88-
?: return null
84+
publication.content() ?: return null
8985

9086
return TtsNavigatorFactory(
9187
application,

‎readium/navigators/media/tts/src/main/java/org/readium/navigator/media/tts/TtsUtteranceIterator.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,19 @@ internal class TtsUtteranceIterator(
105105
private fun createIterator(locator: Locator?): Content.Iterator =
106106
contentService.content(locator).iterator()
107107

108+
suspend fun hasPrevious(): Boolean =
109+
hasNextIn(Direction.Backward)
110+
111+
suspend fun hasNext(): Boolean =
112+
hasNextIn(Direction.Forward)
113+
114+
private suspend fun hasNextIn(direction: Direction): Boolean {
115+
if (utterances.isEmpty()) {
116+
loadNextUtterances(direction)
117+
}
118+
return utterances.hasNextIn(direction)
119+
}
120+
108121
/**
109122
* Advances to the previous item and returns it, or null if we reached the beginning.
110123
*/
@@ -217,6 +230,12 @@ internal class TtsUtteranceIterator(
217230
}
218231
}
219232

233+
private fun <E> CursorList<E>.hasNextIn(direction: Direction): Boolean =
234+
when (direction) {
235+
Direction.Forward -> hasNext()
236+
Direction.Backward -> hasPrevious()
237+
}
238+
220239
private fun <E> CursorList<E>.nextIn(direction: Direction): E? =
221240
when (direction) {
222241
Direction.Forward -> if (hasNext()) next() else null

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import org.readium.r2.shared.publication.services.PositionsService
2727
import org.readium.r2.shared.publication.services.WebPositionsService
2828
import org.readium.r2.shared.publication.services.content.ContentService
2929
import org.readium.r2.shared.publication.services.search.SearchService
30-
import org.readium.r2.shared.util.BaseError
3130
import org.readium.r2.shared.util.Closeable
3231
import org.readium.r2.shared.util.Error
3332
import org.readium.r2.shared.util.ThrowableError
@@ -497,8 +496,10 @@ public class Publication(
497496
/**
498497
* Errors occurring while opening a Publication.
499498
*/
500-
public sealed class OpenError(message: String, cause: Error? = null) :
501-
BaseError(message, cause) {
499+
public sealed class OpenError(
500+
override val message: String,
501+
override val cause: Error? = null
502+
) : Error {
502503

503504
/**
504505
* The file format could not be recognized by any parser.
@@ -514,12 +515,11 @@ public class Publication(
514515
/**
515516
* The publication parsing failed with the given underlying error.
516517
*/
517-
public class InvalidAsset private constructor(
518+
public class InvalidAsset(
518519
message: String,
519-
cause: Error?
520+
cause: Error? = null
520521
) : OpenError(message, cause) {
521-
public constructor(message: String) : this(message, null)
522-
public constructor(cause: Error? = null) : this(
522+
public constructor(cause: Error?) : this(
523523
"The asset seems corrupted so the publication cannot be opened.",
524524
cause
525525
)

‎readium/shared/src/main/java/org/readium/r2/shared/publication/services/content/iterators/HtmlResourceContentIterator.kt‎

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
package org.readium.r2.shared.publication.services.content.iterators
88

9+
import kotlinx.coroutines.Dispatchers
10+
import kotlinx.coroutines.withContext
911
import org.jsoup.Jsoup
1012
import org.jsoup.nodes.Element
1113
import org.jsoup.nodes.Node
@@ -14,6 +16,7 @@ import org.jsoup.parser.Parser
1416
import org.jsoup.select.NodeTraversor
1517
import org.jsoup.select.NodeVisitor
1618
import org.readium.r2.shared.ExperimentalReadiumApi
19+
import org.readium.r2.shared.extensions.tryOrLog
1720
import org.readium.r2.shared.extensions.tryOrNull
1821
import org.readium.r2.shared.publication.Link
1922
import org.readium.r2.shared.publication.Locator
@@ -146,43 +149,44 @@ public class HtmlResourceContentIterator internal constructor(
146149

147150
private var parsedElements: ParsedElements? = null
148151

149-
private suspend fun parseElements(): ParsedElements {
150-
val document = resource.use { res ->
151-
val html = res.readAsString().getOrElse {
152-
Timber.w(it, "Failed to read HTML resource")
153-
return ParsedElements()
152+
private suspend fun parseElements(): ParsedElements =
153+
withContext(Dispatchers.Default) {
154+
val document = resource.use { res ->
155+
val html = res.readAsString().getOrElse {
156+
Timber.w(it, "Failed to read HTML resource")
157+
return@withContext ParsedElements()
158+
}
159+
160+
Jsoup.parse(html)
154161
}
155162

156-
Jsoup.parse(html)
157-
}
163+
val contentParser = ContentParser(
164+
baseLocator = locator,
165+
startElement = locator.locations.cssSelector?.let {
166+
tryOrNull { document.selectFirst(it) }
167+
},
168+
beforeMaxLength = beforeMaxLength
169+
)
170+
NodeTraversor.traverse(contentParser, document.body())
171+
val elements = contentParser.result()
172+
val elementCount = elements.elements.size
173+
if (elementCount == 0) {
174+
return@withContext elements
175+
}
158176

159-
val contentParser = ContentParser(
160-
baseLocator = locator,
161-
startElement = locator.locations.cssSelector?.let {
162-
tryOrNull { document.selectFirst(it) }
163-
},
164-
beforeMaxLength = beforeMaxLength
165-
)
166-
NodeTraversor.traverse(contentParser, document.body())
167-
val elements = contentParser.result()
168-
val elementCount = elements.elements.size
169-
if (elementCount == 0) {
170-
return elements
177+
elements.copy(
178+
elements = elements.elements.mapIndexed { index, element ->
179+
val progression = index.toDouble() / elementCount
180+
element.copy(
181+
progression = progression,
182+
totalProgression = totalProgressionRange?.let {
183+
totalProgressionRange.start + progression * (totalProgressionRange.endInclusive - totalProgressionRange.start)
184+
}
185+
)
186+
}
187+
)
171188
}
172189

173-
return elements.copy(
174-
elements = elements.elements.mapIndexed { index, element ->
175-
val progression = index.toDouble() / elementCount
176-
element.copy(
177-
progression = progression,
178-
totalProgression = totalProgressionRange?.let {
179-
totalProgressionRange.start + progression * (totalProgressionRange.endInclusive - totalProgressionRange.start)
180-
}
181-
)
182-
}
183-
)
184-
}
185-
186190
private fun Content.Element.copy(progression: Double?, totalProgression: Double?): Content.Element {
187191
fun Locator.update(): Locator =
188192
copyWithLocations(
@@ -256,9 +260,12 @@ public class HtmlResourceContentIterator internal constructor(
256260

257261
private data class ParentElement(
258262
val element: Element,
259-
val cssSelector: String
263+
val cssSelector: String?
260264
) {
261-
constructor(element: Element) : this(element, element.cssSelector())
265+
constructor(element: Element) : this(
266+
element = element,
267+
cssSelector = tryOrLog { element.cssSelector() }
268+
)
262269
}
263270

264271
override fun head(node: Node, depth: Int) {
@@ -275,7 +282,9 @@ public class HtmlResourceContentIterator internal constructor(
275282
baseLocator.copy(
276283
locations = Locator.Locations(
277284
otherLocations = buildMap {
278-
put("cssSelector", parent.cssSelector as Any)
285+
parent.cssSelector?.let {
286+
put("cssSelector", it as Any)
287+
}
279288
}
280289
)
281290
)
@@ -402,8 +411,8 @@ public class HtmlResourceContentIterator internal constructor(
402411
locator = baseLocator.copy(
403412
locations = Locator.Locations(
404413
otherLocations = buildMap {
405-
parent?.let {
406-
put("cssSelector", it.cssSelector as Any)
414+
parent?.cssSelector?.let {
415+
put("cssSelector", it as Any)
407416
}
408417
}
409418
),
@@ -442,8 +451,8 @@ public class HtmlResourceContentIterator internal constructor(
442451
locator = baseLocator.copy(
443452
locations = Locator.Locations(
444453
otherLocations = buildMap {
445-
parent?.let {
446-
put("cssSelector", it.cssSelector as Any)
454+
parent?.cssSelector?.let {
455+
put("cssSelector", it as Any)
447456
}
448457
}
449458
),

0 commit comments

Comments
 (0)