fix: keep the start handle on a paragraph's first word

Long-pressing the first word of a paragraph selected it but painted
the start handle on the paragraph's last hyphen, or off the page when
that hyphen was in another column. Dragging then grabbed the whole
paragraph and the reader could not shrink it. This is an Android
WebView bug (Chromium 522869957): a hyphen's offsets are relative to
itself, so a selection starting at the very beginning of a paragraph
makes every hyphen claim the start bound.

Each block that opens with text now begins with two invisible
non-breaking spaces at zero size, so the real first letter is never at
the offset that trips the bug. Nothing moves on the page, drop caps
keep their letter, and highlights and positions never see the spaces.
Blocks that only contain other blocks, and blocks whose author already
generates content there, are left alone.

Closes #189
Refs #125

AGENTS.md

@@ -446,4 +446,20 @@ emulator.
   the gesture — leaving, a rotation, a window resize — goes through
   `PageTurnDrag.abandon`, which puts the page back without animating,
   since an animation wants a next frame there may not be one of.
+- What a stylesheet alone cannot put right in the book's document is
+  repaired in the page by `repairPage()`: `WideContentFit`,
+  `FootnoteLayout` and `SelectionHandleFix` each inject a runtime
+  `<style>` and write only token-owned attributes back, never the author's
+  classes or markup. Each reports `changed`/`stable`/`blocked`/`failed` so
+  the caller knows whether the reader's place has to be put back.
+  `SelectionHandleFix` works around a WebView
+  bug (Chromium 522869957): selecting a paragraph's first word paints the
+  start handle on its last hyphen. It leads each block with two
+  non-breaking spaces at `font-size: 0` — NBSP, not ZWSP, so `::first-letter`
+  still finds the drop cap; two, since offset 1 triggers the bug too — and
+  only on blocks whose first in-flow child is inline with no authored
+  `::before`, because on a block of blocks that pseudo gets a line of its
+  own. It checks the computed pseudo-element after marking and removes the
+  token if a higher-specificity authored rule leaves the reset unsafe. Never
+  write it as a blanket `p::before`.
 - Bundled fonts must be under open licenses (OFL): Literata et al.

app/src/main/kotlin/com/chmouel/liseur/reader/ReaderScreen.kt

@@ -2710,7 +2710,13 @@ fun ReaderScreen(
                                 if (fragment.isNotEmpty()) {
                                     reflow.within {
                                         val before = ExactLocatorAnchor.layoutSignature(nav)
-                                        if (FootnoteLayout.reveal(nav, fragment)) {
+                                        val revealed = FootnoteLayout.reveal(nav, fragment)
+                                        // The hidden note was skipped when the
+                                        // page's repairs first ran. Once it
+                                        // is visible, mark its first blocks
+                                        // before navigating to it.
+                                        val repaired = reflowableText && repairPage(nav)
+                                        if (revealed || repaired) {
                                             awaitReflowSettled(nav, before)
                                         }
                                     }
@@ -3148,14 +3154,14 @@ class ReaderAnnotationActions(
 /**
  * Puts right what a stylesheet alone cannot, in the document on screen.
  *
- * Two unrelated faults are repaired the same way — by measuring the live
+ * Three unrelated faults are repaired the same way — by measuring the live
  * DOM and writing an attribute back into it — so they are asked together
  * and answer as one: whether the page moved. Each is independent of the
- * other and neither is allowed to hide the other's answer, because the
+ * others and none is allowed to hide another's answer, because the
  * caller uses it to decide whether the reader's place has to be put back.
  *
  * A refusal or a failure is not a change. The book's own
- * Content-Security-Policy can turn either of these down, and a page that
+ * Content-Security-Policy can turn any of these down, and a page that
  * was never touched is a page that never moved.
  *
  * A note the navigator's own position names is kept: that fragment is
@@ -3166,7 +3172,8 @@ private suspend fun repairPage(nav: EpubNavigatorFragment): Boolean {
     val fitted = WideContentFit.apply(nav) == WideContentFit.Result.CHANGED
     val here = nav.currentLocator.value.locations.fragments
     val noted = FootnoteLayout.apply(nav, here) == FootnoteLayout.Result.CHANGED
-    return fitted || noted
+    val led = SelectionHandleFix.apply(nav) == SelectionHandleFix.Result.CHANGED
+    return fitted || noted || led
 }
 
 /**

app/src/main/kotlin/com/chmouel/liseur/reader/SelectionHandleFix.kt

@@ -0,0 +1,271 @@
+package com.chmouel.liseur.reader
+
+import org.readium.r2.navigator.epub.EpubNavigatorFragment
+import org.readium.r2.shared.ExperimentalReadiumApi
+import kotlin.coroutines.cancellation.CancellationException
+
+/**
+ * Keeps the start handle of a selection on the word that was selected.
+ *
+ * Long-pressing the *first* word of a paragraph selects it correctly, but
+ * the WebView paints the start handle on the paragraph's last automatic
+ * hyphen — or nowhere, when that hyphen is in another column. Dragging
+ * either handle then re-anchors the selection on that bogus bound, so the
+ * reader gets the whole paragraph and cannot shrink it. This is Chromium
+ * issue 522869957 (and 41496034 for `­`): a hyphen is generated text
+ * whose offsets are relative to itself, `{0, 1}`, and the selection painter
+ * compares those against the paragraph's text-content offset, so a
+ * selection starting at offset 0 or 1 makes every hyphen in the paragraph
+ * claim the start bound, and the last one painted wins. The DOM range is
+ * never wrong; only the handles are. Liseur hyphenates justified text by
+ * default and publishers ship soft hyphens regardless, so the fix applies
+ * whatever the typography says.
+ *
+ * The workaround pushes the first letter of every eligible block to
+ * text-content offset 2 with generated content that takes no space:
+ * `::before { content: "\a0\a0"; font-size: 0 }`. Two characters, because
+ * offset 1 triggers the bug as surely as offset 0. Non-breaking spaces
+ * rather than zero-width spaces, because Blink's `::first-letter` skips text
+ * that is only spaces (U+00A0 included) and moves on to the real first
+ * letter, whereas a zero-width space *becomes* the drop cap. `font-size: 0`
+ * alone does not make the spaces free: `letter-spacing` and `word-spacing`
+ * inherit as computed lengths, so both are zeroed too. Pseudo content is
+ * not in the DOM, so [org.readium.r2.shared.publication.Locator]s,
+ * highlights and search never see it.
+ *
+ * The rule is keyed on a per-document token in the attribute name rather
+ * than written as `p::before` for two reasons. A `::before` on a block
+ * that contains only blocks — a `section`, a `blockquote` holding paragraphs, a `li` with
+ * a `p` inside — gets its own anonymous line box and pushes the content
+ * down a line, so only blocks whose first in-flow child is inline content
+ * are marked. And an author's own `::before` content must never be
+ * overridden, so a block that has one is left alone and keeps the bug.
+ *
+ * Injected at runtime, like [WideContentFit], because Readium decides
+ * whether to link its default stylesheet by looking for `<style` in the
+ * resource.
+ */
+internal object SelectionHandleFix {
+    internal enum class Result {
+        /** The stylesheet was installed on this run: the page may have moved. */
+        CHANGED,
+
+        /** Already in place; the layout is untouched. */
+        STABLE,
+
+        /** The book's Content-Security-Policy refused the stylesheet. */
+        BLOCKED,
+
+        /** No reflowable page to talk to, or the script did not answer. */
+        FAILED,
+    }
+
+    /**
+     * Runs in the book's own document, possibly more than once, and marks
+     * the document's blocks on every pass. The first in-flow child is a
+     * static property of the markup, but an author's `::before` may be
+     * switched on by a media query after a rotation, and the author's
+     * content wins.
+     *
+     * Ownership is a token minted per document and placed in the attribute's
+     * name, as in [com.chmouel.liseur.reader.footnotes.FootnoteLayout]:
+     * a publisher's `data-liseur-lead` attribute or CSS selector is then
+     * never read from, written over or matched by our rule. Nothing of the
+     * author's markup is touched beyond setting that token-owned attribute.
+     *
+     * Every element is a candidate, judged by its computed `display` rather
+     * than its tag, so a `figure` holding text, a `body` with no wrapper
+     * or a custom element styled as a block are treated like a `p`.
+     * Eligibility skips comments, whitespace-only text, empty inlines and
+     * elements taken out of flow (floats, absolute and fixed positioning),
+     * since none of them starts a line box, and asks the computed style for
+     * `::before`, where `none` and `normal` both mean the author generated
+     * nothing.
+     */
+    const val SCRIPT: String = """
+        (function () {
+          var state = window.__liseurLead || (window.__liseurLead = {});
+          var installed = false;
+          var tok = state.token ||
+                    (state.token = "l" + Math.random().toString(36).slice(2, 10));
+          var ATTR = state.attr || (state.attr = "data-liseur-lead-" + tok);
+          var SEL = '[' + ATTR + ']';
+
+          if (!state.styleEl || !state.styleEl.isConnected) {
+            var css = document.createElement("style");
+            var content = 'content:"\u00a0\u00a0"';
+            if (window.CSS && CSS.supports &&
+                CSS.supports("content", '"x" / ""')) {
+              content += ' / ""';
+            }
+            css.textContent =
+              SEL + '::before{' + content + ' !important;' +
+              'font-size:0!important;line-height:0!important;' +
+              'letter-spacing:0!important;word-spacing:0!important;' +
+              // An author's dormant `display: block; margin: 1em` on a
+              // contentless ::before has no box until content arrives;
+              // ours must not give it one.
+              'display:inline!important;position:static!important;' +
+              'float:none!important;margin:0!important;padding:0!important;' +
+              'border:0!important;background:none!important;' +
+              'box-shadow:none!important;outline:none!important;' +
+              'width:auto!important;height:auto!important;' +
+              'vertical-align:baseline!important;' +
+              'counter-increment:none!important;counter-reset:none!important;' +
+              'counter-set:none!important}';
+            document.head.appendChild(css);
+            state.styleEl = css;
+            installed = true;
+          }
+          if (!state.styleEl.sheet) return "blocked";
+
+          var old = document.querySelectorAll(SEL);
+          var had = new Set(), i;
+          for (i = 0; i < old.length; i++) {
+            had.add(old[i]);
+            old[i].removeAttribute(ATTR);
+          }
+          var kept = 0;
+
+          var BLOCK = {
+            "block": 1, "list-item": 1, "flow-root": 1, "inline-block": 1,
+            "table-cell": 1, "table-caption": 1
+          };
+
+          function outOfFlow(s) {
+            return s.getPropertyValue("float") !== "none" ||
+                   s.position === "absolute" || s.position === "fixed" ||
+                   s.display === "none";
+          }
+
+          // Inline elements that are a box of their own, and so start a
+          // line box even when empty.
+          var ATOMIC = {
+            "img": 1, "svg": 1, "math": 1, "video": 1, "audio": 1, "canvas": 1,
+            "iframe": 1, "object": 1, "embed": 1, "input": 1, "select": 1,
+            "textarea": 1, "button": 1, "br": 1
+          };
+
+          function inlineStart(node) {
+            if (node.nodeType === 3) {
+              return /\S/.test(node.data) ? true : null;
+            }
+            if (node.nodeType !== 1) return null;
+            var s = getComputedStyle(node);
+            if (outOfFlow(s)) return null;
+            var d = s.display;
+            if (d === "contents") return startsInline(node);
+            if (d.indexOf("inline") !== 0 && d.indexOf("ruby") !== 0) return false;
+            // A plain inline holds no line box of its own: the empty
+            // `<a id>` a chapter opens with is looked through to what
+            // follows it, and a `<span>` counts by what it holds.
+            if (d === "inline" && !ATOMIC[node.localName]) return startsInline(node);
+            return true;
+          }
+
+          function startsInline(el) {
+            var nodes = el.childNodes, answer;
+            for (var i = 0; i < nodes.length; i++) {
+              answer = inlineStart(nodes[i]);
+              if (answer !== null) return answer;
+            }
+            return null;
+          }
+
+          function zero(value) {
+            return value === "0" || value === "0px";
+          }
+
+          function autoOrZero(value) {
+            return value === "auto" || zero(value);
+          }
+
+          function neutralCounter(style, name) {
+            var value = style.getPropertyValue(name);
+            return value === "" || value === "none";
+          }
+
+          function safePseudo(style) {
+            var transparent = style.backgroundColor === "transparent" ||
+                             style.backgroundColor === "rgba(0, 0, 0, 0)";
+            return style.content !== "none" && style.content !== "normal" &&
+                   style.content !== "\"\"" &&
+                   style.display === "inline" &&
+                   style.position === "static" &&
+                   style.getPropertyValue("float") === "none" &&
+                   zero(style.fontSize) && zero(style.lineHeight) &&
+                   zero(style.letterSpacing) && zero(style.wordSpacing) &&
+                   zero(style.marginTop) && zero(style.marginRight) &&
+                   zero(style.marginBottom) && zero(style.marginLeft) &&
+                   zero(style.paddingTop) && zero(style.paddingRight) &&
+                   zero(style.paddingBottom) && zero(style.paddingLeft) &&
+                   zero(style.borderTopWidth) && zero(style.borderRightWidth) &&
+                   zero(style.borderBottomWidth) && zero(style.borderLeftWidth) &&
+                   style.backgroundImage === "none" && transparent &&
+                   style.boxShadow === "none" &&
+                   (style.outlineStyle === "none" || zero(style.outlineWidth)) &&
+                   autoOrZero(style.width) && autoOrZero(style.height) &&
+                   style.verticalAlign === "baseline" &&
+                   neutralCounter(style, "counter-increment") &&
+                   neutralCounter(style, "counter-reset") &&
+                   neutralCounter(style, "counter-set");
+          }
+
+          var els = document.getElementsByTagName("*");
+          var changed = installed;
+          for (i = 0; i < els.length; i++) {
+            var el = els[i];
+            if (!BLOCK[getComputedStyle(el).display]) continue;
+            var before = getComputedStyle(el, "::before").content;
+            if (before !== "none" && before !== "normal") continue;
+            if (startsInline(el) !== true) continue;
+            el.setAttribute(ATTR, "");
+            if (!safePseudo(getComputedStyle(el, "::before"))) {
+              // An important publisher rule with higher specificity can
+              // beat our important reset. Do not leave a marker that shifts
+              // or paints the book if the cascade did not make it safe.
+              el.removeAttribute(ATTR);
+              continue;
+            }
+            if (!had.has(el)) {
+              changed = true;
+            } else {
+              kept++;
+            }
+          }
+          if (kept !== had.size) changed = true;
+          return changed ? "changed" : "stable";
+        })();
+    """
+
+    /**
+     * `evaluateJavascript` hands back the JSON encoding of the value, so a
+     * plain string arrives wearing quotes.
+     */
+    internal fun parse(result: String?): Result {
+        val value = result?.trim()?.removeSurrounding("\"")?.trim()
+        return when (value) {
+            "changed" -> Result.CHANGED
+            "stable" -> Result.STABLE
+            "blocked" -> Result.BLOCKED
+            else -> Result.FAILED
+        }
+    }
+
+    /**
+     * A cancelled caller is let go, as in
+     * [com.chmouel.liseur.reader.footnotes.FootnoteLayout]: a navigator
+     * being replaced must not read as a page that failed to answer.
+     */
+    @OptIn(ExperimentalReadiumApi::class)
+    internal suspend fun apply(navigator: EpubNavigatorFragment): Result =
+        parse(
+            try {
+                navigator.evaluateJavascript(SCRIPT)
+            } catch (cancellation: CancellationException) {
+                throw cancellation
+            } catch (refused: Exception) {
+                null
+            },
+        )
+}

app/src/test/kotlin/com/chmouel/liseur/reader/SelectionHandleFixTest.kt

@@ -0,0 +1,175 @@
+package com.chmouel.liseur.reader
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.io.File
+
+class SelectionHandleFixTest {
+
+    @Test
+    fun `the javascript answer is read through its json quoting`() {
+        assertEquals(SelectionHandleFix.Result.CHANGED, SelectionHandleFix.parse("\"changed\""))
+        assertEquals(SelectionHandleFix.Result.STABLE, SelectionHandleFix.parse("\"stable\""))
+        assertEquals(SelectionHandleFix.Result.BLOCKED, SelectionHandleFix.parse("\"blocked\""))
+        assertEquals(SelectionHandleFix.Result.STABLE, SelectionHandleFix.parse(" stable "))
+    }
+
+    @Test
+    fun `no answer is a failure, never a silent success`() {
+        assertEquals(SelectionHandleFix.Result.FAILED, SelectionHandleFix.parse(null))
+        assertEquals(SelectionHandleFix.Result.FAILED, SelectionHandleFix.parse(""))
+        assertEquals(SelectionHandleFix.Result.FAILED, SelectionHandleFix.parse("null"))
+        assertEquals(SelectionHandleFix.Result.FAILED, SelectionHandleFix.parse("\"whatever\""))
+    }
+
+    @Test
+    fun `the script is one expression, so evaluateJavascript returns its value`() {
+        val script = SelectionHandleFix.SCRIPT.trim()
+        assertTrue(script.startsWith("(function"))
+        assertTrue(script.endsWith("})();"))
+    }
+
+    @Test
+    fun `two non-breaking spaces lead the block, since offset one still triggers the bug`() {
+        // U+00A0, not a zero-width space: Blink's ::first-letter skips spaces
+        // and lands on the real letter, whereas a ZWSP would become the drop cap.
+        assertTrue(SelectionHandleFix.SCRIPT.contains("\\u00a0\\u00a0"))
+        assertFalse(SelectionHandleFix.SCRIPT.contains("\\u200b"))
+        assertFalse(SelectionHandleFix.SCRIPT.contains("\\200b"))
+    }
+
+    @Test
+    fun `the spaces take no room and use generated content alt text where available`() {
+        // Font size alone is not enough: letter- and word-spacing inherit as
+        // computed lengths and would give the invisible spaces a width.
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("font-size:0!important"))
+        assertTrue(script.contains("line-height:0!important"))
+        assertTrue(script.contains("letter-spacing:0!important"))
+        assertTrue(script.contains("word-spacing:0!important"))
+        assertTrue(script.contains("CSS.supports(\"content\", '\"x\" / \"\"')"))
+        assertTrue(script.contains("content += ' / \"\"'"))
+        assertTrue(script.contains("+ ' !important;'"))
+        assertTrue(script.contains("content:\"\\u00a0\\u00a0\""))
+    }
+
+    @Test
+    fun `an author's dormant pseudo-element styles get no box from our content`() {
+        // `p::before { display: block; margin-top: 1em }` with no content
+        // renders nothing until content arrives; ours must not wake it.
+        val script = SelectionHandleFix.SCRIPT
+        for (reset in listOf(
+            "display:inline!important", "position:static!important", "float:none!important",
+            "margin:0!important", "padding:0!important", "border:0!important",
+            "background:none!important", "width:auto!important", "height:auto!important",
+            "box-shadow:none!important", "outline:none!important",
+            "vertical-align:baseline!important", "counter-increment:none!important",
+            "counter-reset:none!important", "counter-set:none!important",
+        )) {
+            assertTrue(reset, script.contains(reset))
+        }
+    }
+
+    @Test
+    fun `the rule is keyed on a per-document token, never a blanket selector`() {
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("state.token"))
+        assertTrue(script.contains("state.attr"))
+        assertTrue(script.contains("data-liseur-lead-"))
+        assertTrue(script.contains("SEL = '[' + ATTR + ']'"))
+        assertTrue(script.contains("SEL + '::before"))
+        assertFalse(script.contains("p::before"))
+        assertFalse(script.contains("*::before"))
+        assertFalse(script.contains("getElementById"))
+        assertFalse(script.contains("\"data-liseur-lead\""))
+    }
+
+    @Test
+    fun `only blocks that open with inline content are marked`() {
+        // A ::before on a block of blocks gets its own line box and pushes the
+        // content down a line, so the first in-flow child is what decides.
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("childNodes"))
+        assertTrue(script.contains("s.getPropertyValue(\"float\") !== \"none\""))
+        assertTrue(script.contains("d === \"contents\""))
+        assertTrue(script.contains("startsInline(el) !== true"))
+        assertTrue(script.contains("return null"))
+        assertTrue(script.contains("indexOf(\"inline\") !== 0 && d.indexOf(\"ruby\") !== 0) return false"))
+    }
+
+    @Test
+    fun `every block is a candidate whatever its tag`() {
+        // A figure holding text or a custom element styled as a block has
+        // the bug as much as a p; the computed display decides, not the tag.
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("getElementsByTagName(\"*\")"))
+        assertFalse(script.contains("\"p,li"))
+        assertFalse(script.contains("SKIP"))
+    }
+
+    @Test
+    fun `an empty inline is looked through`() {
+        // A chapter often opens with `<a id="..."></a>` before its heading;
+        // treating that anchor as inline content would mark the section and
+        // put a blank line above the heading.
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("if (d === \"inline\" && !ATOMIC[node.localName]) return startsInline(node)"))
+        assertTrue(script.contains("\"img\": 1"))
+        assertTrue(script.contains("\"br\": 1"))
+    }
+
+    @Test
+    fun `a cancelled caller is not reported as a failed page`() {
+        val source = File(System.getProperty("user.dir"), "src/main/kotlin/com/chmouel/liseur/reader/SelectionHandleFix.kt").readText()
+        assertTrue(source.contains("catch (cancellation: CancellationException)"))
+        assertTrue(source.contains("throw cancellation"))
+        assertFalse(source.contains("runCatching"))
+    }
+
+    @Test
+    fun `an author's own generated content is left alone`() {
+        assertTrue(SelectionHandleFix.SCRIPT.contains("getComputedStyle(el, \"::before\").content"))
+        assertTrue(SelectionHandleFix.SCRIPT.contains("before !== \"none\" && before !== \"normal\""))
+    }
+
+    @Test
+    fun `the author's own markup is never rewritten`() {
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("setAttribute(ATTR, \"\")"))
+        assertTrue(script.contains("removeAttribute(ATTR)"))
+        assertFalse(script.contains("className"))
+        assertFalse(script.contains("classList"))
+        assertFalse(script.contains("innerHTML"))
+        assertFalse(script.contains("insertBefore"))
+        assertFalse(script.contains("createTextNode"))
+    }
+
+    @Test
+    fun `eligibility is rechecked on every pass`() {
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("document.querySelectorAll(SEL)"))
+        assertTrue(script.contains("removeAttribute(ATTR)"))
+        assertTrue(script.contains("new Set()"))
+        assertTrue(script.contains("had.has(el)"))
+        assertTrue(script.contains("had.size"))
+        assertTrue(script.contains("getComputedStyle(el, \"::before\").content"))
+        assertFalse(script.contains("state.marked"))
+    }
+
+    @Test
+    fun `a higher specificity authored rule cannot leave an unsafe marker`() {
+        val script = SelectionHandleFix.SCRIPT
+        assertTrue(script.contains("function safePseudo(style)"))
+        assertTrue(script.contains("if (!safePseudo(getComputedStyle(el, \"::before\")))"))
+        assertTrue(script.contains("el.removeAttribute(ATTR)"))
+        assertTrue(script.contains("neutralCounter(style, name)"))
+    }
+
+    @Test
+    fun `a stylesheet the book's policy refused is reported rather than assumed`() {
+        assertTrue(SelectionHandleFix.SCRIPT.contains("if (!state.styleEl.sheet) return \"blocked\""))
+        assertFalse(SelectionHandleFix.SCRIPT.contains("if (!css.sheet) return \"blocked\""))
+    }
+}