Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/android-last-character-on-enter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"aicodeman": patch
---

Stop a phone keyboard losing the last character of every message it sends. Android soft keyboards commit the last typed character and send the Enter key in one InputConnection transaction, so the `input` event and the Enter keydown are both processed before any zero-delay timer runs. The orphaned-input recovery from #388 only resolved its candidate on such a timer, and lost it both ways: xterm emits `\r` synchronously from the Enter keydown, so the local-echo composer submitted the prompt before the recovered character existed, and that `\r` bumped the "did xterm speak for this keystroke" counter, so the candidate then stood itself down and dropped the character outright. Pending candidates are now drained synchronously at the next keydown, from xterm's custom key handler — before xterm processes that key — so the counter still holds the value it had while the candidate's own keystroke was current, and the recovered byte reaches the composer ahead of the Enter. Typing on a physical keyboard is unaffected: there, the timer has already resolved the candidate before the next key arrives.
37 changes: 37 additions & 0 deletions src/web/public/terminal-keycode229-recovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,38 @@
let composing = false;
const pending = [];

/**
* Resolve every candidate still pending, right now, instead of waiting for
* its zero-delay timer.
*
* Android soft keyboards commit the last character and send the Enter key
* in ONE InputConnection transaction: the `input` event and the Enter
* keydown are both processed before any timer runs. Left on its timer the
* candidate lost BOTH ways — xterm emits '\r' synchronously from the Enter
* keydown (so the local-echo composer submitted the prompt without the
* character), and that '\r' bumps `canonicalCount`, so the candidate then
* read "xterm spoke for this keystroke" and stood down, dropping the
* character outright. That is the "every message loses its last character"
* report from phones.
*
* Draining at the next keydown is correct on both counts: the counter still
* holds the value it had while this candidate's keystroke was current, and
* the byte reaches the composer ahead of whatever the new key emits.
*/
function flushPending() {
for (const candidate of pending.splice(0)) {
if (candidate.timer !== null) {
try {
clearTimer(candidate.timer);
} catch {
// A broken timer host must not break input handling.
}
candidate.timer = null;
}
resolveCandidate(candidate);
}
}

function cancelPending() {
for (const candidate of pending.splice(0)) {
candidate.active = false;
Expand Down Expand Up @@ -111,6 +143,11 @@
*/
function handleKeyEvent(event) {
if (destroyed || event?.type !== 'keydown') return;
// Settle the PREVIOUS keystroke before this one can move the counter or
// reach the PTY — see flushPending(). This runs from xterm's custom key
// handler, i.e. before xterm processes the key, so a recovered character
// is always ordered ahead of the bytes this keydown produces.
flushPending();
keydownSnapshot = canonicalCount;
}

Expand Down
46 changes: 46 additions & 0 deletions test/terminal-keycode229-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,52 @@ describe('orphaned terminal input recovery', () => {
expect(reads).toEqual([]);
});

it('delivers the last character BEFORE the Enter that submits it (defect 4)', () => {
// Android soft keyboards commit the last character and send the Enter key in
// ONE InputConnection transaction, so the `input` event and the Enter keydown
// are processed before any zero-delay timer runs. Two things then went wrong
// with a candidate that only resolved on its timer:
//
// 1. ORDER — xterm emits '\r' synchronously from the Enter keydown, and the
// local-echo composer submits `pendingText` right there. The recovered
// character arrived one macrotask too late to be part of the prompt.
// 2. LOSS — that '\r' bumps the canonical counter, so by the time the
// candidate resolved, `canonicalCount > snapshot` read as "xterm spoke
// for this keystroke" and stood the recovery down. The character was
// dropped outright: every message sent from the phone lost its last
// character.
//
// Resolving pending candidates synchronously at the NEXT keydown fixes both:
// the counter still holds the value it had when that candidate was created,
// and the byte reaches the composer ahead of the Enter.
const h = harness();
h.keydown();
h.input('o');
expect(h.emitted).toEqual([]);

h.keydown({ key: 'Enter' });
expect(h.emitted).toEqual(['o']);

// xterm now emits '\r' for the Enter. The already-resolved candidate must
// not fire a second time when its timer is flushed.
h.controller.notifyCanonicalData();
h.flushTimers();
expect(h.emitted).toEqual(['o']);
expect(h.pendingTimers()).toBe(0);
});

it('still stands down at the next keydown when xterm spoke for the candidate', () => {
// The synchronous resolve must not become a "forward everything" path: a
// keystroke xterm delivered itself is still a duplicate if recovered.
const h = harness();
h.keydown();
h.input('x');
h.controller.notifyCanonicalData();
h.keydown({ key: 'Enter' });
h.flushTimers();
expect(h.emitted).toEqual([]);
});

it('ignores input events that are not committed text', () => {
const h = harness();
for (const inputType of ['insertCompositionText', 'deleteContentBackward', 'insertLineBreak', 'insertFromPaste']) {
Expand Down