Repository navigation
fix(terminal): stop Android autocorrect duplicating the typed line - #541
Conversation
xterm diffs the helper textarea with newValue.replace(oldValue, ''), which only works when the keyboard appended. SwiftKey/Gboard autocorrect on space deletes a word and inserts the corrected one, so xterm sent the whole value and then the inserted text again (testing the peompt + space became 'testing the peompttesting the prompt rompt '), and a multi-character delete was one DEL. The keyCode-229 controller now swaps in an edit-based diff against what was already sent. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrzFKEdBLwVfu6ev2ZscJS
|
Verified on the device that logged the bug (SwiftKey in Edge for Android, local echo on): after deploying this, deliberate misspellings followed by a space no longer duplicate the line, and the corrected words reach the prompt once. |
|
Thank you @opticon454, this is a careful fix with an excellent device log behind it. The PR replaces xterm's append-only I ran your sequence in real chromium against xterm 6.0.0, with local echo both on and off. Autocorrect on space and a mid-line insert now reach the PTY exactly once, where master duplicates the line. The unit and browser tests are clear, and the control cases that prove the bug byte for byte are very welcome. One regression needs fixing before merge. A pending 229 edit plus Enter in the same task erases the prompt ( The edit-sync timer is scheduled at the 229 keydown. When the last character's keydown and its With local echo on, which is the default on phones, the Enter branch flushes the text immediately but delays the
Master is already wrong here by one character. The PR turns that into losing almost the whole prompt. With local echo off, the same DELs land after the The fix that worked for me is to settle the pending diff at the next keydown, the same way
The order matters. Settling first bumps With that change, both rows above submit the full line in both local-echo modes, and all existing unit and browser tests still pass. Please also add a browser test that sends a 229 last character + Enter in one task, with local echo on and off, and asserts the submitted line. If you can, try a deliberately misspelled last word followed by Enter on your SwiftKey phone, since autocorrect on Enter is exactly this shape. Smaller things (I can also do these at merge time)
Once the Enter case is settled and tested, this is ready to merge. Thanks again for chasing it down on a real device. |
… cannot erase the line (Ark0N#541 review) A pending 229 edit plus Enter in one page task: xterm clears the textarea for CR before the edit timer runs, so the timer diffed the whole line against '' and sent one DEL per character ahead of the submitted line. The pending diff is now applied synchronously from handleKeyEvent, before flushPending() (which keeps the orphan candidate from sending the character twice). Tests: unit (3) and browser (4, local echo on and off, plain last character and autocorrect), each verified to fail without the settle call. xterm-private-api guard now names the CompositionHelper fields this depends on and checks the shipped bundle; CLAUDE.md notes the edit-based 229 diff. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrzFKEdBLwVfu6ev2ZscJS
|
Thanks, that's a real regression and your recipe fixed it. Pushed (f21ab39):
I will also try a deliberately misspelled last word followed by Enter on the SwiftKey phone and report back. |
|
Test it manually on my phone and it's working fine :) |
A composition that ends in the same task as an Enter keydown was sent twice. The keydown settled the pending edit (sending the composed word and setting _dataAlreadySent), then xterm's own keydown finalized the composition synchronously through _finalizeComposition(false), which ignores _dataAlreadySent and sent the word again. settleEdit() now takes the keydown event and, while xterm has a composition in flight (_isSendingComposition), leaves the text to xterm for any key that makes it finalize synchronously. On 229, CapsLock and the modifiers xterm keeps the composition on its async path, which honours _dataAlreadySent, so the edit still applies there. The waiting timers are cleared before that early return, so a timer cannot fire after Enter's textarea clear and send a run of DELs. The guard sits in settleEdit(), not in applyEdit() as the bot proposed. In applyEdit() it would also silence the timer path, where xterm always finalizes asynchronously and skips _dataAlreadySent, so a non-composing character typed just before a composition (the x in xword) would be lost where master and the PR head both deliver it. Two unit tests pin it, both measured: one fails without the guard (the Enter keydown sends 'ab word' instead of 'ab '), and one fails with the guard moved into applyEdit() (the timer path sends 'ab ' instead of 'ab xword'; a 229 settle must also still send the edit). The xterm private-API guard test now also checks the bundle still ships _isSendingComposition, and names it in its failure message and comment. CLAUDE.md: the surviving #441 sentence said a keydown decides before xterm's 229 rescue has run and that Enter's clear makes the pending diff emit nothing. Neither holds any more (the edit diff is settled first, and master already sent one DEL there), so it now says the edit diff is settled first at that keydown. The PR's sentence notes the composition exception. The PR's own changeset is removed; its text goes into the single combined release changeset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Thirteen contributor PRs, each re-checked against its GitHub head, merged with its own merge commit and landing fixes, reviewed, and gated together (524 test files, 10194 tests): #559, #552, #550, #556, #551, #546, #542, #540, #555, #543, #541, #502, #432. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…#541 parity) #541 fixed Android autocorrect duplicating the typed line in the primary pane: xterm's keyCode-229 textarea diff is append-only, so an autocorrect on space (delete a word, insert the corrected one) sent the whole line again. The fix, an edit-based diff that sends one DEL per deleted code point and then the inserted text, lives in terminal-keycode229-recovery.js together with #441's next-keydown drain (a character committed in the same task as Enter goes out ahead of the \r) and the original orphaned-insertText recovery. Only the primary pane created that controller, so a grid tile or the split's Pane B still ran xterm's stock behaviour. Both are gated on width alone (1180 CSS px), which a wide Android tablet clears. TerminalTile now creates its own controller in connect(), after the xterm opens and before the first await, handed this tile's textarea, this tile's CompositionHelper and _onTerminalData as the send path, so recovered bytes go to the tile's own session through the exactly-once queue. As in the primary pane, handleKeyEvent runs first in the custom key handler, above the keyCode-229 early return, and notifyCanonicalData sits in the onData lambda, gated on the same two CodemanTerminalInput predicates, never in _onTerminalData, which the recovered bytes also take. destroy() tears the controller down before disposing the xterm, which restores xterm's own diff and removes the capture listeners. No mode or device gate, matching the primary. The module itself is unchanged apart from its header; terminal-ui.js gains only a comment naming the twin. Tests: test/terminal-tile-input.test.ts now loads the real module into its vm harness (with window timers, without which create() would silently throw and every test would run against no controller) and drives a fake CompositionHelper carrying xterm's own append-only diff. It covers install and restore on the tile's own helper and textarea, autocorrect sent as an edit (with a control reproducing the device-log duplicate), the last character and an autocorrect each followed by Enter in one task, a self-rescued 229 key delivered once, the onData gate ignoring query replies and focus reports, two refused inserts after one keydown both recovered, robustness when the controller throws, per-tile controllers, and a source pin keeping the call above the early return. Removing the create, the handleKeyEvent call, the notify, its gate, or the destroy each turns at least one of them red, as does moving the notify into _onTerminalData. The browser suite gains a TerminalTile block in test/terminal-keycode229-recovery.browser.test.ts (real xterm, trusted execCommand input, chunks asserted to address the tile's session, with a destroyed-controller control). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…follow-up Two lines the #541 parity commit edited still called the split "desktop-only" and listed "Phones and tablets" as a tile grid non-goal, right next to the new note that a wide Android tablet clears the gate. The same commit documents the gate as width alone in terminal-tile.js and architecture-invariants, and that is what the code does: terminal-split.js and canOpenTileGrid in tile-grid.js only compare window.innerWidth with SPLIT_PANE_MIN_WIDTH. CLAUDE.md's Split-pane line now reads "desktop-only at 1180px (width alone, so a wide Android tablet clears it)", in step with the Tile grid line, and the tile-grid-plan non-goal names phones only and says a wide tablet or an unfolded foldable in landscape can reach the grid, pointing at the keyboard exception below it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Grid tiles and the split view's Pane B now get two fixes the primary pane already had in this release: - opencode's hollow-buffer wheel paging and its click reports (#555) - the Android soft-keyboard controller, so autocorrect no longer duplicates a line in a tile (#541, with the #441 drain) Gate green on the branch tip 7409ad2: 525 files and 10243 tests, plus the touched and adjacent browser files. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Merged in 1.40.0, thanks @opticon454! The edit-based diff, settled at the next keydown, fixes a bug that made Android typing feel unreliable. Applied on the way in (d7140c3): a composition that ended in the same task as Enter was sent twice, because xterm's own synchronous finalize ignores |
…erminal as edits — not whole lines again Reported from a phone: an accented letter and the terminal went mad, copying words at random. xterm.js 6.0.0 with Chrome on Android (xtermjs/xterm.js#3600, open since 2022): every key arrives as keyCode 229 and xterm works out what was typed from its hidden textarea with after.replace(before, ''), which is right only when text is appended. So e → è resent the whole textarea, an autocorrection on the space the whole line, a deletion the keyboard made itself (deleteContentBackward, no Backspace) never reached the terminal, and a word composed again was sent on top of the old one. static/js/imefix.js sends each as the edit it is: a DEL per removed code point, then the inserted text, once — by replacing _handleAnyTextareaChanges, wrapping the composition's start and finish, and listening for delete input events. The internals are checked first; a later xterm without them keeps its own behaviour. The same fix other projects made (Ark0N/Codeman#541). Tests replay Chrome-on-Android's event sequences into a pane whose program records the raw bytes it receives: e → è is one DEL and è; peompt → 'prompt ' five DELs and 'rompt '; a keyboard deletion one DEL; a reselected word corrected, not repeated; an emoji one character; the browser's own IME path unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
What
Typing straight into the terminal prompt on an Android keyboard that autocorrects on space (SwiftKey in Edge here, Gboard behaves the same way) duplicated the line:
testing the peompt+ space reached the shell astesting the peompttesting the prompt rompt.Cause
From a real device log (every key is a keyCode-229 keydown plus a plain
insertText; the helper textarea keeps the whole line): on the space, the keyboard sends one edit that deleteseomptand a second that insertsrompt. xterm answers each 229 keydown with_handleAnyTextareaChanges, which diffsnewValue.replace(oldValue, ''). That only works when the keyboard appended. Here the old value is not a substring of the new one, so the diff is the WHOLE new value, and the second keydown's timer then sends the inserted text again. Separately, a multi-character delete was sent as a single DEL. The Compose dialog was unaffected because nothing is sent until Send.Fix
terminal-keycode229-recovery.jsreplaces xterm's_handleAnyTextareaChangeswith an edit-based diff against the value already sent (shared by every keydown's timer, so two timers that both see the final value cannot both send it): one DEL per deleted character (code points, so an emoji is one), then the new text, once. It resynchronises when xterm clears the textarea itself (after Enter) and when a composition commits, stands down while composing, falls back to xterm's own handler if the internals are not what it expects, and restores it on teardown.terminal-ui.jspassesgetCompositionHelper, the same_core._compositionHelperaccessapp.jsalready uses.Tests
test/terminal-keycode229-recovery.test.ts(CI gate, +10): the device sequence as a control that reproduces the corrupted string with xterm alone, then the fixed behaviour; plain typing unchanged; multi-char delete; equal-length rewrite; clear after Enter; emoji; composing; teardown;editBetween.test/terminal-keycode229-recovery.browser.test.ts(browser suite, +2): the same sequence through trustedexecCommandevents in real xterm, plus a control that proves the bug byte for byte.Verified on the reporter's phone after deploy: see comment.