Skip to content

Normalize keyboard event for menu dispatch under non-Latin IME #2032

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.

Sign up for GitHub

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

Closed
anthhub wants to merge 5 commits into from
Closed

Normalize keyboard event for menu dispatch under non-Latin IME #2032

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49f98c6
Normalize keyboard event for menu dispatch under non-Latin IME
Mar 24, 2026
7d832ff
Fix Shift modifier loss and preserve original characters in IME norma…
Mar 24, 2026
60a3f38
Add Korean font mapping to CJK font fallback system
Mar 24, 2026
d2f1c55
Remove out-of-scope CJK font test changes (belong to #2037)
Mar 26, 2026
10088a0
Harden IME normalization: guard against empty strings and nil chars
Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion Sources/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12605,7 +12605,35 @@ private extension NSWindow {
if firstResponderGhosttyView != nil,
shouldRouteCommandEquivalentDirectlyToMainMenu(event),
let mainMenu = NSApp.mainMenu {
let consumedByMenu = mainMenu.performKeyEquivalent(with: event)
// Normalize event characters for non-Latin IME (Korean, Russian, etc.)
// so NSMenu.performKeyEquivalent can match ASCII-based shortcuts.
// When IME is active, event.charactersIgnoringModifiers may return
// non-ASCII characters (e.g. "ㅅ" instead of "t"), causing menu
// shortcut matching to fail.
var menuEvent = event
let rawChars = event.charactersIgnoringModifiers ?? ""
if (rawChars.isEmpty || !rawChars.allSatisfy(\.isASCII)),
let normalized = KeyboardLayout.character(forKeyCode: event.keyCode, modifierFlags: event.modifierFlags),
Comment on lines +12615 to +12616

@coderabbitai coderabbitai Bot Mar 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

According to Apple AppKit documentation, does NSEvent.charactersIgnoringModifiers ignore Command, Option, and Control while preserving only Shift?

💡 Result:

Yes, according to the official Apple AppKit documentation, NSEvent.charactersIgnoringModifiers ignores all modifier keys except Shift, preserving the Shift-modified character while ignoring Command (⌘), Option (⌥/Alt), and Control. The documentation states: "The characters generated by a key event as if no modifier key (except for Shift) applies." Examples include Option-e returning "e" (ignoring Option), and it is used alongside modifierFlags to detect combinations like Alt-S by checking for "s" and NSAlternateKeyMask.

Citations:

Use Shift-only modifiers when rebuilding charactersIgnoringModifiers.

Line 12616 derives charactersIgnoringModifiers with the full event.modifierFlags. According to Apple AppKit documentation, NSEvent.charactersIgnoringModifiers ignores all modifier keys except Shift. Passing Command, Option, and Control flags to KeyboardLayout.character() will synthesize the wrong character for those key combinations under non-Latin input methods.

Suggested fix 
-            if (rawChars.isEmpty || !rawChars.allSatisfy(\.isASCII)),
-               let normalized = KeyboardLayout.character(forKeyCode: event.keyCode, modifierFlags: event.modifierFlags),
+            if (rawChars.isEmpty || !rawChars.allSatisfy(\.isASCII)),
+               let normalized = KeyboardLayout.character(
+                   forKeyCode: event.keyCode,
+                   modifierFlags: event.modifierFlags.intersection([.shift])
+               ),
                !normalized.isEmpty {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 12615 - 12616, The code currently
reconstructs charactersIgnoringModifiers using the full event.modifierFlags
which can include Command/Option/Control and thus produces incorrect characters
for non‑Latin input methods; update the call to
KeyboardLayout.character(forKeyCode:modifierFlags:) and any reconstruction of
charactersIgnoringModifiers to pass only the Shift flag from event.modifierFlags
(e.g., derive a shiftOnlyFlags value via
event.modifierFlags.intersection(.shift) and use that) so rawChars,
KeyboardLayout.character(forKeyCode:modifierFlags:), and the
charactersIgnoringModifiers logic all use shift-only modifiers.

!normalized.isEmpty {
menuEvent = NSEvent.keyEvent(
with: event.type,
location: event.locationInWindow,
modifierFlags: event.modifierFlags,
timestamp: event.timestamp,
windowNumber: event.windowNumber,
context: nil,
characters: event.characters ?? normalized,
charactersIgnoringModifiers: normalized,
isARepeat: event.isARepeat,
keyCode: event.keyCode
) ?? event
Comment on lines +12618 to +12629

@greptile-apps greptile-apps Bot Mar 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 characters field mirrors charactersIgnoringModifiers in synthetic event

Both characters and charactersIgnoringModifiers are set to the same lowercase ASCII string (normalized, e.g. "t"). For a real Cmd+T key press, characters typically carries the control-character form (e.g. "\u{14}") while charactersIgnoringModifiers is "t".

This is harmless for NSMenu.performKeyEquivalent, which matches solely on charactersIgnoringModifiers + modifierFlags. However, if any downstream menu-item action handler ever inspects event.characters (rare but possible), it would receive "t" instead of the expected control character.

A safer alternative is to preserve the original characters value and only override charactersIgnoringModifiers:

Suggested change
menuEvent = NSEvent.keyEvent(
with: event.type,
location: event.locationInWindow,
modifierFlags: event.modifierFlags,
timestamp: event.timestamp,
windowNumber: event.windowNumber,
context: nil,
characters: normalized,
charactersIgnoringModifiers: normalized,
isARepeat: event.isARepeat,
keyCode: event.keyCode
) ?? event
menuEvent = NSEvent.keyEvent(
with: event.type,
location: event.locationInWindow,
modifierFlags: event.modifierFlags,
timestamp: event.timestamp,
windowNumber: event.windowNumber,
context: nil,
characters: event.characters ?? normalized,
charactersIgnoringModifiers: normalized,
isARepeat: event.isARepeat,
keyCode: event.keyCode
) ?? event

Comment on lines +12613 to +12629

@coderabbitai coderabbitai Bot Mar 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Cover empty-string key equivalents in the new normalization path.

This branch only normalizes non-ASCII charactersIgnoringModifiers, but the same file already handles synthetic key-equivalent paths that can arrive with nil/"". It also accepts KeyboardLayout.character(...) == "", while other call sites treat that as unusable. In either case you can end up handing performKeyEquivalent an event with no usable key equivalent and skipping the raw-event fallback.

🔧 Suggested change 
             var menuEvent = event
-            if let chars = event.charactersIgnoringModifiers,
-               !chars.allSatisfy({ $0.isASCII }),
-               let normalized = KeyboardLayout.character(forKeyCode: event.keyCode, modifierFlags: event.modifierFlags) {
+            let rawChars = event.charactersIgnoringModifiers ?? ""
+            if (rawChars.isEmpty || !rawChars.allSatisfy(\.isASCII)),
+               let normalized = KeyboardLayout.character(forKeyCode: event.keyCode, modifierFlags: event.modifierFlags),
+               !normalized.isEmpty {
                 menuEvent = NSEvent.keyEvent(
                     with: event.type,
                     location: event.locationInWindow,
                     modifierFlags: event.modifierFlags,
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 12613 - 12628, The new normalization
branch for menuEvent can produce an empty-string key equivalent and bypass the
raw-event fallback; update the if-condition around
charactersIgnoringModifiers/normalized so you only replace the event when
normalized is non-nil and not empty (e.g. guard normalized?.isEmpty == false),
and treat empty string the same as nil so performKeyEquivalent still falls back
to the original/raw event; reference the menuEvent and event variables and the
call to KeyboardLayout.character(forKeyCode:modifierFlags:) to locate and adjust
the logic.

}
#if DEBUG
if menuEvent !== event {
dlog(" → normalized menu event chars: \(event.charactersIgnoringModifiers ?? "nil") → \(menuEvent.charactersIgnoringModifiers ?? "nil")")
}
#endif
let consumedByMenu = mainMenu.performKeyEquivalent(with: menuEvent)
#if DEBUG
if browserZoomShortcutTraceCandidate(
flags: event.modifierFlags,
Expand Down