fix(ime): ⌫ deletes the selection — empty commitText instead of caret-only deleteSurroundingText

analytics-events.json

@@ -96,7 +96,7 @@
       "properties": {
         "surface": { "type": "string", "enum": ["main_toolbar", "status_bar", "ime_toolbar", "ime_topbar"] },
         "action": { "type": "string", "description": "Fixed vocabulary; new identifiers require a schema update", "examples": ["new", "open", "save", "pause_play", "game_mode", "settings", "copy", "share", "emoji_toggle", "speed_up", "speed_down", "layout", "float", "dock", "hide", "cursor_left", "cursor_right", "backspace", "select_all", "paste"] },
-        "state": { "type": "string", "enum": ["on", "off", "paused", "playing", "text", "emoji"], "description": "Optional — for toggles" }
+        "state": { "type": "string", "enum": ["on", "off", "paused", "playing", "text", "emoji", "selection", "char", "none", "no_connection"], "description": "Optional — toggle states, or the delete kind for the backspace action" }
       },
       "pii": false
     },

app/src/main/java/at/dasher/android/EditingToolbar.kt

@@ -54,7 +54,15 @@ class EditingToolbar(
         addTool(R.drawable.ic_lucide_arrow_left, "Move cursor left", mutates = true) { moveCursor(-1) }
         addTool(R.drawable.ic_lucide_arrow_right, "Move cursor right", mutates = true) { moveCursor(1) }
         addSpacer(dp(8))
-        addTool(R.drawable.ic_lucide_delete, "Delete", large = true, mutates = true) { backspace() }
+        addTool(R.drawable.ic_lucide_delete, "Delete", large = true, mutates = true, logTelemetry = false) {
+            val ic = inputConnection()
+            // Telemetry symmetry (review NEW-2): every other tool emits
+            // before acting — a dead ⌫ from a missing connection is exactly
+            // the symptom class worth seeing in the data.
+            val state = if (ic == null) "no_connection"
+                else performImeBackspace(ic).telemetryState
+            UiTelemetry.action("ime_toolbar", actionIdFor(tooltip = "Delete"), state)
+        }
         addSpacer(dp(8))
         addTool(R.drawable.ic_lucide_text_select, "Select all", mutates = false) { performAction(android.R.id.selectAll) }
         addTool(R.drawable.ic_lucide_copy, "Copy", mutates = false) { performAction(android.R.id.copy) }
@@ -88,12 +96,15 @@ class EditingToolbar(
     }
 
     private fun addTool(iconRes: Int, tooltip: String,
-                        large: Boolean = false, mutates: Boolean = true, action: () -> Unit) {
+                        large: Boolean = false, mutates: Boolean = true,
+                        logTelemetry: Boolean = true, action: () -> Unit) {
         addView(toolButton(iconRes, tooltip, large).apply {
             setOnClickListener {
-                // RFC 0001 amendment: which IME editing tools are actually
-                // used (Heide's SwiftKey round-trips suggested these matter).
-                UiTelemetry.action("ime_toolbar", actionIdFor(tooltip))
+                if (logTelemetry) {
+                    // RFC 0001 amendment: which IME editing tools are actually
+                    // used (Heide's SwiftKey round-trips suggested these matter).
+                    UiTelemetry.action("ime_toolbar", actionIdFor(tooltip))
+                }
                 action()
                 if (mutates) onBufferChanged()
             }
@@ -118,22 +129,6 @@ class EditingToolbar(
 
     // ── Actions ────────────────────────────────────────────────────────────
 
-    /**
-     * Delete one codepoint before the cursor (#50: Heide's #1 request).
-     * [InputConnection.deleteSurroundingText] expects UTF-16 code-unit
-     * counts; a surrogate pair (emoji, CJK beyond BMP) is 2 units.
-     */
-    private fun backspace() {
-        val ic = inputConnection() ?: return
-        val before = ic.getTextBeforeCursor(2, 0)
-        val units = when {
-            before == null || before.isEmpty() -> return
-            before.length >= 2 && Character.isHighSurrogate(before[before.length - 2]) -> 2
-            else -> 1
-        }
-        ic.deleteSurroundingText(units, 0)
-    }
-
     /** Move the cursor one position left/right via the target's key handler (#50). */
     private fun moveCursor(direction: Int) {
         val ic = inputConnection() ?: return
@@ -147,3 +142,59 @@ class EditingToolbar(
         inputConnection()?.performContextMenuAction(actionId)
     }
 }
+
+/**
+ * The toolbar ⌫, extracted so its contract is unit-testable with a fake
+ * [InputConnection] (the EditorSync.kt / ImeOutputCoalescer.kt pattern).
+ *
+ * Two cases (Heide, 2026-10-29: "if you select something, the delete button
+ * doesn't do anything — it only works when you don't have anything
+ * selected"):
+ * - SELECTION: [InputConnection.deleteSurroundingText] cannot affect a
+ *   selection (it deletes BEFORE the selection start — a precise no-op when
+ *   the selection starts at the field beginning, i.e. Select All).
+ *   commitText("") replaces the selection with nothing. Review loop 1 (I1):
+ *   the editors documented hostile in bug #5 return TRUE while doing
+ *   nothing, so a true commit is re-verified — if the selection survived,
+ *   the DEL key-event fallback fires anyway (at worst a lying editor that
+ *   DID delete loses extra text — the lesser evil vs. a dead ⌫).
+ * - CARET: one codepoint back — deleteSurroundingText with UTF-16 unit
+ *   counts (a surrogate pair, e.g. any emoji, is 2).
+ */
+sealed interface ImeBackspaceOutcome {
+    /**
+     * The ui_action state string for this outcome (review NEW-3: the
+     * mapping lives with the type, testable, not duplicated at call sites).
+     */
+    abstract val telemetryState: String
+
+    /** The selection (if any) was deleted. */
+    object SelectionDeleted : ImeBackspaceOutcome { override val telemetryState = "selection" }
+    /** One codepoint before the caret was deleted. */
+    object CaretDeleted : ImeBackspaceOutcome { override val telemetryState = "char" }
+    /** Nothing to delete (empty selection + empty before-cursor). */
+    object Noop : ImeBackspaceOutcome { override val telemetryState = "none" }
+}
+
+fun performImeBackspace(ic: InputConnection): ImeBackspaceOutcome {
+    val selected = ic.getSelectedText(0)
+    if (selected != null && selected.isNotEmpty()) {
+        val committed = ic.commitText("", 1)
+        // Bug #5's lesson: a `true` return does not mean it happened. An
+        // empty (non-null) surviving selection is success — no fallback.
+        val survived = ic.getSelectedText(0)
+        if (!committed || (survived != null && survived.isNotEmpty())) {
+            ic.sendKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL))
+            ic.sendKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL))
+        }
+        return ImeBackspaceOutcome.SelectionDeleted
+    }
+    val before = ic.getTextBeforeCursor(2, 0)
+    val units = when {
+        before == null || before.isEmpty() -> return ImeBackspaceOutcome.Noop
+        before.length >= 2 && Character.isHighSurrogate(before[before.length - 2]) -> 2
+        else -> 1
+    }
+    ic.deleteSurroundingText(units, 0)
+    return ImeBackspaceOutcome.CaretDeleted
+}

app/src/test/java/at/dasher/android/ImeBackspaceTest.kt

@@ -0,0 +1,143 @@
+package at.dasher.android
+
+import android.view.KeyEvent
+import android.view.inputmethod.InputConnection
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * Heide's 2026-10-29 report: "if you select something, whether it's
+ * everything or partial, and you use the delete button on the toolbar, it
+ * doesn't do anything. It only works when you don't have anything selected."
+ *
+ * performImeBackspace must delete the SELECTION as a unit (empty commitText —
+ * deleteSurroundingText ignores selections) and keep the caret backspace
+ * behaviour (codepoint-correct, surrogate-aware) when nothing is selected.
+ */
+class ImeBackspaceTest {
+
+    /** Records calls; controlled getSelectedText/getTextBeforeCursor. Pure JVM. */
+    private class FakeIc(
+        private val selected: CharSequence? = null,
+        private val beforeCursor: CharSequence? = null,
+        private val commitAccepts: Boolean = true,
+        /** Bug-#5 editor class: commitText returns true AND the selection
+         *  survives (the async-editor lie the review loop flagged). */
+        private val selectionSurvivesCommit: Boolean = false,
+    ) : InputConnection {
+        val calls = mutableListOf<String>()
+        private var committed = false
+        override fun getSelectedText(flags: Int): CharSequence? =
+            if (committed) { if (selectionSurvivesCommit) selected else null } else selected
+        // Real windowing semantics (review M2): the last n UTF-16 units
+        override fun getTextBeforeCursor(n: Int, flags: Int): CharSequence? = beforeCursor?.takeLast(n)
+        override fun commitText(text: CharSequence, newCursorPosition: Int): Boolean {
+            calls += "commitText(${text.length})"
+            committed = true
+            return commitAccepts
+        }
+
+        override fun deleteSurroundingText(beforeLength: Int, afterLength: Int): Boolean {
+            calls += "deleteSurroundingText($beforeLength)"
+            return true
+        }
+        override fun sendKeyEvent(event: KeyEvent): Boolean {
+            // KeyEvent.getAction is not mocked on the JVM — record without
+            // inspecting (performImeBackspace sends DOWN+UP; assert on count).
+            calls += "sendKeyEvent"
+            return true
+        }
+        // Unused in these paths
+        override fun getTextAfterCursor(n: Int, flags: Int): CharSequence? = null
+        override fun getCursorCapsMode(reqModes: Int): Int = 0
+        override fun setComposingText(text: CharSequence, newCursorPosition: Int): Boolean = false
+        override fun setComposingRegion(start: Int, end: Int): Boolean = false
+        override fun finishComposingText(): Boolean = true
+        override fun setSelection(start: Int, end: Int): Boolean = false
+        override fun performEditorAction(actionCode: Int): Boolean = false
+        override fun performContextMenuAction(id: Int): Boolean = false
+        override fun beginBatchEdit(): Boolean = true
+        override fun endBatchEdit(): Boolean = true
+        override fun clearMetaKeyStates(states: Int): Boolean = true
+        override fun reportFullscreenMode(enabled: Boolean): Boolean = true
+        override fun performPrivateCommand(action: String?, data: android.os.Bundle?): Boolean = false
+        override fun getExtractedText(request: android.view.inputmethod.ExtractedTextRequest?, flags: Int): android.view.inputmethod.ExtractedText? = null
+        override fun deleteSurroundingTextInCodePoints(beforeLength: Int, afterLength: Int): Boolean = false
+        override fun getHandler(): android.os.Handler? = null
+        override fun closeConnection() {}
+        override fun commitContent(inputContent: android.view.inputmethod.InputContentInfo, flags: Int, opts: android.os.Bundle?): Boolean = false
+        override fun commitCorrection(correctionInfo: android.view.inputmethod.CorrectionInfo?): Boolean = false
+        override fun commitCompletion(text: android.view.inputmethod.CompletionInfo?): Boolean = false
+        override fun requestCursorUpdates(cursorUpdateMode: Int): Boolean = false
+    }
+
+    @Test
+    fun selection_is_deleted_with_an_empty_commit() {
+        val ic = FakeIc(selected = "everything or partial")
+        val outcome = performImeBackspace(ic)
+        assertEquals(ImeBackspaceOutcome.SelectionDeleted, outcome)
+        assertEquals(listOf("commitText(0)"), ic.calls)
+    }
+
+    @Test
+    fun selection_refusing_editor_gets_del_keyevent_fallback() {
+        val ic = FakeIc(selected = "sel", commitAccepts = false)
+        assertEquals(ImeBackspaceOutcome.SelectionDeleted, performImeBackspace(ic))
+        assertEquals(listOf("commitText(0)", "sendKeyEvent", "sendKeyEvent"), ic.calls)
+    }
+
+    @Test
+    fun lying_editor_true_commit_but_selection_survives_gets_del_fallback() {
+        // Review loop 1 (I1): the bug-#5 editor class returns TRUE while
+        // doing nothing. The re-verify must catch it and fire the fallback.
+        val ic = FakeIc(selected = "sel", commitAccepts = true, selectionSurvivesCommit = true)
+        assertEquals(ImeBackspaceOutcome.SelectionDeleted, performImeBackspace(ic))
+        assertEquals(listOf("commitText(0)", "sendKeyEvent", "sendKeyEvent"), ic.calls)
+    }
+
+    @Test
+    fun empty_surviving_selection_is_success_no_fallback() {
+        // Post-commit re-verify: a non-null but EMPTY selection means the
+        // delete landed — no DEL fallback may fire.
+        val ic = FakeIc(selected = "sel", selectionSurvivesCommit = false)
+        assertEquals(ImeBackspaceOutcome.SelectionDeleted, performImeBackspace(ic))
+        assertEquals(listOf("commitText(0)"), ic.calls)
+    }
+
+    @Test
+    fun empty_selection_falls_through_to_caret_backspace() {
+        // Some editors return "" rather than null with no selection
+        val ic = FakeIc(selected = "", beforeCursor = "ab")
+        performImeBackspace(ic)
+        assertEquals(listOf("deleteSurroundingText(1)"), ic.calls)
+    }
+
+    @Test
+    fun no_selection_deletes_one_unit() {
+        val ic = FakeIc(selected = null, beforeCursor = "ab")
+        performImeBackspace(ic)
+        assertEquals(listOf("deleteSurroundingText(1)"), ic.calls)
+    }
+
+    @Test
+    fun surrogate_pair_before_caret_deletes_two_units() {
+        val ic = FakeIc(selected = null, beforeCursor = "a😀")
+        performImeBackspace(ic)
+        assertEquals(listOf("deleteSurroundingText(2)"), ic.calls)
+    }
+
+    @Test
+    fun telemetry_state_mapping_is_pinned() {
+        assertEquals("selection", ImeBackspaceOutcome.SelectionDeleted.telemetryState)
+        assertEquals("char", ImeBackspaceOutcome.CaretDeleted.telemetryState)
+        assertEquals("none", ImeBackspaceOutcome.Noop.telemetryState)
+    }
+
+    @Test
+    fun empty_field_is_a_noop() {
+        val ic = FakeIc(selected = null, beforeCursor = "")
+        performImeBackspace(ic)
+        assertTrue(ic.calls.isEmpty())
+    }
+}