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}${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 { 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); } }