Skip to content

fix: IME Shift+punctuation requires double-press on macOS - #983

Open
lewisay wants to merge 2 commits into
crynta:mainfrom
lewisay:fix/ime-shift-punctuation
Open

fix: IME Shift+punctuation requires double-press on macOS#983
lewisay wants to merge 2 commits into
crynta:mainfrom
lewisay:fix/ime-shift-punctuation

Conversation

@lewisay

@lewisay lewisay commented Jul 10, 2026

Copy link
Copy Markdown

Problem

On macOS with Chinese IME active, Shift+digit keys (producing @ # ¥ etc.) require pressing twice — the first press is silently consumed.

Root Cause

xterm.js attachCustomKeyEventHandler returned false for all keyCode=229 events. This skips xterm's _keyDown entirely, falling through to CompositionHelper._handleAnyTextareaChanges() which uses a textarea value-diff to detect input. However, the first Shift press triggers an IME mode-switch on macOS that doesn't update the textarea value, so the diff is empty and the character is lost.

Fix

When keyCode=229 but isComposing=false and event.key is a single printable character, write it directly to the PTY via writeToPty(), bypassing the textarea-diff path entirely.

Also adds an IME guard to the global shortcut listener to prevent interference during composition.

Testing

  1. Switch to Chinese IME on macOS
  2. Open a terminal tab
  3. Press Shift+2 (@), Shift+3 (#), Shift+4 (¥)
  4. Verify each character appears on the first press
  5. Verify normal Chinese input (pinyin → hanzi) still works
  6. Verify global shortcuts (Cmd+T, Cmd+P, etc.) still work

Summary by CodeRabbit

  • Bug Fixes
    • Improved shortcut handling during IME composition so composing text no longer triggers application shortcuts or unwanted browser behavior.
    • Improved terminal text input for Chromium IME sessions, allowing printable characters to be entered correctly while preserving modifier-key behavior.

@lewisay
lewisay requested a review from crynta as a code owner July 10, 2026 02:58
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

IME keyboard handling

Layer / File(s) Summary
Global shortcut composition filter
src/modules/shortcuts/lib/useGlobalShortcuts.ts
Global shortcut matching now skips active IME composition and Chromium’s keyCode === 229 events.
Terminal Process key forwarding
src/modules/terminal/lib/rendererPool.ts
Eligible printable, unmodified Process-key characters are written directly to the PTY; composition and other Process-key cases remain suppressed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits with a clear fix scope and accurately reflects the IME-related changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/modules/terminal/lib/rendererPool.ts`:
- Around line 255-264: Restrict the keyCode 229 IME fallback in the renderer’s
key handler to macOS by adding the existing IS_MAC guard to the condition before
resolving the leaf and forwarding event.key; preserve the current character and
modifier checks for the macOS path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 43d21249-0f96-4541-803c-dc5b7c32fea8

📥 Commits

Reviewing files that changed from the base of the PR and between 85a5653 and ff9c089.

📒 Files selected for processing (2)
  • src/modules/shortcuts/lib/useGlobalShortcuts.ts
  • src/modules/terminal/lib/rendererPool.ts

Comment on lines +255 to +264
if (event.keyCode === 229 && event.type === "keydown") {
const leafId = slot.currentLeafId;
const bridge = leafId !== null ? adapter?.resolveLeaf(leafId) : null;
if (bridge && event.key.length === 1 && !event.ctrlKey && !event.metaKey && !event.altKey) {
bridge.writeToPty(event.key);
event.preventDefault();
return false;
}
return false;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Chrome keydown keyCode 229 preventDefault effect on IME text insertion Windows Linux

💡 Result:

The keyCode 229 is the Windows Virtual-Key code (VK_PROCESSKEY) used by browsers to indicate that an Input Method Editor (IME) is processing a keystroke [1][2][3]. When a user types using an IME, the browser fires keydown events with this keyCode to signal that the IME has intercepted the key event [1][4]. Using event.preventDefault on a keydown event with keyCode 229 does not reliably prevent text insertion in Chrome on Windows or Linux [5][6][2]. Because the IME has already processed the keystroke before or simultaneously with the browser's event handling, the browser's attempt to cancel the default action typically fails to override the IME's operation [3][7]. Consequently, text will often still be committed to the input field regardless of the preventDefault call [6][2][7]. To manage text input effectively while an IME is active, you should avoid relying on keydown/keyup events with keyCode 229 [2][4]. Instead, use the modern Composition Events API: 1. compositionstart: Fired when an IME composition session begins. 2. compositionupdate: Fired when the composition value changes. 3. compositionend: Fired when the composition session is finished and the text is committed [7]. For scenarios where you need to restrict or sanitize user input (e.g., allowing only numeric characters), combining these composition events with the input event is the recommended approach, as the input event captures the final committed text [6][4]. Attempting to filter text via keydown is generally considered an anti-pattern when IME support is required [2][4].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="src/modules/terminal/lib/rendererPool.ts"

echo "## Around the reviewed block"
sed -n '220,310p' "$file" | cat -n

echo
echo "## IS_MAC references in file"
rg -n "IS_MAC|keyCode === 229|composition|preventDefault" "$file"

echo
echo "## File outline"
ast-grep outline "$file" --view expanded

Repository: crynta/terax-ai

Length of output: 8848


Gate the 229 IME fallback to macOS src/modules/terminal/lib/rendererPool.ts:255-264

On Windows/Linux, keyCode === 229 also routes through IME processing, and forwarding event.key here can double-insert characters outside the macOS Chinese IME case. IS_MAC is already in scope, so scope this handler to macOS.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/terminal/lib/rendererPool.ts` around lines 255 - 264, Restrict
the keyCode 229 IME fallback in the renderer’s key handler to macOS by adding
the existing IS_MAC guard to the condition before resolving the leaf and
forwarding event.key; preserve the current character and modifier checks for the
macOS path.

Source: Path instructions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant