fix(terminal): keep IME commit gating after a composition (#1231)

Typing CJK punctuation such as `；` or `，` with fcitx5 in the Linux
AppImage sent each character twice: WebKitGTK commits it as
insertFromComposition + compositionend without compositionstart, so
xterm's CompositionHelper finalized the previous composition's stale
position and emitted a suffix. ImeCommitGate was meant to drop that second
emission, but compositionStart() disabled it for the rest of the session
and cancelled its rebase. The gate now works per commit: compositionstart
stands down only for its own composition, every keydown 229 starts a fresh
commit, and the textarea is rebased after each commit.

A keydown the IME did not take (Enter, an arrow, a letter outside the IME)
ends the open commit, so a key pressed right after a directly committed
punctuation mark is not dropped by the commit's timeout.

Closes #1230

Co-authored-by: devadapter <[email protected]>

src/features/terminal/imeCommit.test.ts

@@ -31,6 +31,12 @@ function gateWithRebase() {
   return { gate, run, pending, rebases: () => rebased };
 }
 
+/** Collects the emissions that reach the PTY, like `Session.fromTerminal`. */
+function emitter(gate: ImeCommitGate) {
+  const sent: string[] = [];
+  return { sent, emit: (data: string) => gate.accept() && sent.push(data) };
+}
+
 describe("ImeCommitGate", () => {
   it("takes the first emission of a commit and drops the repeated one", () => {
     const { gate } = gateWithRebase();
@@ -42,10 +48,7 @@ describe("ImeCommitGate", () => {
   it("keeps the real text of the pi case and drops the stale suffix", () => {
     // xterm sends the committed diff first, then a stale `value.substring(start)`.
     const { gate, run } = gateWithRebase();
-    const sent: string[] = [];
-    const emit = (data: string) => {
-      if (gate.accept()) sent.push(data);
-    };
+    const { sent, emit } = emitter(gate);
     gate.begin();
     emit("在吗");
     gate.finish();
@@ -54,37 +57,101 @@ describe("ImeCommitGate", () => {
     expect(sent).toEqual(["在吗"]);
   });
 
-  it("accepts every emission once the commit is over", () => {
-    const { gate, run } = gateWithRebase();
+  it("lets a key the IME did not take through right after a commit", () => {
+    // Punctuation committed directly, then Enter within the commit's timeout: Enter must run.
+    const { gate, rebases } = gateWithRebase();
+    const { sent, emit } = emitter(gate);
+    gate.begin();
+    emit("，");
+    gate.keystroke();
+    emit("\r");
+    expect(sent).toEqual(["，", "\r"]);
+    expect(rebases()).toBe(1);
+    // Outside a commit, or during a composition, a keystroke changes nothing.
+    gate.keystroke();
+    gate.compositionStart();
+    gate.keystroke();
+    expect(rebases()).toBe(1);
+  });
+
+  it("starts a new commit on the next IME keystroke even inside the timeout", () => {
+    // Two commits can arrive back to back; the second must not be swallowed by the first
+    // commit's still-running timeout.
+    const { gate } = gateWithRebase();
+    gate.begin();
+    expect(gate.accept()).toBe(true);
     gate.begin();
     expect(gate.accept()).toBe(true);
     expect(gate.accept()).toBe(false);
+  });
+
+  it("drops the stale suffix of a WebKitGTK punctuation commit without compositionstart", () => {
+    // fcitx5 commits `，` as insertFromComposition + compositionend without compositionstart:
+    // xterm sends the keydown diff `，`, then compositionend emits `好，` from the previous
+    // composition's position. That suffix is what showed up as a duplicate in the terminal.
+    const { gate, run, rebases } = gateWithRebase();
+    const { sent, emit } = emitter(gate);
+    gate.begin();
+    emit("，");
+    gate.finish();
+    emit("好，");
+    run();
+    expect(sent).toEqual(["，"]);
+    expect(rebases()).toBe(1);
+  });
+
+  it("stands down for a real composition, then gates the next commit again", () => {
+    // compositionstart must not disable the gate for the whole session: the very next
+    // punctuation commit still needs deduplication.
+    const { gate, run, rebases } = gateWithRebase();
+    gate.begin();
+    gate.compositionStart();
+    expect(gate.accept()).toBe(true);
+    expect(gate.accept()).toBe(true);
     gate.finish();
     run();
+    expect(rebases()).toBe(1);
+
+    gate.begin();
+    expect(gate.accept()).toBe(true);
+    expect(gate.accept()).toBe(false);
+  });
+
+  it("accepts every emission while composing", () => {
+    const { gate } = gateWithRebase();
+    gate.compositionStart();
+    expect(gate.accept()).toBe(true);
     expect(gate.accept()).toBe(true);
     expect(gate.accept()).toBe(true);
   });
 
-  it("rebases the textarea when the commit closes, not before", () => {
+  it("rebases after a composition that xterm sent by itself", () => {
     const { gate, run, rebases } = gateWithRebase();
-    gate.begin();
+    gate.compositionStart();
     gate.finish();
     expect(rebases()).toBe(0);
     run();
     expect(rebases()).toBe(1);
   });
 
-  it("stands down once a real compositionstart arrives", () => {
-    const { gate, rebases } = gateWithRebase();
+  it("rebases after a commit whose compositionend arrives, not before", () => {
+    const { gate, run, rebases } = gateWithRebase();
     gate.begin();
-    gate.compositionStart();
-    expect(gate.accept()).toBe(true);
     expect(gate.accept()).toBe(true);
+    gate.finish();
     expect(rebases()).toBe(0);
-    // And a later commit is not gated either.
-    gate.begin();
-    expect(gate.accept()).toBe(true);
+    run();
+    expect(rebases()).toBe(1);
+  });
+
+  it("opens a commit from a composition input when no keydown preceded it", () => {
+    const { gate } = gateWithRebase();
+    gate.input();
     expect(gate.accept()).toBe(true);
+    expect(gate.accept()).toBe(false);
+    // A second input event belongs to the same commit and must not reopen it.
+    gate.input();
+    expect(gate.accept()).toBe(false);
   });
 
   it("closes a commit whose compositionend never arrives", () => {
@@ -96,4 +163,13 @@ describe("ImeCommitGate", () => {
     expect(rebases()).toBe(1);
     expect(gate.accept()).toBe(true);
   });
+
+  it("does not rebase into a composition that started before the close ran", () => {
+    const { gate, run, rebases } = gateWithRebase();
+    gate.begin();
+    gate.finish();
+    gate.compositionStart();
+    run();
+    expect(rebases()).toBe(0);
+  });
 });

src/features/terminal/imeCommit.ts

@@ -1,14 +1,31 @@
 /**
  * Collapses the repeated data emissions of one IME commit.
  *
- * WebKitGTK + fcitx5 commit text without firing `compositionstart`, so xterm's `CompositionHelper`
- * never advances `_compositionPosition.start` and keeps appending the committed text to its hidden
- * textarea. A commit then reaches `onData` twice — once with the committed text, once with a stale
- * `value.substring(start)` — and the same text shows up in the terminal twice.
+ * WebKitGTK commits IME text with an inconsistent mix of events, and xterm's
+ * `CompositionHelper` only copes with part of it:
  *
- * One commit must send one chunk: take the first emission and drop the rest, then rebase the
- * textarea so the next commit starts from an empty value. IMEs that do report `compositionstart`
- * (macOS, IBus) keep xterm's own bookkeeping, so the gate stands down for them.
+ * - A composed word (`你好`) reports `compositionstart`, so xterm records where the
+ *   composition starts in its hidden textarea and sends the committed text once on
+ *   `compositionend`.
+ * - Text that the IME commits directly (punctuation such as `；` or `，`) reports
+ *   `insertFromComposition` + `compositionend` **without** `compositionstart`. xterm then
+ *   finalizes the *previous* composition's position: it sends the keydown's textarea diff
+ *   first and, once `compositionend` runs, a stale `value.substring(start)` on top. That
+ *   second emission is the duplicated suffix users see (typing `；你好，` sends `，` and then
+ *   `好，`). Newer WebKitGTK delivers the same commits as `insertText` without any
+ *   composition events, which xterm sends through its keydown path; the stale emission there
+ *   carries the previous commit's text instead.
+ *
+ * One commit must send one chunk, so the gate lets the first emission through and drops the
+ * rest until the commit closes. A commit is delimited by the IME's own keystroke (code 229)
+ * outside a composition, or by a composition input when the platform sends no keydown.
+ * Commits that really compose are left to xterm: `compositionstart` stands the gate down for
+ * that commit only — it must not disable the gate for the session, because the very next
+ * punctuation commit still needs deduplication.
+ *
+ * xterm never clears its hidden textarea by itself, so every commit ends with a rebase: the
+ * textarea is emptied after xterm's own senders have run, keeping the next commit from
+ * carrying old text.
  */
 
 /** How long a commit may stay open when its `compositionend` never arrives. */
@@ -27,8 +44,8 @@ const WINDOW_SCHEDULER: ImeScheduler = {
 export class ImeCommitGate {
   private open = false;
   private sent = false;
+  private composing = false;
   private timer: number | null = null;
-  private native = false;
   private readonly rebase: () => void;
   private readonly scheduler: ImeScheduler;
 
@@ -37,21 +54,61 @@ export class ImeCommitGate {
     this.scheduler = scheduler;
   }
 
-  /** A commit starts: the IME's own keystroke (code 229) or its composition input event. */
+  /**
+   * A commit starts: the IME's own keystroke (code 229) outside a composition.
+   *
+   * Always starts a fresh commit, even when the previous one is still inside its timeout:
+   * two commits can arrive back to back, and the second must not be swallowed.
+   */
   begin(): void {
-    if (this.native || this.open) return;
+    if (this.composing) return;
+    this.cancelTimer();
     this.open = true;
     this.sent = false;
     this.timer = this.scheduler.after(() => this.close(), IME_COMMIT_TIMEOUT_MS);
   }
 
-  /** The commit ended: let the emissions both of xterm's senders queued go out, then rebase. */
+  /**
+   * A composition input arrived. Opens a commit only when no commit is open: resetting
+   * `sent` here would let the emissions already queued for this commit go out again.
+   */
+  input(): void {
+    if (this.composing || this.open) return;
+    this.open = true;
+    this.sent = false;
+    this.timer = this.scheduler.after(() => this.close(), IME_COMMIT_TIMEOUT_MS);
+  }
+
+  /**
+   * An IME composition begins. xterm's own composition handling owns this commit, so the
+   * gate stays out of its way — but only until the composition ends.
+   */
+  compositionStart(): void {
+    this.composing = true;
+    this.cancelTimer();
+    this.open = false;
+    this.sent = false;
+  }
+
+  /**
+   * The composition — or, on WebKitGTK, a directly committed character — ended.
+   * Bubble phase: xterm's own `compositionend` listener ran first and queued its send, so a
+   * 0 ms close lands behind that emission and cannot cut the committed text off.
+   */
   finish(): void {
-    if (!this.open) return;
+    this.composing = false;
     this.cancelTimer();
     this.timer = this.scheduler.after(() => this.close(), 0);
   }
 
+  /**
+   * A keystroke the IME did not take (Enter, an arrow, a letter typed outside it): the commit
+   * is over, and what that key sends must reach the PTY.
+   */
+  keystroke(): void {
+    if (this.open && !this.composing) this.close();
+  }
+
   /** Whether this emission should reach the PTY. */
   accept(): boolean {
     if (!this.open) return true;
@@ -60,19 +117,13 @@ export class ImeCommitGate {
     return true;
   }
 
-  /** The IME reports composition events, so xterm needs no help — stay out of its way. */
-  compositionStart(): void {
-    this.native = true;
-    this.cancelTimer();
-    this.open = false;
-    this.sent = false;
-  }
-
   /** Ends the commit and rebases the textarea to an empty value. */
   close(): void {
     this.cancelTimer();
-    if (!this.open) return;
+    // A new composition started before this close ran; its own finish will rebase.
+    if (this.composing) return;
     this.open = false;
+    this.sent = false;
     this.rebase();
   }
 

src/features/terminal/session.ts

@@ -413,17 +413,18 @@ export class TerminalSession {
 
   /**
    * Routes a WebKitGTK / fcitx5 IME commit through `ImeCommitGate`: the platform reports the
-   * composition's own keystroke (code 229) and its input event but never `compositionstart`,
-   * which is what xterm's own bookkeeping is built on.
+   * commit's own keystroke (code 229) and its input event, but reports `compositionstart` only
+   * for composed words — never for the punctuation an IME commits directly.
    */
   private watchImeCommit() {
     const textarea = this.term.textarea;
     if (!textarea) return;
     const onKeyDown = (e: KeyboardEvent) => {
       if (e.keyCode === 229) this.ime.begin();
+      else this.ime.keystroke();
     };
     const onInput = (e: InputEvent) => {
-      if (e.inputType === "insertFromComposition" || e.inputType === "insertCompositionText") this.ime.begin();
+      if (e.inputType === "insertFromComposition" || e.inputType === "insertCompositionText") this.ime.input();
     };
     const onCompositionStart = () => this.ime.compositionStart();
     // Bubble phase, i.e. after xterm's own listener: the rebase must land behind the emissions