diff --git a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java index af91356e7..975f571a8 100644 --- a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java +++ b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java @@ -745,6 +745,15 @@ public void onStartBatchInput(final SettingsValues settingsValues, // Combining mode: snapshot before cancel; the gesture will re-arm the timer on completion. final boolean wasInCombiningMode = mInCombiningMode; cancelCombiningMode(); + // Two-thumb typing: defensively drop any stale merged-trail extend-base before this + // gesture's extend decision. The base is meant to live for exactly one gesture (set in + // the extend branch below, cleared in onUpdateTailBatchInputCompleted), but an abnormal + // prior gesture end — a cancel, or an empty-top recognition that early-returns before + // that clear — can leave it armed. A fresh gesture would then merge with the ghost + // trail; most visibly at the start of a text box, where no composing word exists to + // re-set it. Clearing here guarantees every gesture begins from a clean base; the + // extend branch re-arms it from the real composing word when appropriate. + mWordComposer.setExtendBatchInputBase(null); mConnection.beginBatchEdit(); // Two-thumb typing (#1.1 + combining-mode): two ways the gesture can EXTEND an existing // composing word instead of replacing it: @@ -895,6 +904,10 @@ public void onCancelBatchInput(final LatinIME.UIHandler handler) { // Drop any seed codepoint stashed by PointerTracker so the next gesture doesn't // strip its first letter against a stale seed. helium314.keyboard.keyboard.PointerTracker.consumeGestureSeedCodepoint(); + // Two-thumb typing: a cancelled gesture never reaches onUpdateTailBatchInputCompleted, + // which is the only routine site that clears the merged-trail extend-base. Drop it here + // so this cancelled gesture's trail can't leak into the next one. + mWordComposer.setExtendBatchInputBase(null); // Tap-promotion-extend was a per-gesture decision; clear on cancel so the NEXT // gesture re-evaluates against current timing. mGestureExtendsByTapPromotion = false; @@ -2360,6 +2373,12 @@ private void handleBackspaceEvent(final Event event, final InputTransaction inpu // stroke. The whole-word and batch delete branches below call mWordComposer.reset() // directly rather than resetComposingState(), so clear here to cover every path. mLiveStroke.reset(); + // Two-thumb typing: a backspace invalidates the merged-trail extend-base for the same + // reason it invalidates mLiveStroke above — re-doing the word must start from a clean + // stroke, not merge with the geometry we just partially deleted. This single clear + // covers all three backspace modes (character / fragment / whole-word), since it runs + // before any of their branches. + mWordComposer.setExtendBatchInputBase(null); // Typing-insight overlay: a backspace edits/clears the gesture word, so its trail is now // stale. Drop it so it doesn't linger. final MainKeyboardView backspaceKv = KeyboardSwitcher.getInstance().getMainKeyboardView(); @@ -3547,6 +3566,9 @@ private void resetComposingState(final boolean alsoResetLastComposedWord) { mCombiningWordHasGestureFragment = false; // Live-converge (#1.7): the word is gone, so its accumulated stroke is stale. mLiveStroke.reset(); + // Two-thumb typing: likewise drop the merged-trail extend-base. This path also covers + // the delete slider (finishInput -> resetComposingState) and cursor-move resets. + mWordComposer.setExtendBatchInputBase(null); // Combining mode is keyed on a composing word existing; if we're wiping it, the // pending timer would commit nothing useful, so cancel. cancelCombiningMode(); @@ -4177,6 +4199,8 @@ private void commitChosenWord(final SettingsValues settingsValues, final String mCombiningWordHasGestureFragment = false; // Live-converge (#1.7): word committed — its accumulated stroke no longer applies. mLiveStroke.reset(); + // Two-thumb typing: the merged-trail extend-base belongs to the just-committed word too. + mWordComposer.setExtendBatchInputBase(null); if (DebugFlags.DEBUG_ENABLED) { long runTimeMillis = System.currentTimeMillis() - startTimeMillis; Log.d(TAG, "commitChosenWord() : " + runTimeMillis + " ms to run " @@ -4339,6 +4363,8 @@ private void setComposingTextInternalWithBackgroundColor(final CharSequence newC // Live-converge (#1.7): composing was force-cancelled due to a desync — the stored // stroke no longer matches anything on screen, so drop it. mLiveStroke.reset(); + // Two-thumb typing: the merged-trail extend-base is equally stale after a desync. + mWordComposer.setExtendBatchInputBase(null); } } diff --git a/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt b/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt index 3ef7bb4fc..b73ab10fe 100644 --- a/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt +++ b/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt @@ -39,9 +39,12 @@ import org.robolectric.shadows.ShadowLog import java.util.* import kotlin.math.min import kotlin.streams.asSequence +import helium314.keyboard.latin.common.InputPointers import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue @RunWith(RobolectricTestRunner::class) @Config(shadows = [ @@ -607,6 +610,107 @@ class InputLogicTest { assertEquals("", composingText) } + // --- Two-thumb typing: the merged-trail extend-base (WordComposer.mExtendBatchInputBase) + // must never outlive the word it belonged to. Before the fix it was only cleared on a + // normally-completing gesture, so an abnormal end (cancel / empty-top recognition) left it + // armed, and NO deletion path cleared it. A later fresh swipe — most visibly at the start of + // a text box — then merged with that ghost trail. Each test below arms the base, performs one + // user action, and asserts the base is dropped. (We arm the base directly because the JVM + // harness has no native recognizer to produce a real merged trail.) Each fails pre-fix. --- + + @Test fun extendBaseClearedByCharacterBackspace() { + reset() + latinIME.prefs().edit { + putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) + // Character mode = neither fragment-pop nor whole-word delete. Both default ON + // (Defaults.kt), so they must be explicitly disabled to exercise the per-char path. + putBoolean(Settings.PREF_GESTURE_FRAGMENT_BACKSPACE, false) + putBoolean(Settings.PREF_COMBINING_BACKSPACE_DELETES_GESTURE_WORD, false) + } + chainInput("hello") + armExtendBase() + functionalKeyPress(KeyCode.DELETE) // character delete; word stays composing as "hell" + assertEquals("hell", composingText) + assertFalse(composer.isExtendBatchInputBaseSet) + } + + @Test fun extendBaseClearedByFragmentBackspace() { + reset() + latinIME.prefs().edit { + putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) + putBoolean(Settings.PREF_GESTURE_FRAGMENT_BACKSPACE, true) + putBoolean(Settings.PREF_COMBINING_BACKSPACE_DELETES_GESTURE_WORD, false) + } + chainInput("hello") // each tap records a fragment boundary under manual spacing + armExtendBase() + functionalKeyPress(KeyCode.DELETE) // fragment pop + assertFalse(composer.isExtendBatchInputBaseSet) + } + + @Test fun extendBaseClearedByWholeWordBackspace() { + reset() + latinIME.prefs().edit { + putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) + putBoolean(Settings.PREF_COMBINING_BACKSPACE_DELETES_GESTURE_WORD, true) + putBoolean(Settings.PREF_COMBINING_BACKSPACE_DELETES_COMPOSING_TEXT, true) + } + chainInput("hello") + armExtendBase() + functionalKeyPress(KeyCode.DELETE) // whole-word delete of the composing word + assertEquals("", composingText) + assertFalse(composer.isExtendBatchInputBaseSet) + } + + @Test fun extendBaseClearedByDeleteSlider() { + reset() + latinIME.prefs().edit { putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) } + chainInput("hello") + armExtendBase() + // The delete slider (swipe-from-backspace) routes through inputLogic.finishInput() in + // KeyboardActionListenerImpl.onMoveDeletePointer / onUpWithDeletePointerActive. + inputLogic.finishInput() + assertFalse(composer.isExtendBatchInputBaseSet) + } + + @Test fun extendBaseClearedByFreshGestureStart() { + reset() + latinIME.prefs().edit { putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) } + // No composing word: the next gesture is fresh and must not inherit a leaked base. + armExtendBase() + inputLogic.onStartBatchInput(settingsValues, KeyboardSwitcher.getInstance(), latinIME.mHandler) + handleMessages() + assertFalse(composer.isExtendBatchInputBaseSet) + } + + @Test fun extendBaseClearedByGestureCancel() { + reset() + latinIME.prefs().edit { putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) } + armExtendBase() + inputLogic.onCancelBatchInput(latinIME.mHandler) + handleMessages() + assertFalse(composer.isExtendBatchInputBaseSet) + } + + // Static-seed reachability guard. PointerTracker's tap-seed path (sLastLetterTap*) is gated + // on (!isMultipartComposeActive() && mCombiningGraceMs > 0). But grace > 0 forces multi-part + // composition active, so that conjunction is unsatisfiable and the seed is currently + // unreachable dead code. These pin the interlock: if a future settings refactor decouples + // them and re-arms the seed, it must first add the stale-static cleanup (the seed statics are + // process-global and never reset on delete / commit / field switch). + @Test fun graceImpliesMultipartComposeActive_keepsSeedPathDead() { + reset() + latinIME.prefs().edit { putInt(Settings.PREF_COMBINING_GRACE_MS, 1000) } + setText("") // force a settings reload + assertTrue(settingsValues.isMultipartComposeActive) + } + + @Test fun manualSpacingImpliesMultipartComposeActive() { + reset() + latinIME.prefs().edit { putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) } + setText("") + assertTrue(settingsValues.isMultipartComposeActive) + } + @Test fun forceAutoCapWorksWhenAutoCapIsOff() { reset() latinIME.prefs().edit { @@ -1309,6 +1413,17 @@ class InputLogicTest { checkConnectionConsistency() } + // Arm the merged-trail extend-base with a non-empty trail, simulating the state left by a + // prior gesture fragment. (A single empty InputPointers is treated as "clear", so two real + // points are required for isExtendBatchInputBaseSet to become true.) + private fun armExtendBase() { + val base = InputPointers(8) + base.addPointer(10, 20, 0, 0) + base.addPointer(30, 40, 0, 0) + composer.setExtendBatchInputBase(base) + assertTrue(composer.isExtendBatchInputBaseSet) + } + private fun checkConnectionConsistency() { // RichInputConnection only has composing text up to cursor, but InputConnection has full composing text val expectedConnectionComposingText = if (composingStart == -1 || composingEnd == -1) "" diff --git a/dev-log.md b/dev-log.md index 35da9bbfa..5a976bcda 100644 --- a/dev-log.md +++ b/dev-log.md @@ -825,3 +825,92 @@ The whole-word branch for a *composing* word used `mConnection.deleteTextBeforeC - Whole-word delete through a sentence deletes word-by-word with NO mashing/desync. - Re-doing a word after deletion recognizes cleanly (no ever-growing `th…`). - Swipe-on-backspace bulk delete leaves no stale stroke for the next word. + +--- + +## 2026-06-06 — Fix stale merged-trail extend-base leaking into the next gesture + +### Context +Bug report: gestures occasionally get "stored" after a deletion and bleed into the +next swipe — most visibly when swiping a fresh word at the start of a text box, +which produced a word fused with previously-deleted gesture geometry. Branch: +`fix/extend-base-leak-on-delete` off `main`. + +### Root cause +There are two per-word gesture-geometry stores. `mLiveStroke` (live-converge, in +`InputLogic`) was already cleared on every word-end path. But its WordComposer-side +counterpart — `mExtendBatchInputBase` (the multi-part merged-trail base consumed by +`WordComposer.setBatchInputPointers`) — was only ever cleared by a *normally +completing* gesture (`onUpdateTailBatchInputCompleted`, the lone routine clear, +which sits AFTER two empty-recognition early-returns). An abnormal gesture end left +the `mExtendBatchInputBaseSet` flag armed: +- `onCancelBatchInput` never cleared it, +- the empty-top-word early-returns in `onUpdateTailBatchInputCompleted` skip the clear, +- `WordComposer.reset()` does not touch it, so NO deletion path cleared it either. + +Once leaked, the flag survived every backspace / selection-delete / commit. A later +fresh gesture (no composing word ⇒ the `onStartBatchInput` extend branch never runs, +so it neither set nor cleared the base) hit the merge guard in +`setBatchInputPointers` and prepended the ghost trail. Start-of-text-box made it +maximally visible because there is no composing word there to legitimately re-set +the base. + +### Actions Taken +- `latin/inputlogic/InputLogic.java`: clear the merged-trail base + (`mWordComposer.setExtendBatchInputBase(null)`) at the same word-end sites where + `mLiveStroke` is already dropped, plus the two gesture-lifecycle origins: + - `onStartBatchInput` (top, before the extend decision — guarantees every gesture + starts from a clean base; the extend branch re-arms it from the real composing + word when appropriate), + - `onCancelBatchInput` (a cancelled gesture never reaches the routine clear), + - `handleBackspaceEvent` (next to the existing `mLiveStroke.reset()`, before any + mode branch — covers all three backspace modes in one place), + - `resetComposingState` (covers the delete slider's `finishInput` and cursor-move + resets), + - `commitChosenWord` and the composing-desync recovery path. + Deliberately did NOT add the clear to the hot `WordComposer.reset()` to avoid any + interaction with the mid-gesture merge timing; mirroring the reviewed `mLiveStroke` + sites is equivalent and lower-risk. +- `app/src/test/.../InputLogicTest.kt`: 6 base-clear regression tests (one per + backspace mode — character / fragment / whole-word — plus delete slider, fresh + `onStartBatchInput`, and `onCancelBatchInput`), each arming the base then asserting + it is dropped (each fails pre-fix). Added `armExtendBase()` helper and `InputPointers` + / `assertTrue` / `assertFalse` imports. +- Also added two reachability-guard tests pinning the (currently dead) PointerTracker + static-seed interlock: `grace > 0 ⇒ isMultipartComposeActive()` and the + manual-spacing equivalent. If a future settings refactor decouples them and re-arms + the seed, these fail and force adding the missing stale-static cleanup first. + +### Decisions Made +- Threading was verified safe: every `mExtendBatchInputBase` mutation runs on the UI + thread (the recognizer runs on the suggestions HandlerThread but only reads a + snapshot), so the new clears need no locking and can't race the merge. +- Scoped to dropping the base. The *related* "re-extend after a partial (character / + fragment) delete still merges the un-trimmed `mInputPointers`" issue is left for a + later ticket per the established "drop, don't trim" philosophy — the fix neither + causes nor worsens it. + +### Testing status +- Built and ran on this machine (newly installed JDK 21 + Android SDK platform-35 / + build-tools 35.0.0). `:app:testStandardDebugUnitTest` filtered to the 8 new tests + + `WordComposerTest`: **BUILD SUCCESSFUL, 11/11 passing.** Full-suite still carries the + 3 pre-existing known failures (`insertLetterIntoWordHangulFails`, + `revert autocorrect on delete`, `tapOnlyCombiningWordDoesNotShowAutospaceIndicator…`), + unrelated to this change. +- On-device validation still recommended: reproduce the start-of-text-box ghost-merge + (swipe a word, trigger an abnormal gesture end, delete, swipe a fresh word) and + confirm it no longer fuses; re-check all three backspace modes + the delete slider. + +### Manual Tests — Extend-base leak +| # | Steps | Expected Result | +|---|---|---| +| 1 | Two-thumb on. Swipe a word; cancel/abort a follow-up gesture; delete everything; swipe a fresh word at the start of the box. | Fresh word recognizes alone — no fusion with the deleted gesture. | +| 2 | Repeat #1 with backspace mode = Delete one character. | Same; no ghost merge. | +| 3 | Repeat #1 with backspace mode = Delete last fragment. | Same; no ghost merge. | +| 4 | Repeat #1 with backspace mode = Delete whole word. | Same; no ghost merge. | +| 5 | Repeat #1 using swipe-from-backspace (delete slider) to clear. | Same; no ghost merge. | + +### Open Questions / Next Steps +- Build the standard debug APK and install for on-device validation. +- Decide whether to also tackle the re-extend-after-partial-delete (stale + `mInputPointers`) follow-up.