Files
orca/config/patches/xterm-src/@xterm__xterm@6.1.0-beta.287.src.patch
T
Neil e04e0c88da fix(xterm): size the preedit overlay to the cells its text will occupy
updateCompositionElements computed the overlay's left edge from the grid
but never its width, so the preedit rendered at the font's natural advance
while the committed text takes two cells per wide glyph. Measured in
Chromium 150: 가나다라 drew 48.45px as a preedit and 69.20px once committed
— the same characters, same font, 30% narrower, and drifting further with
each syllable. Every macOS mono font carrying Hangul measured 0.49–0.72 of
two cells; never 1.0.

Deriving the width from wcwidth and the cell measure moves Korean, Japanese
and Chinese to 1.000 and leaves ASCII at 1.000, which it already was:

  한        12.125 -> 17.297   (17.30 expected)
  가나다라   48.453 -> 69.188   (69.20)
  안녕하세요 60.563 -> 86.500   (86.50)
  日本語     42.000 -> 51.906   (51.90)
  abcdefgh  69.234 -> 69.203   (69.20, unchanged)

Edited in config/patches/xterm-src/ and regenerated through the harness, so
the emitted patch and lockfile hash are derived rather than hand-written.

The unit test asserts the arithmetic, which is what CI can run. The pixel
consequence was measured on macOS with SF Mono in an Electron harness, not
on the Windows font stack STA-3232 reports from — so this demonstrates the
mechanism and does not stand as that row's platform evidence.
2026-08-06 11:30:19 -07:00

351 lines
16 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..67188a5c3310ac694bde8112f5387eb712227358 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.
@@ -129,9 +142,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 +163,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 +177,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 +190,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 +208,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 +248,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 +270,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}${text}`;
+ if (key === this._gridAdvanceKey) {
+ return;
+ }
+ this._gridAdvanceKey = key;
+ // Letter-spacing lands after each character that advances, so the LTR marks
+ // wrapping the preedit are not among the gaps the shortfall is divided over.
+ let advancing = 0;
+ for (const character of text) {
+ if (this._unicodeService.wcwidth(character.codePointAt(0)!) > 0) {
+ advancing++;
+ }
+ }
+ this._compositionView.style.letterSpacing = '';
+ if (advancing === 0) {
+ return;
+ }
+ // Measured with maxWidth cleared: a long preedit's natural advance exceeds it.
+ this._compositionView.style.maxWidth = '';
+ const naturalWidth = this._compositionView.getBoundingClientRect().width;
+ const gridWidth = this._unicodeService.getStringCellWidth(text) * cellWidth;
+ this._compositionView.style.letterSpacing = `${(gridWidth - naturalWidth) / advancing}px`;
+ }
}
diff --git a/src/common/SortedList.ts b/src/common/SortedList.ts
index 8a10076e3963e33b4a7d1e4602333eb3f4772dc9..df0761c35907ddc48eb102ba181b0dac8e61f00d 100644
--- a/src/common/SortedList.ts
+++ b/src/common/SortedList.ts
@@ -87,6 +87,24 @@ export class SortedList<T> {
if (key === undefined) {
return false;
}
+ if (this._deleteAtKey(value, key)) {
+ return true;
+ }
+ // A pending deletion whose key mutated after `delete()` (disposing a marker
+ // resets `line` to -1, and `line` is the sort key) leaves `_array` out of
+ // order, so the binary search above can miss a value that is present.
+ // Compacting those entries out restores the order; retry before reporting
+ // the value absent, else its `onDecorationRemoved` never fires and the
+ // decoration paints forever. Miss path only, so the common bulk delete
+ // keeps its O(log n) search and deferred-compaction batching.
+ if (this._deletedIndices.length === 0) {
+ return false;
+ }
+ this._flushCleanupDeleted();
+ return this._deleteAtKey(value, key);
+ }
+
+ private _deleteAtKey(value: T, key: number): boolean {
i = this._search(key);
if (i === -1) {
return false;
diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts
index bedfa6ba1fdd6fb24646f52167e4881dc03498be..5d87be2d5cc52cb4504fd383243c2de72290bd04 100644
--- a/src/common/TestUtils.test.ts
+++ b/src/common/TestUtils.test.ts
@@ -225,8 +225,10 @@ export class MockUnicodeService implements IUnicodeService {
}
return UnicodeService.createPropertyValue(0, width, shouldJoin);
}
+ // Reuses the real traversal against this mock's own provider; CompositionHelper
+ // sizes the preedit overlay with it, so throwing here would fail its tests.
public getStringCellWidth(s: string): number {
- throw new Error('Method not implemented.');
+ return UnicodeService.prototype.getStringCellWidth.call(this, s);
}
}