From 478dc113164b63c31e13834656a1fa734e3fdd5d Mon Sep 17 00:00:00 2001 From: Leask Wong Date: Thu, 4 Dec 2025 13:35:09 -0500 Subject: [PATCH 1/3] working on IME Composition, step 1 --- lib/input-handler.test.ts | 104 ++++++++++++++++++++++++++++++++++---- lib/input-handler.ts | 83 ++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 9 deletions(-) diff --git a/lib/input-handler.test.ts b/lib/input-handler.test.ts index e7c2af9..567b5bc 100644 --- a/lib/input-handler.test.ts +++ b/lib/input-handler.test.ts @@ -49,8 +49,8 @@ function createKeyEvent( shiftKey: modifiers.shift ?? false, metaKey: modifiers.meta ?? false, repeat: false, - preventDefault: mock(() => {}), - stopPropagation: mock(() => {}), + preventDefault: mock(() => { }), + stopPropagation: mock(() => { }), }; } @@ -66,17 +66,35 @@ function createClipboardEvent(text: string | null): MockClipboardEvent { clipboardData: text !== null ? { - getData: (format: string) => data.get(format) || '', - setData: (format: string, value: string) => { - data.set(format, value); - }, - } + getData: (format: string) => data.get(format) || '', + setData: (format: string, value: string) => { + data.set(format, value); + }, + } : null, - preventDefault: mock(() => {}), - stopPropagation: mock(() => {}), + preventDefault: mock(() => { }), + stopPropagation: mock(() => { }), }; } +interface MockCompositionEvent { + type: string; + data: string | null; + preventDefault: () => void; + stopPropagation: () => void; +} +// Helper to create mock composition event +function createCompositionEvent( + type: 'compositionstart' | 'compositionupdate' | 'compositionend', + data: string | null +): MockCompositionEvent { + return { + type, + data, + preventDefault: mock(() => { }), + stopPropagation: mock(() => { }), + }; +} // Helper to create mock container function createMockContainer(): MockHTMLElement & { _listeners: Map void)[]>; @@ -269,6 +287,74 @@ describe('InputHandler', () => { }); }); + describe('IME Composition', () => { + test('handles composition sequence', () => { + const handler = new InputHandler( + ghostty, + container as any, + (data) => dataReceived.push(data), + () => { + bellCalled = true; + } + ); + + // Start composition + const startEvent = createCompositionEvent('compositionstart', ''); + container.dispatchEvent(startEvent); + + // Update composition (typing) + const updateEvent1 = createCompositionEvent('compositionupdate', 'n'); + container.dispatchEvent(updateEvent1); + + // Keydown events during composition should be ignored + const keyEvent1 = createKeyEvent('KeyN', 'n'); + Object.defineProperty(keyEvent1, 'isComposing', { value: true }); + simulateKey(container, keyEvent1); + + // Update composition (more typing) + const updateEvent2 = createCompositionEvent('compositionupdate', 'ni'); + container.dispatchEvent(updateEvent2); + + // End composition (commit) + const endEvent = createCompositionEvent('compositionend', '你好'); + container.dispatchEvent(endEvent); + + // Should only receive the final committed text + expect(dataReceived).toEqual(['你好']); + }); + + test('ignores keydown during composition', () => { + const handler = new InputHandler( + ghostty, + container as any, + (data) => dataReceived.push(data), + () => { + bellCalled = true; + } + ); + + // Start composition + container.dispatchEvent(createCompositionEvent('compositionstart', '')); + + // Simulate keydown with isComposing=true + const keyEvent = createKeyEvent('KeyA', 'a'); + Object.defineProperty(keyEvent, 'isComposing', { value: true }); + simulateKey(container, keyEvent); + + // Simulate keydown with keyCode 229 + const keyEvent229 = createKeyEvent('KeyB', 'b'); + Object.defineProperty(keyEvent229, 'keyCode', { value: 229 }); + simulateKey(container, keyEvent229); + + // Should not receive any data + expect(dataReceived.length).toBe(0); + + // End composition + container.dispatchEvent(createCompositionEvent('compositionend', 'a')); + expect(dataReceived).toEqual(['a']); + }); + }); + describe('Control Characters', () => { test('encodes Ctrl+A', () => { const handler = new InputHandler( diff --git a/lib/input-handler.ts b/lib/input-handler.ts index 711bb61..4b94aeb 100644 --- a/lib/input-handler.ts +++ b/lib/input-handler.ts @@ -169,6 +169,10 @@ export class InputHandler { private keydownListener: ((e: KeyboardEvent) => void) | null = null; private keypressListener: ((e: KeyboardEvent) => void) | null = null; private pasteListener: ((e: ClipboardEvent) => void) | null = null; + private compositionStartListener: ((e: CompositionEvent) => void) | null = null; + private compositionUpdateListener: ((e: CompositionEvent) => void) | null = null; + private compositionEndListener: ((e: CompositionEvent) => void) | null = null; + private isComposing = false; private isDisposed = false; /** @@ -233,6 +237,24 @@ export class InputHandler { this.pasteListener = this.handlePaste.bind(this); this.container.addEventListener('paste', this.pasteListener); + + this.compositionStartListener = this.handleCompositionStart.bind(this); + this.container.addEventListener( + 'compositionstart', + this.compositionStartListener + ); + + this.compositionUpdateListener = this.handleCompositionUpdate.bind(this); + this.container.addEventListener( + 'compositionupdate', + this.compositionUpdateListener + ); + + this.compositionEndListener = this.handleCompositionEnd.bind(this); + this.container.addEventListener( + 'compositionend', + this.compositionEndListener + ); } /** @@ -287,6 +309,12 @@ export class InputHandler { private handleKeyDown(event: KeyboardEvent): void { if (this.isDisposed) return; + // Ignore keydown events during composition + // Note: Some browsers send keyCode 229 for all keys during composition + if (this.isComposing || event.isComposing || event.keyCode === 229) { + return; + } + // Emit onKey event first (before any processing) if (this.onKeyCallback) { this.onKeyCallback({ key: event.key, domEvent: event }); @@ -493,6 +521,37 @@ export class InputHandler { this.onDataCallback(text); } + /** + * Handle compositionstart event + */ + private handleCompositionStart(_event: CompositionEvent): void { + if (this.isDisposed) return; + this.isComposing = true; + } + + /** + * Handle compositionupdate event + */ + private handleCompositionUpdate(_event: CompositionEvent): void { + if (this.isDisposed) return; + // We could track the current composition string here if we wanted to + // display it in a custom way, but for now we rely on the browser's + // input method editor UI. + } + + /** + * Handle compositionend event + */ + private handleCompositionEnd(event: CompositionEvent): void { + if (this.isDisposed) return; + this.isComposing = false; + + const data = event.data; + if (data && data.length > 0) { + this.onDataCallback(data); + } + } + /** * Dispose the InputHandler and remove event listeners */ @@ -514,6 +573,30 @@ export class InputHandler { this.pasteListener = null; } + if (this.compositionStartListener) { + this.container.removeEventListener( + 'compositionstart', + this.compositionStartListener + ); + this.compositionStartListener = null; + } + + if (this.compositionUpdateListener) { + this.container.removeEventListener( + 'compositionupdate', + this.compositionUpdateListener + ); + this.compositionUpdateListener = null; + } + + if (this.compositionEndListener) { + this.container.removeEventListener( + 'compositionend', + this.compositionEndListener + ); + this.compositionEndListener = null; + } + this.isDisposed = true; } From ecb0fe46113d54c569c7efcf48463b67dd23adde Mon Sep 17 00:00:00 2001 From: Leask Wong Date: Thu, 4 Dec 2025 13:41:39 -0500 Subject: [PATCH 2/3] fixed double line issue --- lib/input-handler.test.ts | 46 +++++++++++++++++++++++++++++++++++++++ lib/input-handler.ts | 13 +++++++++++ 2 files changed, 59 insertions(+) diff --git a/lib/input-handler.test.ts b/lib/input-handler.test.ts index 567b5bc..ca54292 100644 --- a/lib/input-handler.test.ts +++ b/lib/input-handler.test.ts @@ -33,6 +33,9 @@ interface MockClipboardEvent { interface MockHTMLElement { addEventListener: (event: string, handler: (e: any) => void) => void; removeEventListener: (event: string, handler: (e: any) => void) => void; + childNodes: Node[]; + removeChild: (node: Node) => Node; + appendChild: (node: Node) => Node; } // Helper to create mock keyboard event @@ -125,6 +128,19 @@ function createMockContainer(): MockHTMLElement & { handler(event); } }, + // Mock childNodes and removeChild for text node cleanup test + childNodes: [] as Node[], + removeChild(node: Node) { + const index = this.childNodes.indexOf(node); + if (index >= 0) { + this.childNodes.splice(index, 1); + } + return node; + }, + appendChild(node: Node) { + this.childNodes.push(node); + return node; + } }; } @@ -353,6 +369,36 @@ describe('InputHandler', () => { container.dispatchEvent(createCompositionEvent('compositionend', 'a')); expect(dataReceived).toEqual(['a']); }); + + test('cleans up text nodes in container after composition', () => { + const handler = new InputHandler( + ghostty, + container as any, + (data) => dataReceived.push(data), + () => { + bellCalled = true; + } + ); + + // Simulate browser inserting text node during composition + const textNode = { nodeType: 3, textContent: '你好' } as Node; + container.appendChild(textNode); + + // Also add a non-text node (e.g. canvas) to ensure it's not removed + const elementNode = { nodeType: 1, nodeName: 'CANVAS' } as Node; + container.appendChild(elementNode); + + expect(container.childNodes.length).toBe(2); + + // End composition + const endEvent = createCompositionEvent('compositionend', '你好'); + container.dispatchEvent(endEvent); + + // Should have removed the text node but kept the element node + expect(container.childNodes.length).toBe(1); + expect(container.childNodes[0]).toBe(elementNode); + expect(dataReceived).toEqual(['你好']); + }); }); describe('Control Characters', () => { diff --git a/lib/input-handler.ts b/lib/input-handler.ts index 4b94aeb..12e4806 100644 --- a/lib/input-handler.ts +++ b/lib/input-handler.ts @@ -550,6 +550,19 @@ export class InputHandler { if (data && data.length > 0) { this.onDataCallback(data); } + + // Cleanup text nodes in container (fix for duplicate text display) + // When the container is contenteditable, the browser might insert text nodes + // upon composition end. We need to remove them to prevent duplicate display. + if (this.container && this.container.childNodes) { + for (let i = this.container.childNodes.length - 1; i >= 0; i--) { + const node = this.container.childNodes[i]; + // Node.TEXT_NODE === 3 + if (node.nodeType === 3) { + this.container.removeChild(node); + } + } + } } /** From ebbe2a7b189c9d6fc8e07f8ca100d56f6fba9551 Mon Sep 17 00:00:00 2001 From: Leask Wong Date: Thu, 4 Dec 2025 13:55:32 -0500 Subject: [PATCH 3/3] reformat with fmt --- lib/input-handler.test.ts | 24 ++++++++++++------------ lib/input-handler.ts | 30 ++++++------------------------ 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/lib/input-handler.test.ts b/lib/input-handler.test.ts index ca54292..c7e2f58 100644 --- a/lib/input-handler.test.ts +++ b/lib/input-handler.test.ts @@ -52,8 +52,8 @@ function createKeyEvent( shiftKey: modifiers.shift ?? false, metaKey: modifiers.meta ?? false, repeat: false, - preventDefault: mock(() => { }), - stopPropagation: mock(() => { }), + preventDefault: mock(() => {}), + stopPropagation: mock(() => {}), }; } @@ -69,14 +69,14 @@ function createClipboardEvent(text: string | null): MockClipboardEvent { clipboardData: text !== null ? { - getData: (format: string) => data.get(format) || '', - setData: (format: string, value: string) => { - data.set(format, value); - }, - } + getData: (format: string) => data.get(format) || '', + setData: (format: string, value: string) => { + data.set(format, value); + }, + } : null, - preventDefault: mock(() => { }), - stopPropagation: mock(() => { }), + preventDefault: mock(() => {}), + stopPropagation: mock(() => {}), }; } interface MockCompositionEvent { @@ -94,8 +94,8 @@ function createCompositionEvent( return { type, data, - preventDefault: mock(() => { }), - stopPropagation: mock(() => { }), + preventDefault: mock(() => {}), + stopPropagation: mock(() => {}), }; } // Helper to create mock container @@ -140,7 +140,7 @@ function createMockContainer(): MockHTMLElement & { appendChild(node: Node) { this.childNodes.push(node); return node; - } + }, }; } diff --git a/lib/input-handler.ts b/lib/input-handler.ts index 12e4806..6e3bd5f 100644 --- a/lib/input-handler.ts +++ b/lib/input-handler.ts @@ -239,22 +239,13 @@ export class InputHandler { this.container.addEventListener('paste', this.pasteListener); this.compositionStartListener = this.handleCompositionStart.bind(this); - this.container.addEventListener( - 'compositionstart', - this.compositionStartListener - ); + this.container.addEventListener('compositionstart', this.compositionStartListener); this.compositionUpdateListener = this.handleCompositionUpdate.bind(this); - this.container.addEventListener( - 'compositionupdate', - this.compositionUpdateListener - ); + this.container.addEventListener('compositionupdate', this.compositionUpdateListener); this.compositionEndListener = this.handleCompositionEnd.bind(this); - this.container.addEventListener( - 'compositionend', - this.compositionEndListener - ); + this.container.addEventListener('compositionend', this.compositionEndListener); } /** @@ -587,26 +578,17 @@ export class InputHandler { } if (this.compositionStartListener) { - this.container.removeEventListener( - 'compositionstart', - this.compositionStartListener - ); + this.container.removeEventListener('compositionstart', this.compositionStartListener); this.compositionStartListener = null; } if (this.compositionUpdateListener) { - this.container.removeEventListener( - 'compositionupdate', - this.compositionUpdateListener - ); + this.container.removeEventListener('compositionupdate', this.compositionUpdateListener); this.compositionUpdateListener = null; } if (this.compositionEndListener) { - this.container.removeEventListener( - 'compositionend', - this.compositionEndListener - ); + this.container.removeEventListener('compositionend', this.compositionEndListener); this.compositionEndListener = null; }