diff --git a/analytics-events.json b/analytics-events.json index 4b71d53..aa25042 100644 --- a/analytics-events.json +++ b/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 }, diff --git a/app/src/main/java/at/dasher/android/EditingToolbar.kt b/app/src/main/java/at/dasher/android/EditingToolbar.kt index 8d8a623..c8cb9a6 100644 --- a/app/src/main/java/at/dasher/android/EditingToolbar.kt +++ b/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 +} diff --git a/app/src/test/java/at/dasher/android/ImeBackspaceTest.kt b/app/src/test/java/at/dasher/android/ImeBackspaceTest.kt new file mode 100644 index 0000000..02666b3 --- /dev/null +++ b/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() + 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()) + } +}