Fast IME typing drops a committed composition: `_isSendingComposition` cancels every pending send, not just one
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 21.2k
- Forks
- 2k
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 3
Description
CompositionHelper defers each finished composition to a setTimeout(…, 0) and guards every
pending send with a single shared boolean, _isSendingComposition. The code assumes that timer
runs before the next composition finishes, so that at most one send is ever in flight.
Nothing guarantees that. Under load, input events arrive in bursts and two compositions can
complete before the timer gets a turn, leaving two sends pending at once.
The next non-composition keystroke then calls _finalizeComposition(false), which sets that
boolean to false intending to cancel the pending send — and cancels all of them,
including one that has not run yet. The composition it was carrying is dropped silently: no
error, no partial glyph, the text simply never reaches the pty.
This is a scheduling bug, not an off-by-one in the offset arithmetic. The dropped composition's
slice would have been computed correctly had its callback been allowed to run. It is invisible
on an idle page, which is why a bare demo looks fine while a real application drops characters.
Details
- Browser and browser version: Google Chrome 151.0.7922.71 (native Wayland,
--enable-wayland-ime --wayland-text-input-version=3); originally found in VS Code 1.130.0 / Electron - OS version: NixOS, Wayland session (Hyprland 0.56.1), fcitx5 5.1.21 + fcitx5-hangul, Korean Dubeolsik
- xterm.js version: reproduced on
6.0.0(latest),6.1.0-beta.292(beta) and6.1.0-beta.288; the code is unchanged on currentmaster
Steps to reproduce
Standalone xterm.js in a plain page — no Electron, no pty, no framework. A Terminal, an
onData handler recording what the library emits, and a timer that blocks the main thread.
const term = new Terminal({ rows: 6 });
term.open(document.getElementById('term'));
const chunks = [];
term.onData(d => { chunks.push(d); term.write(d); render(chunks); });
// Stand in for a busy application. Without this the bug does not appear.
// Both 100ms-every-40ms and 80ms-every-100ms reproduce it; what matters is that
// input events queue during the block and are then delivered back to back.
setInterval(() => {
const until = performance.now() + 100;
while (performance.now() < until) { /* make input events queue up */ }
}, 40);
- Focus the terminal and switch to a Korean IME (fcitx5-hangul, Dubeolsik).
- Type
알겠습니다.— including the trailing period — at a normal-to-fast pace.
Expected: ["알", "겠", "습", "니", "다", "."]
Actual: ["알", "겠", "습", "다", "."] — the syllable 니 is gone.
Remove the load and the same typing is correct every time. In the VS Code integrated terminal
it happens with no artificial load at all — the renderer is busy enough on its own; cat was
running, so nothing but xterm.js sat between the keyboard and the pty.
Korean makes this easy to hit because a syllable commits the instant the next one starts, so a
fast run produces back-to-back compositionend → compositionstart pairs. Any IME with that
commit pattern should reproduce it.
The invariant that breaks
The deferred send is only safe if the 0 ms timer runs between one compositionend and the
next. If it does, at most one send is ever pending and the single boolean is sufficient.
Nothing enforces that ordering, and it fails in at least two distinct ways.
The setTimeout(0) fired after … lines below are an independent 0 ms timer scheduled from
each compositionend, standing in for xterm.js's own flush.
Route 1 — the timer is starved (blocking 100 ms every 40 ms):
4963ms compositionend data="니" ← 니 finishes, flush T_니 scheduled
4963ms compositionstart data="" ← 다 begins
5065ms compositionupdate data="다"
5066ms compositionend data="다" ← 다 finishes — T_니 STILL PENDING
5068ms └ setTimeout(0) fired after 104.2ms ← T_니's turn, 100ms too late
5170ms └ setTimeout(0) fired after 103.6ms
Route 2 — the timer is barely delayed at all, but the next composition is faster
(blocking 80 ms every 100 ms):
2500ms compositionend data="니" ← 니 finishes, flush T_니 scheduled
2501ms compositionstart data="" ← 다 begins
2501ms compositionupdate data="다"
2501ms compositionupdate data="다"
2501ms compositionend data="다" ← the whole 다 composition, inside 1ms
2503ms └ setTimeout(0) fired after 2.3ms ← T_니 was only 2ms late — still too late
2503ms └ setTimeout(0) fired after 1.7ms
Route 2 is the interesting one, and it is why "make the timer faster" is not a fix. The
flush was delayed by 2.3 ms. It still lost, because 다 started and finished in the
millisecond after 니 ended — the input events had queued up behind the blocked main
thread and were delivered back to back once it freed up, several of them inside one task,
before the timer queue was serviced at all.
So load is the trigger, not the mechanism. Load causes input events to arrive in bursts; a
burst lets two compositions complete before one setTimeout(…, 0) gets a turn; and from
there the shared boolean does the damage. In both runs the . keypress that follows takes
the synchronous path, emits 다 from _compositionPosition, and clears
_isSendingComposition — after which both pending timers find it false and return without
sending. 니 is lost.
With the load off, the same page and the same typing produce the correct
["알","겠","습","니","다","."] every time. That is the entire difference between "works"
and "drops characters", and it is why this survives in a bare demo while showing up
immediately in a real application.
Root cause
// _finalizeComposition(true) — one boolean for however many sends are in flight
this._isSendingComposition = true;
setTimeout(() => {
if (this._isSendingComposition) { // ← "is *any* send live?", not "is *mine* live?"
this._isSendingComposition = false;
…
}
}, 0);
// _finalizeComposition(false) — reached from keydown for a non-composition key
this._isSendingComposition = false; // ← intends to cancel one; cancels all
const input = this._textarea.value.substring(
this._compositionPosition.start, this._compositionPosition.end);
this._coreService.triggerDataEvent(input, true);
The field's own comment says it plainly — "Whether a composition is in the process of being
sent, setting this to false will cancel any in-progress composition" — and the singular "a
composition" is the assumption that breaks. Nothing serialises the sends or prevents two
from overlapping.
Reproduced on 6.0.0, 6.1.0-beta.288 and 6.1.0-beta.292 — the structure is unchanged
across all of them, and unchanged on master as of today.
Suggested fix
Replace the shared boolean with an ordered queue, and have the synchronous path drain it
rather than cancel it. A send watermark then makes double-sending impossible regardless of
which path emits first:
private _pendingSends: (() => void)[] = [];
private _sentUpTo: number = 0;
private _finalizeComposition(waitForPropagation: boolean): void {
this._compositionView.classList.remove('active');
this._isComposing = false;
if (waitForPropagation) {
const pos = { start: this._compositionPosition.start, end: this._compositionPosition.end };
const suffix = this._compositionSuffix;
const alreadySent = this._dataAlreadySent;
const send = (): void => {
this._isSendingComposition = false;
pos.start += alreadySent.length;
const value = this._textarea.value;
// Not `_isComposing` — that is already false when this runs as part of a
// drain. Ask what the check actually meant: did a newer composition start
// past my start offset? If so, its start is my end boundary.
const end = this._compositionPosition.start > pos.start
? this._compositionPosition.start
: (suffix.length > 0 && value.endsWith(suffix)
? value.length - suffix.length
: value.length);
const from = Math.max(pos.start, this._sentUpTo);
const to = Math.max(from, end);
const input = value.substring(from, to);
this._sentUpTo = Math.max(this._sentUpTo, to);
if (input.length > 0) {
this._coreService.triggerDataEvent(input, true);
}
};
this._pendingSends.push(send);
this._isSendingComposition = true;
setTimeout(() => {
const i = this._pendingSends.indexOf(send);
if (i >= 0) {
this._pendingSends.splice(i, 1);
send(); // still mine to run — nobody else cancelled it
}
}, 0);
} else {
// Flush what is already queued, in order, before emitting our own slice.
for (const send of this._pendingSends.splice(0, this._pendingSends.length)) {
send();
}
this._isSendingComposition = false;
const value = this._textarea.value;
const end = Math.max(this._compositionPosition.end,
this._textarea.selectionEnd ?? this._compositionPosition.end);
const from = Math.max(this._compositionPosition.start, this._sentUpTo);
const to = Math.max(from, end);
const input = value.substring(from, to);
this._sentUpTo = Math.max(this._sentUpTo, to);
if (input.length > 0) {
this._coreService.triggerDataEvent(input, true);
}
}
}
_sentUpTo has to be lowered again when the textarea is rewritten underneath the helper
(_syncTextArea), otherwise a stale high watermark suppresses later sends. Doing it in
compositionstart is enough:
this._sentUpTo = Math.min(this._sentUpTo, this._compositionPosition.start);
Two details worth flagging for review:
- The end boundary in the deferred path can no longer branch on
this._isComposing. When
the callback runs as part of a drain,_finalizeCompositionhas already set it to
false, so the old check would take the wrong branch and over-read into the next
composition's preedit. Comparing_compositionPosition.startagainst the captured
pos.startexpresses the original intent without depending on timing. - The watermark is what lets the drain be unconditional. Without it, draining the queued
다and then emitting the synchronous slice would send it twice.
Verification
I replayed the captured event sequence — the real offsets and textarea contents from a
failing run — against both implementations:
current chunks=["알","겠","습"] joined="알겠습"
patched chunks=["알","겠","습","니","다"] joined="알겠습니다"
The patched build has since been in daily use for Korean input with no drops and no
duplicates, including the 받침 + vowel cases (국어, 옷을, 밥이, 학교에서) that the
existing comment in _finalizeComposition warns about — the fix stays offset-based, so that
concern is untouched.
Happy to open a PR with the above plus a unit test that drives the recorded event sequence,
if the approach looks right.
Related, but not the same
- #6049 also lands in
_finalizeComposition, but it is an offset defect —
_compositionPosition.startgoing stale when a TSF IME replaces the whole textarea value.
Here the offsets are correct; the callback that would have used them never runs. - #6045 and #5887 concern
_handleAnyTextareaChangesand the_keyDownSeengate on
insertText, in IME modes that emit nocomposition*events at all. This one needs no
unusual IME behaviour — only two compositions finishing before one 0 ms timer gets a turn. - microsoft/vscode#267568 is labelled
upstreambut describes raw key events being consumed
instead of macOS IME output, which is a different mechanism again.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating CompositionHelper and read _finalizeComposition alongside _syncTextArea, focusing on how deferred composition sends and textarea offsets are tracked. Replay the captured event sequence described in the issue, then verify that fast consecutive compositions reach onData without drops or duplicates and that later textarea rewrites still send correctly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100