fix(ime): atomic per-frame output + force-show on hw-keyboard apps (bug #5)

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

@@ -11,9 +11,11 @@ import android.inputmethodservice.InputMethodService
 import android.net.Uri
 import android.os.Handler
 import android.os.Looper
+import android.text.InputType
 import android.util.Log
 import android.util.TypedValue
 import android.view.Gravity
+import android.view.KeyEvent
 import android.view.MotionEvent
 import android.view.View
 import android.view.ViewGroup
@@ -34,6 +36,12 @@ import at.dasher.android.ui.DasherCanvasView
 class DasherImeService : InputMethodService() {
 
     private var engine: DasherEngine? = null
+
+    // Bug #5 ("letters get completely jumbled up in OneNote and Word"):
+    // engine deltas coalesce here and flush ONCE per frame as one atomic
+    // batch edit — see ImeOutputCoalescer.
+    private val outputCoalescer = ImeOutputCoalescer()
+
     private var canvasView: DasherCanvasView? = null
     private var canvasHost: android.widget.FrameLayout? = null
     private var dockedRoot: LinearLayout? = null
@@ -424,6 +432,10 @@ class DasherImeService : InputMethodService() {
         val userDir = DataInstaller.userDir(this).apply { mkdirs() }
         val eng = DasherEngine.create(dataDir, userDir.absolutePath) { commands, strings ->
             canvasView?.submitFrame(commands, strings)
+            // All of this frame's engine deltas have now fired — apply them
+            // to the target field as ONE atomic edit (bug #5: unbatched
+            // deltas garble text in OneNote/Word's async editors).
+            flushOutputToTarget()
         }
         if (eng == null) {
             Log.e(TAG, "IME engine creation failed (dataDir=$dataDir)")
@@ -544,12 +556,13 @@ class DasherImeService : InputMethodService() {
         // Per-engine-instance listeners (#56): the JNI dispatch passes the
         // handle; only THIS engine's listener is called. No cross-talk with
         // the main app's engine.
+        // Output deltas are COALESCED, not applied — see flushOutputToTarget
+        // (bug #5: OneNote/Word jumbled unbatched per-delta edits).
         eng.setOutputListener { type, text ->
-            val ic = currentInputConnection
-            if (ic != null) {
-                if (type == 0) ic.commitText(text, 1)
-                else if (text.isNotEmpty()) ic.deleteSurroundingText(text.length, 0)
+            if (type != 0 && type != 1 && type != 2) {
+                Log.w(TAG, "Unknown engine output event type $type (text='${text.take(32)}')")
             }
+            outputCoalescer.onEngineOutput(type, text)
         }
         eng.setMessageListener { _, msg ->
             Toast.makeText(this, msg, Toast.LENGTH_SHORT).show()
@@ -561,6 +574,64 @@ class DasherImeService : InputMethodService() {
         eng.clearSpeakListener() // IME doesn't speak (the app does)
     }
 
+    /**
+     * Applies the coalesced frame output to the target field as one atomic
+     * batch edit (bug #5): first the deletion, then the insertion, inside a
+     * beginBatchEdit/endBatchEdit pair so the editor cannot reformat between
+     * the two halves.
+     *
+     * Deletion strategy: deleteSurroundingText first; if the editor reports
+     * it did not handle it (WebView/custom editors — precisely the
+     * OneNote/Word class), fall back to KEYCODE_DEL key events through the
+     * target's own key pipeline (the same principle as Dasher-Windows #17's
+     * deletion forwarding). DELs count CHARACTERS, not UTF-16 units
+     * (delKeyEventCount), and are capped (delFallbackPlan): a bulk
+     * control-mode delete must not flood the target with hundreds of key
+     * events (jank/ANR). A skipped deletion re-anchors the engine from the
+     * field (reanchorEngineToTarget) so the mismatch doesn't compound.
+     *
+     * No connection yet (field switch in flight): keep the deltas pending —
+     * the next frame flushes them once a field is served. (Stranded deltas
+     * are dropped on a fresh field start — onStartInput.)
+     */
+    private fun flushOutputToTarget() {
+        val ic = currentInputConnection ?: return
+        val flush = outputCoalescer.takeFlush() ?: return
+        var skippedDeletion = false
+        ic.beginBatchEdit()
+        try {
+            if (flush.deleteUnits > 0) {
+                if (!ic.deleteSurroundingText(flush.deleteUnits, 0)) {
+                    val dels = delKeyEventCount(
+                        flush.deleteUnits,
+                        ic.getTextBeforeCursor(flush.deleteUnits, 0)?.toString()
+                    )
+                    when (val plan = delFallbackPlan(dels, MAX_DEL_KEY_EVENTS)) {
+                        is DelFallback.Send ->
+                            repeat(plan.keyEvents) {
+                                ic.sendKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL))
+                                ic.sendKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL))
+                            }
+                        is DelFallback.SkipOverCap ->
+                            Log.w(TAG, "DEL fallback skipped: $plan — bulk delete over the " +
+                                "${MAX_DEL_KEY_EVENTS}-event flood cap")
+                        DelFallback.SkipNoText ->
+                            Log.w(TAG, "DEL fallback skipped: editor returned no text before caret")
+                    }
+                    if (dels == null || dels > MAX_DEL_KEY_EVENTS) skippedDeletion = true
+                }
+            }
+            if (flush.insert.isNotEmpty()) ic.commitText(flush.insert, 1)
+        } finally {
+            ic.endBatchEdit()
+        }
+        // Review M-1: a skipped deletion leaves engine and field desynced —
+        // re-anchor from the field so the mismatch doesn't compound on every
+        // later delete (seedBuffer re-reads the field and fires the type-2
+        // clear, dropping any stale pending state).
+        if (skippedDeletion) reanchorEngineToTarget()
+    }
+
     // API 36+ (Android 16): measure the actual docked-root position in the
     // window so the insets animation has a concrete target (review C1/C3:
     // imeHeightPx() is a HEIGHT, not an offset — a constant created a dead
@@ -581,6 +652,38 @@ class DasherImeService : InputMethodService() {
         outInsets.touchableInsets = Insets.TOUCHABLE_INSETS_CONTENT
     }
 
+    /**
+     * Bug #5b ("Dasher wouldn't pop up on Chrome… on the MuMu emulator, I had
+     * to switch to Firefox"): while a hardware keyboard is attached (emulators
+     * default to one), Chrome never asks the system to show the IME when a
+     * field gains focus — Firefox asks regardless, which is why it worked
+     * there. onEvaluateInputViewShown()=true only helps when something asks;
+     * nobody asked. We serve motor-impaired AAC users who always want the
+     * soft keyboard, so ask ourselves on every fresh focus of a real text
+     * field (TYPE_NULL fields are non-editing views and stay untouched).
+     *
+     * Deliberate consequence (review M1): the Hide button dismisses the
+     * keyboard only until the next field focus — on hardware-keyboard devices
+     * nothing else would ever bring it back, and "keyboard follows focus" is
+     * the accessibility stance this IME already takes. Hide still works
+     * within a field (re-focuses of the same field arrive as restarting=true).
+     */
+    override fun onStartInput(info: EditorInfo?, restarting: Boolean) {
+        super.onStartInput(info, restarting)
+        if (!restarting) {
+            // Fresh target field: any deltas still pending from the previous
+            // field must NEVER land here (review I2 — for an AAC user, text
+            // appearing in the wrong app's field is a trust failure).
+            outputCoalescer.clear()
+            if (info != null &&
+                info.inputType != InputType.TYPE_NULL &&
+                !isInputViewShown
+            ) {
+                requestShowSelf(0)
+            }
+        }
+    }
+
     override fun onStartInputView(info: EditorInfo?, restarting: Boolean) {
         super.onStartInputView(info, restarting)
         // Re-assert ALL IME-owned static listeners — the main app may have
@@ -629,5 +732,13 @@ class DasherImeService : InputMethodService() {
         const val TAG = "DasherImeService"
         private val MATCH_PARENT = ViewGroup.LayoutParams.MATCH_PARENT
         private val WRAP_CONTENT = ViewGroup.LayoutParams.WRAP_CONTENT
+
+        /**
+         * Upper bound on KEYCODE_DEL fallback events per flush (review C2):
+         * each is a key event the target must process — a bulk control-mode
+         * delete beyond this cap is skipped with a log (engine/field desync
+         * is recoverable; an ANR or an editor dropping half the flood is not).
+         */
+        const val MAX_DEL_KEY_EVENTS = 64
     }
 }

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

@@ -0,0 +1,160 @@
+package at.dasher.android
+
+/**
+ * Coalesces engine output deltas into ONE atomic flush per frame (user bug
+ * report #5: "typed letters get completely jumbled up in applications like
+ * OneNote and Word, though it works perfectly fine inside AquaMail").
+ *
+ * Dasher emits text as a stream of small deltas — one commit per node entry,
+ * one delete per deletion. Applied delta-by-delta, each op is a separate
+ * binder call that the TARGET app applies on its own pipeline. Editors with
+ * synchronous InputConnections (AquaMail's EditText) apply them in order and
+ * everything works. OneNote and Word route edits through their own async
+ * editing engines (undo coalescing, autosave), where rapid unbatched deltas
+ * can land out of order or partially — the field ends up jumbled. (Android's
+ * version of the problem Dasher-Windows #17 fixed by forwarding deletions
+ * through the target's own input path.)
+ *
+ * The IME feeds every engine output event here as it fires during
+ * dasher_frame(), then flushes ONCE after the frame inside a
+ * beginBatchEdit/endBatchEdit pair: the editor sees a single atomic
+ * "delete N, insert S" step instead of a burst of unrelated calls.
+ *
+ * Delete-event contract (see DasherCore src/CAPI.cpp) and its limits:
+ * type-1 events come from two engine paths. `editDelete` is a
+ * caret-relative backspace — it erases N units immediately BEFORE
+ * cursorPos, which is exactly `deleteSurroundingText(N, 0)` semantics
+ * (mid-buffer only after a control-mode ctrlMove moved the cursor — see
+ * below). `ctrlDelete` (control-mode delete nodes, if the user enables
+ * them) removes a range AT the cursor — possibly spanning it, possibly
+ * forwards — but the event carries only the removed string, with no
+ * position or direction, so no frontend can express it faithfully. Both
+ * non-caret-relative cases are approximated as caret deletes — exactly
+ * what the pre-batching code did per-event, so no regression; the proper
+ * fix needs the event contract to grow position/direction (DasherCore
+ * #100; #101 covers editDelete firing the event even when its guard
+ * rejected the erase).
+ *
+ * Type-2 (buffer clear) DROPS pending deltas — dasher.h says subscribers
+ * "must treat this as 'clear your copy'", and the pending state is exactly a
+ * shadow copy of the engine buffer. This matters because the flush can strand
+ * deltas when the connection is momentarily null (WebView focus churn); a
+ * later reset (dasher_seed_buffer fires type-2 on every toolbar reanchor —
+ * CAPI_edit.cpp) must not let those stale deltas apply to post-reset context.
+ *
+ * Pure logic — no InputConnection, so the delta arithmetic (including the
+ * surrogate-pair and boundary-spanning cases) is unit-testable, following
+ * the EditorSync.kt pattern.
+ */
+class ImeOutputCoalescer {
+
+    /**
+     * One atomic edit: delete [deleteUnits] UTF-16 code units before the
+     * caret, then insert [insert]. Either half may be empty.
+     */
+    data class Flush(val deleteUnits: Int, val insert: String)
+
+    private var pendingDeleteUnits = 0
+    private val pendingInsert = StringBuilder()
+
+    /**
+     * Engine output event (dasher_output_callback semantics from dasher.h):
+     * 0 = commit `text`; 1 = delete `text` (see the class doc for the
+     * tail-delete assumption and its control-mode limits); 2 = buffer clear
+     * (dasher_reset / alphabet change — no delta can express it; the IME
+     * re-anchors the engine separately, and committed text already belongs
+     * to the target field).
+     */
+    fun onEngineOutput(type: Int, text: String) {
+        when (type) {
+            0 -> pendingInsert.append(text)
+            1 -> {
+                if (text.isEmpty()) return
+                // A tail delete removes the engine buffer's tail. That tail
+                // is [text already flushed to the field][pending insert].
+                // Split the deleted length across that boundary:
+                if (text.length <= pendingInsert.length) {
+                    // Entirely inside this frame's pending insert — those
+                    // chars never reached the field; just cancel them.
+                    pendingInsert.setLength(pendingInsert.length - text.length)
+                } else {
+                    // Spans past the pending insert into text the field
+                    // already holds — cancel all of pending, and delete only
+                    // the remainder from the field.
+                    pendingDeleteUnits += text.length - pendingInsert.length
+                    pendingInsert.setLength(0)
+                }
+            }
+            else -> clear() // type 2 (buffer clear) — drop the shadow copy; see class doc
+        }
+    }
+
+    /**
+     * The pending edit, or null when nothing accumulated. Resets the state —
+     * callers own the returned edit. (UTF-16 units throughout:
+     * deleteSurroundingText and String.length agree; a surrogate pair is 2.)
+     */
+    fun takeFlush(): Flush? {
+        if (pendingDeleteUnits == 0 && pendingInsert.isEmpty()) return null
+        val flush = Flush(pendingDeleteUnits, pendingInsert.toString())
+        pendingDeleteUnits = 0
+        pendingInsert.setLength(0)
+        return flush
+    }
+
+    /** Drop any pending deltas (e.g. on a field switch — see onStartInput). */
+    fun clear() {
+        pendingDeleteUnits = 0
+        pendingInsert.setLength(0)
+    }
+}
+
+/**
+ * How many KEYCODE_DEL events delete the same text as
+ * `deleteSurroundingText(units, 0)` — the fallback path for editors that
+ * refuse delta deletions (bug #5 review: OneNote/Word class).
+ *
+ * One DEL deletes one CHARACTER (a codepoint), not one UTF-16 unit: a
+ * surrogate pair (e.g. any emoji — this IME ships an Emoji alphabet) is two
+ * units but ONE DEL. Sending `units` DELs would over-delete into the
+ * preceding character, so the count is derived from the actual text before
+ * the caret.
+ *
+ * Boundary note (decided behavior, review M-3): if the range start lands
+ * INSIDE a surrogate pair (only possible when engine and field are already
+ * desynced — this is a fallback of a fallback), the orphaned low surrogate
+ * counts as one codepoint. Pinned by test.
+ *
+ * @param units UTF-16 units the engine wants deleted
+ * @param beforeCursor the field text actually before the caret, at least
+ *   `units` long from a cooperative editor — shorter text is clamped (the
+ *   editor truncated the read; deleting what it reported is the best move)
+ * @return the DEL count, or null when no safe count exists (editor returned
+ *   no text — connection effectively dead)
+ */
+fun delKeyEventCount(units: Int, beforeCursor: String?): Int? {
+    if (units <= 0) return 0
+    if (beforeCursor == null) return null
+    val from = (beforeCursor.length - units).coerceAtLeast(0)
+    return Character.codePointCount(beforeCursor, from, beforeCursor.length)
+}
+
+/**
+ * The DEL-fallback decision for one flush (review M-4 — extracted so the
+ * cap boundary is unit-testable, not buried in the service).
+ * [DelFallback.Send] means `keyEvents` DOWN/UP pairs (i.e. 2×keyEvents
+ * `sendKeyEvent` calls) inside the batch edit.
+ */
+sealed interface DelFallback {
+    data class Send(val keyEvents: Int) : DelFallback
+    /** Editor returned no text before the caret — no safe count exists. */
+    object SkipNoText : DelFallback
+    /** Bulk delete over the flood cap — skipped to avoid jank/ANR. */
+    data class SkipOverCap(val requested: Int, val cap: Int) : DelFallback
+}
+
+fun delFallbackPlan(dels: Int?, cap: Int): DelFallback = when {
+    dels == null -> DelFallback.SkipNoText
+    dels > cap -> DelFallback.SkipOverCap(dels, cap)
+    else -> DelFallback.Send(dels)
+}