Bugfixes for TTS (#390)

Author: mickael-menu

readium/navigator/src/main/java/org/readium/r2/navigator/media3/tts/TtsEngineFacade.kt

@@ -26,8 +26,6 @@ internal class TtsEngineFacade<S : TtsEngine.Settings, P : TtsEngine.Preferences
         engine.setListener(listener)
     }
 
-    private var currentTask: UtteranceTask<E>? = null
-
     val voices: Set<V>
         get() = engine.voices
 
@@ -45,54 +43,44 @@ internal class TtsEngineFacade<S : TtsEngine.Settings, P : TtsEngine.Preferences
         engine.close()
     }
 
+    private var currentTask: UtteranceTask<E>? = null
+
     private data class UtteranceTask<E : TtsEngine.Error>(
         val requestId: TtsEngine.RequestId,
         val continuation: CancellableContinuation<E?>,
         val onRange: (IntRange) -> Unit
     )
 
+    private fun getTask(id: TtsEngine.RequestId) =
+        currentTask?.takeIf { it.requestId == id }
+
+    private fun popTask(id: TtsEngine.RequestId) =
+        getTask(id)
+            ?.also { currentTask = null }
+
     private inner class EngineListener : TtsEngine.Listener<E> {
 
         override fun onStart(requestId: TtsEngine.RequestId) {
         }
 
         override fun onRange(requestId: TtsEngine.RequestId, range: IntRange) {
-            currentTask
-                ?.takeIf { it.requestId == requestId }
-                ?.onRange
-                ?.invoke(range)
+            getTask(requestId)?.onRange?.invoke(range)
         }
 
         override fun onInterrupted(requestId: TtsEngine.RequestId) {
-            currentTask
-                ?.takeIf { it.requestId == requestId }
-                ?.continuation
-                ?.cancel()
-            currentTask = null
+            popTask(requestId)?.continuation?.cancel()
         }
 
         override fun onFlushed(requestId: TtsEngine.RequestId) {
-            currentTask
-                ?.takeIf { it.requestId == requestId }
-                ?.continuation
-                ?.cancel()
-            currentTask = null
+            popTask(requestId)?.continuation?.cancel()
         }
 
         override fun onDone(requestId: TtsEngine.RequestId) {
-            currentTask
-                ?.takeIf { it.requestId == requestId }
-                ?.continuation
-                ?.resume(null) {}
-            currentTask = null
+            popTask(requestId)?.continuation?.resume(null) {}
         }
 
         override fun onError(requestId: TtsEngine.RequestId, error: E) {
-            currentTask
-                ?.takeIf { it.requestId == requestId }
-                ?.continuation
-                ?.resume(error) {}
-            currentTask = null
+            popTask(requestId)?.continuation?.resume(error) {}
         }
     }
 }

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

@@ -134,14 +134,17 @@ public data class Locator(
         }
 
         public fun substring(range: IntRange): Text {
-            highlight ?: return this
-            return tryOr(this) {
-                copy(
-                    before = (before ?: "") + highlight.substring(0, range.first),
-                    highlight = highlight.substring(range),
-                    after = highlight.substring(range.last) + (after ?: "")
-                )
-            }
+            if (highlight.isNullOrBlank()) return this
+
+            val fixedRange = range.first.coerceIn(0, highlight.length)..range.last.coerceIn(
+                0,
+                highlight.length - 1
+            )
+            return copy(
+                before = (before ?: "") + highlight.substring(0, fixedRange.first),
+                highlight = highlight.substring(fixedRange),
+                after = highlight.substring((fixedRange.last + 1).coerceAtMost(highlight.length)) + (after ?: "")
+            )
         }
 
         public companion object {

readium/shared/src/test/java/org/readium/r2/shared/publication/LocatorTest.kt

@@ -337,6 +337,131 @@ class LocatorTest {
             ).toJSON()
         )
     }
+
+    @Test fun `substring from a range`() {
+        val text = Locator.Text(
+            before = "before",
+            highlight = "highlight",
+            after = "after"
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "before",
+                highlight = "h",
+                after = "ighlightafter"
+            ),
+            text.substring(0..-1)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "before",
+                highlight = "h",
+                after = "ighlightafter"
+            ),
+            text.substring(0..0)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "beforehigh",
+                highlight = "lig",
+                after = "htafter"
+            ),
+            text.substring(4..6)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "before",
+                highlight = "highlight",
+                after = "after"
+            ),
+            text.substring(0..8)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "beforehighli",
+                highlight = "ght",
+                after = "after"
+            ),
+            text.substring(6..12)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "beforehighligh",
+                highlight = "t",
+                after = "after"
+            ),
+            text.substring(8..12)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "beforehighlight",
+                highlight = "",
+                after = "after"
+            ),
+            text.substring(9..12)
+        )
+    }
+
+    @Test fun `substring from a range with null components`() {
+        assertEquals(
+            Locator.Text(
+                before = "high",
+                highlight = "lig",
+                after = "htafter"
+            ),
+            Locator.Text(
+                before = null,
+                highlight = "highlight",
+                after = "after"
+            ).substring(4..6)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "beforehigh",
+                highlight = "lig",
+                after = "ht"
+            ),
+            Locator.Text(
+                before = "before",
+                highlight = "highlight",
+                after = null
+            ).substring(4..6)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "before",
+                highlight = null,
+                after = "after"
+            ),
+            Locator.Text(
+                before = "before",
+                highlight = null,
+                after = "after"
+            ).substring(4..6)
+        )
+
+        assertEquals(
+            Locator.Text(
+                before = "before",
+                highlight = "",
+                after = "after"
+            ),
+            Locator.Text(
+                before = "before",
+                highlight = "",
+                after = "after"
+            ).substring(4..6)
+        )
+    }
 }
 
 @RunWith(RobolectricTestRunner::class)