Repository navigation
fix(ime): atomic per-frame output + force-show on hw-keyboard apps (bug #5) - #71
Merged
Merged
Conversation
#5) User report #5: "When using Dasher in docked mode, the typed letters get completely jumbled up in applications like OneNote and Word, though it works perfectly fine inside AquaMail" and "Dasher wouldn't pop up on Chrome… on the MuMu emulator, I had to switch to Firefox." Part 1 — jumbled text in OneNote/Word. The IME applied every engine delta as its own binder call (one commitText per node entry, one deleteSurroundingText per deletion). Editors with synchronous InputConnections (AquaMail's EditText) apply those in order; OneNote and Word route InputConnection edits through their own async editing pipeline (undo coalescing, autosave) where a burst of unbatched deltas can land out of order or partially — garbled text. Same class of problem Dasher-Windows #17 fixed by forwarding deletions through the target's own input path. - ImeOutputCoalescer (pure, unit-tested): accumulates the frame's deltas into one Flush(deleteUnits, insert); a delete that spans the flushed/pending boundary is split exactly (UTF-16 units, surrogate pairs included). - flushOutputToTarget(): applied once per frame after the canvas submit, inside beginBatchEdit/endBatchEdit — the editor sees one atomic edit, not N unrelated calls. - Deletion fallback: if deleteSurroundingText returns false (WebView/custom editors), send KEYCODE_DEL key events through the target's own key pipeline instead of dropping the deletion (which silently desynced the engine buffer from the field — every later commit then landed wrong). - Buffer-clear (type 2) stays a no-op: committed text belongs to the field; the engine re-anchors separately. Part 2 — no popup on Chrome under a hardware keyboard. Emulators (MuMu) ship with a hardware keyboard attached; in that state Chrome never calls showSoftInput when a field gains focus (Firefox does). onEvaluateInputViewShown()=true only helps when someone asks. The IME now requestShowSelf()s on every fresh focus of a real text field (TYPE_NULL excluded) — consistent with the existing accessibility decision that our users always want the soft keyboard. Signed-off-by: will wade <willwade@gmail.com>
This was referenced Oct 15, 2026
…), flood cap, field-switch reset, contract docs Addresses the #71 review (score 7/10): - C1: the KEYCODE_DEL fallback sent one DEL per UTF-16 unit, but one DEL deletes one CHARACTER — a surrogate pair (any emoji; we ship an Emoji alphabet) would over-delete into the preceding character. New pure delKeyEventCount(units, beforeCursor) derives the count from the actual text before the caret (codepoints in the last N units); unit-tested. - C2: bulk control-mode deletes could flood the target with 2×N key events in one frame (jank/ANR). DEL fallback is now capped at MAX_DEL_KEY_EVENTS=64; beyond it the deletion is skipped with a log (desync is recoverable, an ANR is not). - I2: pending deltas from the previous field could land in the next focused field after a null-IC flush. onStartInput(restarting=false) now clears the coalescer — text must never appear in the wrong app. - I1: the class doc claimed deletes always remove the buffer tail — false for control-mode deletes (CAPI.cpp ctrlDelete removes AT the cursor, possibly mid-buffer/forwards; the event carries no position or direction). Documented as an approximation (same as pre-batching behavior); upstream filed as DasherCore#100. editDelete also fires the callback when its guard rejects the erase — DasherCore#101. - I3: tests for the risky paths — delKeyEventCount (surrogates, null, clamped), accumulating deletes, interleaved delete/commit/spanning- delete, clear() semantics. M3 (NewStringUTF mangling astral chars) split out as #72. - M1/M2/M4/M5: Hide-vs-force-show documented as deliberate; InputType import; unknown event types logged; type-2 safety invariant noted. Signed-off-by: will wade <willwade@gmail.com>
…overy, caret-relative docs, cap decision extracted Final polish from the #71 second review (8/10): - I-1: type-2 (buffer clear) now clears pending deltas — dasher.h says subscribers must 'clear your copy', and the pending state IS a shadow copy. The old comment claimed an 'always empty at reset' invariant the code itself violated (stranded null-connection flushes + seedBuffer's type-2 on every toolbar reanchor). Test added. - M-1: a skipped DEL fallback (no-text or over-cap) now re-anchors the engine from the field so the desync cannot compound on later deletes. - M-2: editDelete doc corrected — the real invariant is caret-relative ('N units before the caret' = deleteSurroundingText semantics); mid-buffer-cursor after ctrlMove joins ctrlDelete in the documented approximation bucket. - M-3: delKeyEventCount mid-surrogate-pair boundary pinned as decided desync behavior (orphaned low surrogate counts as one codepoint). - M-4: cap decision extracted to pure delFallbackPlan (Send/SkipNoText/ SkipOverCap) with boundary tests at the cap. - M-5: docs cite DasherCore#100/#101. Signed-off-by: will wade <willwade@gmail.com>
willwade
added a commit
that referenced
this pull request
Oct 15, 2026
…e restore, engine re-entry guard, unbind cleanup (#74) Fixes #73. Heide (testing v0.1.21, before v0.1.22): "The IME still starts in float mode, then you have to tap on dock. Then it's fine unless you switch keyboards." **Not a v0.1.22 regression** — v0.1.22's IME changes are output batching + force-show. This hardens #64's mode-remember machinery, which has three lifecycle holes found by inspection: 1. **`onCreateInputView` re-entry on a live instance** (config change; OEM IME-switch flows that keep the service alive): `createEngine()` built a *second* engine — the first kept its `dasher_frame` loop, both submitting into the same canvas — and a stale `floating=true` made the posted restore early-return inside `enterFloatingMode()`, presenting a dead overlay canvas as "floating" over a full-height docked root. Tap Dock → clean; switch keyboards → repeats. This matches the report verbatim. 2. **No `onUnbindInput` cleanup** — a surviving service left the overlay hovering over the next keyboard. 3. **`exitFloatingMode` early-returned on `floatingView == null`** — a swallowed `removeView` failure (Samsung overlay quirks, cf. #46) stranded a dead overlay over every subsequent show. ## Changes (single file, +51/−4) - `createEngine()`: re-entry guard — the engine is per-process by design (#53/#56) - Posted restore: **self-heal** — `if (floating) exitFloatingMode(persist = false)` *before* evaluating the saved preference, so every keyboard show converges to the persisted mode regardless of view recreation - `onUnbindInput()`: exit floating without persisting (saved mode stays the user's deliberate choice) - `exitFloatingMode(persist)`: never early-returns on a missing overlay window; `removeView` failures now log ## Testing - [x] `:app:assembleDebug` + `:app:testDebugUnitTest` green locally - [ ] Manual on Heide's device (Samsung/Android 16): dock → switch to SwiftKey → switch back → still docked, no dead overlay - [ ] Float → switch away → back: overlay gone during the other keyboard, float restored on return Given #71's human review is pending, happy to fold this into the same review session. Signed-off-by: will wade <willwade@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #70 (external user bug report #5).
Part 1 — jumbled text in OneNote/Word (fine in AquaMail). The IME applied every engine delta as its own binder call. Synchronous editors apply them in order; OneNote/Word route InputConnection edits through their own async editing pipeline where rapid unbatched deltas can land out of order or partially — garbled text (the Android cousin of Dasher-Windows #17's deletion-forwarding fix).
ImeOutputCoalescer(pure, unit-tested, EditorSync.kt pattern): accumulates each frame's deltas into oneFlush(deleteUnits, insert); a delete spanning the flushed/pending boundary is split exactly (UTF-16 units, surrogate pairs included)flushOutputToTarget()applies it once per frame insidebeginBatchEdit/endBatchEdit— the editor sees one atomic edit instead of N unrelated callsdeleteSurroundingTextreturning false (WebView/custom editors) now falls back toKEYCODE_DELkey events through the target's own key pipeline, instead of dropping the deletion and silently desyncing the engine buffer from the fieldPart 2 — "Dasher wouldn't pop up on Chrome… on the MuMu emulator." Emulators default to an attached hardware keyboard; Chrome never calls
showSoftInputin that state (Firefox does).onEvaluateInputViewShown()=trueonly helps when someone asks. The IME nowrequestShowSelf()s on every fresh focus of a real text field (TYPE_NULLexcluded) — consistent with the existing accessibility decision that our users always want the soft keyboard.Testing
ImeOutputCoalescerTest— 10 new unit tests: commit merging, boundary-spanning deletes, delete-then-commit ordering, surrogate pairs, buffer-clear no-op, flush reset semantics:app:assembleDebug+:app:testDebugUnitTestgreen locally (against the v0.2.26 submodule pin)Definition of Done