-
Notifications
You must be signed in to change notification settings - Fork 77
fix: Korean/CJK IME composition events not captured #120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f572fa1
3180b52
efeacec
3db9771
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -195,6 +195,8 @@ export class InputHandler { | |
| private mousemoveListener: ((e: MouseEvent) => void) | null = null; | ||
| private wheelListener: ((e: WheelEvent) => void) | null = null; | ||
| private isComposing = false; | ||
| private compositionJustEnded = false; // Block keydown briefly after composition ends | ||
| private pendingKeyAfterComposition: string | null = null; // Key to output after composition | ||
| private isDisposed = false; | ||
| private mouseButtonsPressed = 0; // Track which buttons are pressed for motion reporting | ||
| private lastKeyDownData: string | null = null; | ||
|
|
@@ -288,14 +290,19 @@ export class InputHandler { | |
| this.inputElement.addEventListener('beforeinput', this.beforeInputListener); | ||
| } | ||
|
|
||
| // Attach composition events to inputElement (textarea) if available. | ||
| // IME composition events fire on the focused element, and when using a hidden | ||
| // textarea for input (as ghostty-web does), the textarea receives focus, | ||
| // not the container. This fixes Korean/Chinese/Japanese IME input. | ||
| const compositionTarget = this.inputElement || this.container; | ||
| this.compositionStartListener = this.handleCompositionStart.bind(this); | ||
| this.container.addEventListener('compositionstart', this.compositionStartListener); | ||
| compositionTarget.addEventListener('compositionstart', this.compositionStartListener); | ||
|
|
||
| this.compositionUpdateListener = this.handleCompositionUpdate.bind(this); | ||
| this.container.addEventListener('compositionupdate', this.compositionUpdateListener); | ||
| compositionTarget.addEventListener('compositionupdate', this.compositionUpdateListener); | ||
|
|
||
| this.compositionEndListener = this.handleCompositionEnd.bind(this); | ||
| this.container.addEventListener('compositionend', this.compositionEndListener); | ||
| compositionTarget.addEventListener('compositionend', this.compositionEndListener); | ||
|
|
||
| // Mouse event listeners (for terminal mouse tracking) | ||
| this.mousedownListener = this.handleMouseDown.bind(this); | ||
|
|
@@ -365,7 +372,23 @@ export class InputHandler { | |
|
|
||
| // Ignore keydown events during composition | ||
| // Note: Some browsers send keyCode 229 for all keys during composition | ||
| if (this.isComposing || event.isComposing || event.keyCode === 229) { | ||
| if (event.isComposing || event.keyCode === 229) { | ||
| return; | ||
| } | ||
|
|
||
| // If we're still in composition (our flag) but browser says composition ended, | ||
| // this is the key that ended the composition (space, period, etc.). | ||
| // Queue it to be processed after compositionend to maintain correct order. | ||
| if (this.isComposing) { | ||
| // Store the key to be processed after composition ends | ||
| this.pendingKeyAfterComposition = event.key; | ||
| event.preventDefault(); | ||
| return; | ||
| } | ||
|
|
||
| // Block the key that triggered composition end if we just processed a pending key | ||
| if (this.compositionJustEnded) { | ||
| this.compositionJustEnded = false; | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -689,13 +712,31 @@ export class InputHandler { | |
| if (data && data.length > 0) { | ||
| if (this.shouldIgnoreCompositionEnd(data)) { | ||
| this.cleanupCompositionTextNodes(); | ||
| // Still process pending key even if composition data is ignored | ||
| this.processPendingKeyAfterComposition(); | ||
| return; | ||
| } | ||
| this.onDataCallback(data); | ||
| this.recordCompositionData(data); | ||
| } | ||
|
|
||
| this.cleanupCompositionTextNodes(); | ||
|
|
||
| // Process the key that ended composition (space, period, etc.) | ||
| // This ensures correct order: composed text first, then the terminating key | ||
| this.processPendingKeyAfterComposition(); | ||
| } | ||
|
|
||
| /** | ||
| * Process the pending key that was queued during composition | ||
| */ | ||
| private processPendingKeyAfterComposition(): void { | ||
| if (this.pendingKeyAfterComposition) { | ||
| const key = this.pendingKeyAfterComposition; | ||
| this.pendingKeyAfterComposition = null; | ||
| // Output the key that ended composition | ||
| this.onDataCallback(key); | ||
|
Comment on lines
+735
to
+738
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1059,18 +1100,20 @@ export class InputHandler { | |
| this.beforeInputListener = null; | ||
| } | ||
|
|
||
| // Remove composition listeners from the same element they were attached to | ||
| const compositionTarget = this.inputElement || this.container; | ||
| if (this.compositionStartListener) { | ||
| this.container.removeEventListener('compositionstart', this.compositionStartListener); | ||
| compositionTarget.removeEventListener('compositionstart', this.compositionStartListener); | ||
| this.compositionStartListener = null; | ||
| } | ||
|
|
||
| if (this.compositionUpdateListener) { | ||
| this.container.removeEventListener('compositionupdate', this.compositionUpdateListener); | ||
| compositionTarget.removeEventListener('compositionupdate', this.compositionUpdateListener); | ||
| this.compositionUpdateListener = null; | ||
| } | ||
|
|
||
| if (this.compositionEndListener) { | ||
| this.container.removeEventListener('compositionend', this.compositionEndListener); | ||
| compositionTarget.removeEventListener('compositionend', this.compositionEndListener); | ||
| this.compositionEndListener = null; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,7 @@ export class Terminal implements ITerminalCore { | |
| private inputHandler?: InputHandler; | ||
| private selectionManager?: SelectionManager; | ||
| private canvas?: HTMLCanvasElement; | ||
| private compositionPreview?: HTMLDivElement; | ||
|
|
||
| // Link detection system | ||
| private linkDetector?: LinkDetector; | ||
|
|
@@ -348,20 +349,15 @@ export class Terminal implements ITerminalCore { | |
| this.isOpen = true; | ||
|
|
||
| try { | ||
| // Make parent focusable if it isn't already | ||
| if (!parent.hasAttribute('tabindex')) { | ||
| parent.setAttribute('tabindex', '0'); | ||
| } | ||
| // Set tabindex="-1" on parent so it's not focusable via click/tab. | ||
| // We want all focus to go to the hidden textarea for proper IME handling. | ||
| // The textarea will handle keyboard input and composition events. | ||
| parent.setAttribute('tabindex', '-1'); | ||
|
|
||
| // Mark as contenteditable so browser extensions (Vimium, etc.) recognize | ||
| // this as an input element and don't intercept keyboard events. | ||
| parent.setAttribute('contenteditable', 'true'); | ||
| // Prevent actual content editing - we handle input ourselves | ||
| parent.addEventListener('beforeinput', (e) => { | ||
| if (e.target === parent) { | ||
| e.preventDefault(); | ||
| } | ||
| }); | ||
| // Note: We intentionally do NOT set contenteditable on the parent container. | ||
| // Setting contenteditable causes IME (Korean, Chinese, Japanese) input to be | ||
| // inserted directly into the container as text nodes, bypassing our textarea. | ||
| // Instead, we use the hidden textarea for all keyboard/IME input. | ||
|
|
||
| // Add accessibility attributes for screen readers and extensions | ||
| parent.setAttribute('role', 'textbox'); | ||
|
|
@@ -400,9 +396,35 @@ export class Terminal implements ITerminalCore { | |
| this.textarea.style.resize = 'none'; | ||
| parent.appendChild(this.textarea); | ||
|
|
||
| // Create composition preview element for IME input (Korean, Chinese, Japanese) | ||
| this.compositionPreview = document.createElement('div'); | ||
| this.compositionPreview.style.position = 'absolute'; | ||
| this.compositionPreview.style.top = '4px'; | ||
| this.compositionPreview.style.right = '4px'; | ||
| this.compositionPreview.style.padding = '2px 8px'; | ||
| this.compositionPreview.style.backgroundColor = 'rgba(0, 0, 0, 0.7)'; | ||
| this.compositionPreview.style.color = '#ffcc00'; | ||
| this.compositionPreview.style.fontFamily = 'monospace'; | ||
| this.compositionPreview.style.fontSize = '12px'; | ||
| this.compositionPreview.style.borderRadius = '3px'; | ||
| this.compositionPreview.style.display = 'none'; | ||
| this.compositionPreview.style.zIndex = '1000'; | ||
| parent.appendChild(this.compositionPreview); | ||
|
|
||
| // Listen to composition events for preview | ||
| this.textarea.addEventListener('compositionupdate', (e: CompositionEvent) => { | ||
| if (e.data) { | ||
| this.compositionPreview!.textContent = `조합중: ${e.data}`; | ||
| this.compositionPreview!.style.display = 'block'; | ||
| } | ||
| }); | ||
| this.textarea.addEventListener('compositionend', () => { | ||
| this.compositionPreview!.style.display = 'none'; | ||
| }); | ||
|
|
||
| // Focus textarea on interaction - preventDefault before focus | ||
| const textarea = this.textarea; | ||
| // Desktop: mousedown | ||
| // Desktop: mousedown on canvas | ||
| this.canvas.addEventListener('mousedown', (ev) => { | ||
| ev.preventDefault(); | ||
| textarea.focus(); | ||
|
|
@@ -412,6 +434,17 @@ export class Terminal implements ITerminalCore { | |
| ev.preventDefault(); | ||
| textarea.focus(); | ||
| }); | ||
| // Redirect focus from parent container to textarea | ||
| // This ensures IME composition events always go to the textarea | ||
| parent.addEventListener('mousedown', (ev) => { | ||
| if (ev.target === parent) { | ||
| ev.preventDefault(); | ||
| textarea.focus(); | ||
| } | ||
| }); | ||
| parent.addEventListener('focus', () => { | ||
| textarea.focus(); | ||
| }); | ||
|
|
||
| // Create renderer | ||
| this.renderer = new CanvasRenderer(this.canvas, { | ||
|
|
@@ -717,15 +750,21 @@ export class Terminal implements ITerminalCore { | |
| * Focus terminal input | ||
| */ | ||
| focus(): void { | ||
| if (this.isOpen && this.element) { | ||
| // Focus immediately for immediate keyboard/wheel event handling | ||
| this.element.focus(); | ||
|
|
||
| // Also schedule a delayed focus as backup to ensure it sticks | ||
| // (some browsers may need this if DOM isn't fully settled) | ||
| setTimeout(() => { | ||
| this.element?.focus(); | ||
| }, 0); | ||
| if (this.isOpen) { | ||
| // Focus the textarea for keyboard/IME input. | ||
| // The textarea is the actual input element that receives keyboard events | ||
| // and IME composition events. Focusing the container doesn't work for IME | ||
| // because composition events fire on the focused element. | ||
| const target = this.textarea || this.element; | ||
|
Comment on lines
+754
to
+758
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new focus path now targets the hidden textarea when it exists, but Useful? React with 👍 / 👎. |
||
| if (target) { | ||
| target.focus(); | ||
|
Comment on lines
+758
to
+760
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
|
|
||
| // Also schedule a delayed focus as backup to ensure it sticks | ||
| // (some browsers may need this if DOM isn't fully settled) | ||
| setTimeout(() => { | ||
| target?.focus(); | ||
| }, 0); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1195,6 +1234,12 @@ export class Terminal implements ITerminalCore { | |
| this.textarea = undefined; | ||
| } | ||
|
|
||
| // Remove composition preview from DOM | ||
| if (this.compositionPreview && this.compositionPreview.parentNode) { | ||
| this.compositionPreview.parentNode.removeChild(this.compositionPreview); | ||
| this.compositionPreview = undefined; | ||
| } | ||
|
|
||
| // Remove event listeners | ||
| if (this.element) { | ||
| this.element.removeEventListener('wheel', this.handleWheel); | ||
|
|
@@ -1203,8 +1248,7 @@ export class Terminal implements ITerminalCore { | |
| this.element.removeEventListener('mouseleave', this.handleMouseLeave); | ||
| this.element.removeEventListener('click', this.handleClick); | ||
|
|
||
| // Remove contenteditable and accessibility attributes added in open() | ||
| this.element.removeAttribute('contenteditable'); | ||
| // Remove accessibility attributes added in open() | ||
| this.element.removeAttribute('role'); | ||
| this.element.removeAttribute('aria-label'); | ||
| this.element.removeAttribute('aria-multiline'); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new
compositionJustEndedguard can never trigger because this patch only reads/resets the flag and never sets it totrue. On browsers that emit a follow-upkeydownfor the same composition-ending key, that duplicate event is not suppressed, so the terminating character can be inserted twice.Useful? React with 👍 / 👎.