mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
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.
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -47,19 +47,43 @@ index 497afcf535f3eaca00889525a77e15eb633ccd96..e3cad77734795f6cf34bb7120264fe81
|
||||
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..d5c532339da18e0fecf6193ad579839bae269b83 100644
|
||||
index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..67188a5c3310ac694bde8112f5387eb712227358 100644
|
||||
--- a/src/browser/input/CompositionHelper.ts
|
||||
+++ b/src/browser/input/CompositionHelper.ts
|
||||
@@ -5,7 +5,6 @@
|
||||
@@ -4,8 +4,7 @@
|
||||
*/
|
||||
|
||||
import { IRenderService } from '../services/Services';
|
||||
import { IBufferService, ICoreService, IOptionsService } from '../../common/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,9 @@ export class CompositionHelper {
|
||||
@@ -42,15 +41,12 @@ export class CompositionHelper {
|
||||
*/
|
||||
private _isSendingComposition: boolean;
|
||||
|
||||
@@ -67,18 +91,28 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
- * Data already sent due to keydown event.
|
||||
- */
|
||||
- private _dataAlreadySent: string;
|
||||
-
|
||||
- /**
|
||||
- * The pending textarea change timer, if any.
|
||||
- */
|
||||
- private _textareaChangeTimer?: number;
|
||||
+ 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,
|
||||
@@ -64,7 +57,6 @@ export class CompositionHelper {
|
||||
@@ -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 = '';
|
||||
@@ -86,7 +120,7 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -80,10 +72,27 @@ export class CompositionHelper {
|
||||
@@ -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 = '';
|
||||
@@ -115,7 +149,7 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
/**
|
||||
* Handles the compositionupdate event, updating the composition view.
|
||||
* @param ev The event.
|
||||
@@ -129,9 +138,6 @@ export class CompositionHelper {
|
||||
@@ -129,9 +142,6 @@ export class CompositionHelper {
|
||||
}
|
||||
|
||||
if (ev.keyCode === 229) {
|
||||
@@ -125,7 +159,7 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -153,7 +159,11 @@ export class CompositionHelper {
|
||||
@@ -153,7 +163,11 @@ export class CompositionHelper {
|
||||
if (!waitForPropagation) {
|
||||
// Cancel any delayed composition send requests and send the input immediately.
|
||||
this._isSendingComposition = false;
|
||||
@@ -138,7 +172,7 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
this._coreService.triggerDataEvent(input, true);
|
||||
} else {
|
||||
// Make a deep copy of the composition position here as a new compositionstart event may
|
||||
@@ -163,6 +173,7 @@ export class CompositionHelper {
|
||||
@@ -163,6 +177,7 @@ export class CompositionHelper {
|
||||
end: this._compositionPosition.end
|
||||
};
|
||||
const currentCompositionSuffix = this._compositionSuffix;
|
||||
@@ -146,7 +180,7 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
|
||||
// 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 +186,10 @@ export class CompositionHelper {
|
||||
@@ -175,12 +190,10 @@ export class CompositionHelper {
|
||||
this._isSendingComposition = true;
|
||||
setTimeout(() => {
|
||||
// Ensure that the input has not already been sent
|
||||
@@ -162,7 +196,7 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
if (this._isComposing) {
|
||||
// Use the start position of the new composition to get the string
|
||||
// if a new composition has started.
|
||||
@@ -195,47 +204,22 @@ export class CompositionHelper {
|
||||
@@ -195,47 +208,22 @@ export class CompositionHelper {
|
||||
: value.length;
|
||||
input = value.substring(currentCompositionPosition.start, Math.max(currentCompositionPosition.start, valueEnd));
|
||||
}
|
||||
@@ -221,6 +255,54 @@ index c9ec396ab66cb966d49aa63bed09cdf9cd6c4246..d5c532339da18e0fecf6193ad579839b
|
||||
/**
|
||||
* 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} | ||||