mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
CompositionHelper.keydown exempted keyCodes 16/17/18 from tearing a composition down, which covers Shift/Ctrl/Alt but not macOS Meta — 91/93 in Chromium, 224 in Firefox. A lone Cmd press mid-composition therefore reached _finalizeComposition(false), which dropped the preedit overlay's `active` class and committed the live syllable early. macOS keeps the marked text alive across that press, so no later compositionstart re-arms the overlay and the rest of the word composes invisibly. Measured on hardware (m4air, macOS 26.5.2, Apple M4, 2-Set Korean) with the Cmd posted as a CGEventType.flagsChanged, which is what a physical modifier emits. AppleScript `key code 55` posts nothing a browser can see — a bare `key code 56` for Shift is equally silent — which is why no capture in the corpus ever reached this branch. Three arms, same build otherwise: overlay live throughout with the exemption, dark and prematurely committed without it, live again with it restored. Evidence under .tmp/ime-handoff/swarm-scratch/wave31-cmd-preedit/evidence/. The fix cannot widen past a lone modifier: only a standalone press reports these keyCodes, and a Cmd chord during composition is reported by Chromium as 229, which was already exempt. Cmd+A still ends the composition, via the IME's own compositionend. Ghostty draws the same line, returning early from flagsChanged under hasMarkedText() for every modifier including Super. Orca's terminal pane was never affected — shouldSuppressTerminalModifierKeyboardEvent drops a standalone Meta keydown before xterm sees it, and deleting only 'Meta' from that set is what flipped the hardware arm to broken. The popout preview terminal and mobile's webview install no such guard and did reach the teardown. terminal-ime-xterm-composition-commit-overlap.test.ts asked its fixer to update the two Cmd arms to the values it named as correct; both now emit a single ['한'].
364 lines
17 KiB
Diff
364 lines
17 KiB
Diff
diff --git a/src/browser/CoreBrowserTerminal.ts b/src/browser/CoreBrowserTerminal.ts
|
|
index 4557e1652c34737fdf853436bd9328d9918eee2b..c16096341dadfd215a8086e13f7a0551025c0e77 100644
|
|
--- a/src/browser/CoreBrowserTerminal.ts
|
|
+++ b/src/browser/CoreBrowserTerminal.ts
|
|
@@ -735,6 +735,10 @@ export class CoreBrowserTerminal extends CoreTerminal implements ITerminal {
|
|
paste(data, this.textarea!, this.coreService, this.optionsService);
|
|
}
|
|
|
|
+ public override input(data: string, wasUserInput: boolean = true): void {
|
|
+ if (!wasUserInput || !this._compositionHelper?.handleCompositionInput(data, false)) super.input(data, wasUserInput);
|
|
+ }
|
|
+
|
|
public attachCustomKeyEventHandler(customKeyEventHandler: CustomKeyEventHandler): void {
|
|
this._customKeyEventHandler = customKeyEventHandler;
|
|
}
|
|
@@ -1029,6 +1033,7 @@ export class CoreBrowserTerminal extends CoreTerminal implements ITerminal {
|
|
// Only support emoji IMEs when screen reader mode is disabled as the event must bubble up to
|
|
// support reading out character input which can doubling up input characters
|
|
// Based on these event traces: https://github.com/xtermjs/xterm.js/issues/3679
|
|
+ if (ev.data && ev.inputType === 'insertText' && this._compositionHelper?.handleCompositionInput(ev.data, true)) return true;
|
|
if (ev.data && ev.inputType === 'insertText' && (!ev.composed || !this._keyDownSeen) && !this.optionsService.rawOptions.screenReaderMode) {
|
|
if (this._keyPressHandled) {
|
|
return false;
|
|
diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts
|
|
index 7b346459521dd77f9de25d43fba3c36a6f8459b2..4837a7c1a3eb36ec6f6223cbcec0f2dd6cc4e235 100644
|
|
--- a/src/browser/TestUtils.test.ts
|
|
+++ b/src/browser/TestUtils.test.ts
|
|
@@ -342,6 +342,9 @@ export class MockViewport implements IViewport {
|
|
}
|
|
|
|
export class MockCompositionHelper implements ICompositionHelper {
|
|
+ public handleCompositionInput(data: string, nativeCommit: boolean): boolean {
|
|
+ throw new Error('Method not implemented.');
|
|
+ }
|
|
public get isComposing(): boolean {
|
|
return false;
|
|
}
|
|
diff --git a/src/browser/Types.ts b/src/browser/Types.ts
|
|
index 497afcf535f3eaca00889525a77e15eb633ccd96..e3cad77734795f6cf34bb7120264fe81070941ed 100644
|
|
--- a/src/browser/Types.ts
|
|
+++ b/src/browser/Types.ts
|
|
@@ -39,6 +39,7 @@ export type LineData = CharData[];
|
|
|
|
export interface ICompositionHelper {
|
|
readonly isComposing: boolean;
|
|
+ handleCompositionInput(data: string, nativeCommit: boolean): boolean;
|
|
compositionstart(): void;
|
|
compositionupdate(ev: CompositionEvent): void;
|
|
compositionend(): void;
|
|
diff --git a/src/browser/input/CompositionHelper.test.ts b/src/browser/input/CompositionHelper.test.ts
|
|
index 5a1e6c38c9799f7f57d5df6d4a122a7beb0cf04f..2d78e414177804261acbbe2f53d71b5c8709ceb6 100644
|
|
--- a/src/browser/input/CompositionHelper.test.ts
|
|
+++ b/src/browser/input/CompositionHelper.test.ts
|
|
@@ -6,7 +6,7 @@
|
|
import { assert } from 'chai';
|
|
import { CompositionHelper } from './CompositionHelper';
|
|
import { MockRenderService } from '../TestUtils.test';
|
|
-import { MockCoreService, MockBufferService, MockOptionsService } from '../../common/TestUtils.test';
|
|
+import { MockCoreService, MockBufferService, MockOptionsService, MockUnicodeService } from '../../common/TestUtils.test';
|
|
|
|
describe('CompositionHelper', () => {
|
|
let compositionHelper: CompositionHelper;
|
|
@@ -42,7 +42,7 @@ describe('CompositionHelper', () => {
|
|
};
|
|
handledText = '';
|
|
const bufferService = new MockBufferService(10, 5);
|
|
- compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), coreService, new MockRenderService());
|
|
+ compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), coreService, new MockRenderService(), new MockUnicodeService());
|
|
});
|
|
|
|
describe('Input', () => {
|
|
diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts
|
|
index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..ea77d070322479e75201970d63fc41336d64dc93 100644
|
|
--- a/src/browser/input/CompositionHelper.ts
|
|
+++ b/src/browser/input/CompositionHelper.ts
|
|
@@ -4,8 +4,7 @@
|
|
*/
|
|
|
|
import { IRenderService } from '../services/Services';
|
|
-import { IBufferService, ICoreService, IOptionsService } from '../../common/services/Services';
|
|
-import { C0 } from '../../common/data/EscapeSequences';
|
|
+import { IBufferService, ICoreService, IOptionsService, IUnicodeService } from '../../common/services/Services';
|
|
|
|
interface IPosition {
|
|
start: number;
|
|
@@ -42,15 +41,12 @@ export class CompositionHelper {
|
|
*/
|
|
private _isSendingComposition: boolean;
|
|
|
|
- /**
|
|
- * Data already sent due to keydown event.
|
|
- */
|
|
- private _dataAlreadySent: string;
|
|
+ private _pendingCompositionStart?: number;
|
|
+ private _pendingInput = '';
|
|
+ private _sentComposition = '';
|
|
|
|
- /**
|
|
- * The pending textarea change timer, if any.
|
|
- */
|
|
- private _textareaChangeTimer?: number;
|
|
+ /** Text and cell width the current letter-spacing was measured for. */
|
|
+ private _gridAdvanceKey = '';
|
|
|
|
constructor(
|
|
private readonly _textarea: HTMLTextAreaElement,
|
|
@@ -58,13 +54,13 @@ export class CompositionHelper {
|
|
@IBufferService private readonly _bufferService: IBufferService,
|
|
@IOptionsService private readonly _optionsService: IOptionsService,
|
|
@ICoreService private readonly _coreService: ICoreService,
|
|
- @IRenderService private readonly _renderService: IRenderService
|
|
+ @IRenderService private readonly _renderService: IRenderService,
|
|
+ @IUnicodeService private readonly _unicodeService: IUnicodeService
|
|
) {
|
|
this._isComposing = false;
|
|
this._isSendingComposition = false;
|
|
this._compositionPosition = { start: 0, end: 0 };
|
|
this._compositionSuffix = '';
|
|
- this._dataAlreadySent = '';
|
|
}
|
|
|
|
/**
|
|
@@ -80,10 +76,27 @@ export class CompositionHelper {
|
|
this._compositionPosition.end = Math.max(start, end);
|
|
this._compositionSuffix = this._textarea.value.substring(this._compositionPosition.end);
|
|
this._compositionView.textContent = '';
|
|
- this._dataAlreadySent = '';
|
|
this._compositionView.classList.add('active');
|
|
}
|
|
|
|
+ public handleCompositionInput(data: string, nativeCommit: boolean): boolean {
|
|
+ if (nativeCommit) {
|
|
+ if (!this._isSendingComposition) return false;
|
|
+ // Once the deferred send has run, an IME that delivers its commit a task late (IBus) repeats
|
|
+ // text we already sent; anything else is new input and must not be discarded with it.
|
|
+ const alreadySent = this._pendingCompositionStart === undefined && data === this._sentComposition;
|
|
+ const input = (alreadySent ? '' : data) + this._pendingInput;
|
|
+ this._pendingCompositionStart = undefined;
|
|
+ this._pendingInput = '';
|
|
+ if (input.length > 0) this._coreService.triggerDataEvent(input, true);
|
|
+ this._isSendingComposition = false;
|
|
+ return true;
|
|
+ }
|
|
+ if (!this._isComposing && !this._isSendingComposition) return false;
|
|
+ this._pendingInput += data;
|
|
+ return true;
|
|
+ }
|
|
+
|
|
/**
|
|
* Handles the compositionupdate event, updating the composition view.
|
|
* @param ev The event.
|
|
@@ -119,8 +132,10 @@ export class CompositionHelper {
|
|
// Continue composing if the keyCode is the "composition character"
|
|
return false;
|
|
}
|
|
- if (ev.keyCode === 16 || ev.keyCode === 17 || ev.keyCode === 18) {
|
|
- // Continue composing if the keyCode is a modifier key
|
|
+ if (ev.keyCode === 16 || ev.keyCode === 17 || ev.keyCode === 18 || ev.keyCode === 91 || ev.keyCode === 93 || ev.keyCode === 224) {
|
|
+ // Continue composing if the keyCode is a modifier key. 91/93 are Chromium's left and
|
|
+ // right Meta, 224 is Firefox's. Only a lone modifier press reports these, and on macOS
|
|
+ // finalizing there hides the preedit for the rest of a word the IME is still composing.
|
|
return false;
|
|
}
|
|
// Finish composition immediately. This is mainly here for the case where enter is
|
|
@@ -129,9 +144,6 @@ export class CompositionHelper {
|
|
}
|
|
|
|
if (ev.keyCode === 229) {
|
|
- // If the "composition character" is used but gets to this point it means a non-composition
|
|
- // character (eg. numbers and punctuation) was pressed when the IME was active.
|
|
- this._handleAnyTextareaChanges();
|
|
return false;
|
|
}
|
|
|
|
@@ -153,7 +165,11 @@ export class CompositionHelper {
|
|
if (!waitForPropagation) {
|
|
// Cancel any delayed composition send requests and send the input immediately.
|
|
this._isSendingComposition = false;
|
|
- const input = this._textarea.value.substring(this._compositionPosition.start, this._compositionPosition.end);
|
|
+ const start = this._pendingCompositionStart ?? this._compositionPosition.start;
|
|
+ const end = Math.max(start, this._textarea.selectionEnd ?? this._compositionPosition.end);
|
|
+ this._pendingCompositionStart = undefined;
|
|
+ const input = this._textarea.value.substring(start, end) + this._pendingInput;
|
|
+ this._pendingInput = '';
|
|
this._coreService.triggerDataEvent(input, true);
|
|
} else {
|
|
// Make a deep copy of the composition position here as a new compositionstart event may
|
|
@@ -163,6 +179,7 @@ export class CompositionHelper {
|
|
end: this._compositionPosition.end
|
|
};
|
|
const currentCompositionSuffix = this._compositionSuffix;
|
|
+ this._pendingCompositionStart ??= currentCompositionPosition.start;
|
|
|
|
// Since composition* events happen before the changes take place in the textarea on most
|
|
// browsers, use a setTimeout with 0ms time to allow the native compositionend event to
|
|
@@ -175,12 +192,10 @@ export class CompositionHelper {
|
|
this._isSendingComposition = true;
|
|
setTimeout(() => {
|
|
// Ensure that the input has not already been sent
|
|
- if (this._isSendingComposition) {
|
|
- this._isSendingComposition = false;
|
|
+ if (this._isSendingComposition && this._pendingCompositionStart !== undefined) {
|
|
+ currentCompositionPosition.start = this._pendingCompositionStart;
|
|
+ this._pendingCompositionStart = undefined;
|
|
let input;
|
|
- // Add length of data already sent due to keydown event,
|
|
- // otherwise input characters can be duplicated. (Issue #3191)
|
|
- currentCompositionPosition.start += this._dataAlreadySent.length;
|
|
if (this._isComposing) {
|
|
// Use the start position of the new composition to get the string
|
|
// if a new composition has started.
|
|
@@ -195,47 +210,22 @@ export class CompositionHelper {
|
|
: value.length;
|
|
input = value.substring(currentCompositionPosition.start, Math.max(currentCompositionPosition.start, valueEnd));
|
|
}
|
|
- if (input.length > 0) {
|
|
- this._coreService.triggerDataEvent(input, true);
|
|
- }
|
|
+ this._sentComposition = input;
|
|
+ input += this._pendingInput;
|
|
+ this._pendingInput = '';
|
|
+ if (input.length > 0) this._coreService.triggerDataEvent(input, true);
|
|
+ setTimeout(() => {
|
|
+ if (this._pendingCompositionStart === undefined) {
|
|
+ if (this._pendingInput.length > 0) this._coreService.triggerDataEvent(this._pendingInput, true);
|
|
+ this._pendingInput = '';
|
|
+ this._isSendingComposition = false;
|
|
+ }
|
|
+ }, 0);
|
|
}
|
|
}, 0);
|
|
}
|
|
}
|
|
|
|
- /**
|
|
- * Apply any changes made to the textarea after the current event chain is allowed to complete.
|
|
- * This should be called when not currently composing but a keydown event with the "composition
|
|
- * character" (229) is triggered, in order to allow non-composition text to be entered when an
|
|
- * IME is active.
|
|
- */
|
|
- private _handleAnyTextareaChanges(): void {
|
|
- if (this._textareaChangeTimer) {
|
|
- return;
|
|
- }
|
|
- const oldValue = this._textarea.value;
|
|
- this._textareaChangeTimer = window.setTimeout(() => {
|
|
- this._textareaChangeTimer = undefined;
|
|
- // Ignore if a composition has started since the timeout
|
|
- if (!this._isComposing) {
|
|
- const newValue = this._textarea.value;
|
|
-
|
|
- const diff = newValue.replace(oldValue, '');
|
|
-
|
|
- this._dataAlreadySent = diff;
|
|
-
|
|
- if (newValue.length > oldValue.length) {
|
|
- this._coreService.triggerDataEvent(diff, true);
|
|
- } else if (newValue.length < oldValue.length) {
|
|
- this._coreService.triggerDataEvent(`${C0.DEL}`, true);
|
|
- } else if ((newValue.length === oldValue.length) && (newValue !== oldValue)) {
|
|
- this._coreService.triggerDataEvent(newValue, true);
|
|
- }
|
|
-
|
|
- }
|
|
- }, 0);
|
|
- }
|
|
-
|
|
/**
|
|
* Positions the composition view on top of the cursor and the textarea just below it (so the
|
|
* IME helper dialog is positioned correctly).
|
|
@@ -260,6 +250,7 @@ export class CompositionHelper {
|
|
this._compositionView.style.lineHeight = cellHeight + 'px';
|
|
this._compositionView.style.fontFamily = this._optionsService.rawOptions.fontFamily;
|
|
this._compositionView.style.fontSize = this._optionsService.rawOptions.fontSize + 'px';
|
|
+ this._alignPreeditToGrid(this._renderService.dimensions.css.cell.width);
|
|
// Limit the composition view width to the space between the cursor and
|
|
// the terminal's right edge, preventing it from overflowing the terminal.
|
|
const maxWidth = this._bufferService.cols * this._renderService.dimensions.css.cell.width - cursorLeft;
|
|
@@ -281,4 +272,39 @@ export class CompositionHelper {
|
|
setTimeout(() => this.updateCompositionElements(true), 0);
|
|
}
|
|
}
|
|
+
|
|
+ /**
|
|
+ * The overlay is laid out as plain text, so its extent is whatever advance the font
|
|
+ * gives the preedit. For Hangul and CJK that is well under the cells the same text
|
|
+ * occupies once committed (measured on macOS/SF Mono: 0.70 for Hangul, 0.81 for CJK,
|
|
+ * against 1.00 for Latin), and the shortfall accumulates across the composition.
|
|
+ * Spread it as letter-spacing, which is how DomRendererRowFactory lands committed
|
|
+ * glyphs on the cell grid, so the preedit covers the cells it is about to become.
|
|
+ */
|
|
+ private _alignPreeditToGrid(cellWidth: number): void {
|
|
+ const text = this._compositionView.textContent ?? '';
|
|
+ // Runs on every render frame while composing; the measurement below forces layout.
|
|
+ const key = `${cellWidth} |