Repository navigation
Fix keyboard shortcuts not working with Korean input mode - #1913
lawrencecchen merged 1 commit into
Conversation
When a non-Latin input source like Korean 두벌식 is active, event.charactersIgnoringModifiers returns Hangul characters (e.g. ㅅ for T key) instead of Latin letters. This caused all character-based shortcut matching to fail — Cmd+T, Cmd+D, Cmd+1-9, Ctrl+N/P, etc. Root cause: KeyboardLayout.character(forKeyCode:modifierFlags:) assumed CJK input sources lack kTISPropertyUnicodeKeyLayoutData, but Korean 두벌식 has it. UCKeyTranslate returned Korean characters and the ASCII fallback was never reached. Fix: - KeyboardLayout.character(): check result is ASCII before accepting; fall through to TISCopyCurrentASCIICapableKeyboardInputSource() when the current source returns non-ASCII characters - Add KeyboardLayout.normalizedCharacters(for:) helper that normalizes event.charactersIgnoringModifiers for shortcut comparison - Apply normalization in handleCustomShortcut (AppDelegate), BrowserPanelView omnibar key handler, and BrowserPopupWindowController Cmd+W handler Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@JunyeongChoi0 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis change introduces keyboard input normalization improvements by adding a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/KeyboardLayout.swift">
<violation number="1" location="Sources/KeyboardLayout.swift:47">
P2: Non-ASCII fallback in shortcut normalization drops modifier flags, breaking command-aware keyboard layout translation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| static func normalizedCharacters(for event: NSEvent) -> String { | ||
| let raw = (event.charactersIgnoringModifiers ?? "").lowercased() | ||
| if raw.allSatisfy(\.isASCII) { return raw } | ||
| if let layoutChar = character(forKeyCode: event.keyCode, modifierFlags: []) { |
There was a problem hiding this comment.
P2: Non-ASCII fallback in shortcut normalization drops modifier flags, breaking command-aware keyboard layout translation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/KeyboardLayout.swift, line 47:
<comment>Non-ASCII fallback in shortcut normalization drops modifier flags, breaking command-aware keyboard layout translation.</comment>
<file context>
@@ -15,25 +15,41 @@ class KeyboardLayout {
+ static func normalizedCharacters(for event: NSEvent) -> String {
+ let raw = (event.charactersIgnoringModifiers ?? "").lowercased()
+ if raw.allSatisfy(\.isASCII) { return raw }
+ if let layoutChar = character(forKeyCode: event.keyCode, modifierFlags: []) {
+ return layoutChar
+ }
</file context>
| if let layoutChar = character(forKeyCode: event.keyCode, modifierFlags: []) { | |
| if let layoutChar = character(forKeyCode: event.keyCode, modifierFlags: event.modifierFlags) { |
Greptile SummaryThis PR fixes keyboard shortcuts (Cmd+T, Cmd+W, Ctrl+N/P, Cmd+1-9, etc.) failing when a Korean 두벌식 (or other CJK) input source is active. The root cause was two-fold: Key changes:
One minor observation: Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[NSEvent key press] --> B[normalizedCharacters for event]
B --> C{charactersIgnoringModifiers\nis ASCII or empty?}
C -- Yes --> D[Return lowercased raw string\nfast path - no TIS calls]
C -- No\nCJK char e.g. ㅅ --> E[character forKeyCode modifierFlags empty]
E --> F[TISCopyCurrentKeyboardInputSource\ne.g. Korean 두벌식]
F --> G[characterFromInputSource\nUCKeyTranslate]
G --> H{result.allSatisfy\nisASCII?}
H -- Yes e.g. Dvorak --> I[Return ASCII char]
H -- No e.g. Hangul --> J[TISCopyCurrentASCIICapableKeyboardInputSource\ne.g. US QWERTY]
J --> K[characterFromInputSource\nUCKeyTranslate on ASCII layout]
K --> L[Return t for T keyCode]
L --> M[Shortcut match succeeds\nCmd+T, Ctrl+N, Cmd+W, etc.]
D --> M
I --> M
Last reviewed commit: "Fix keyboard shortcu..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static func normalizedCharacters(for event: NSEvent) -> String { | ||
| let raw = (event.charactersIgnoringModifiers ?? "").lowercased() | ||
| if raw.allSatisfy(\.isASCII) { return raw } | ||
| if let layoutChar = character(forKeyCode: event.keyCode, modifierFlags: []) { | ||
| return layoutChar | ||
| } | ||
| return raw | ||
| } |
There was a problem hiding this comment.
allSatisfy on nil/empty input
"".allSatisfy(\.isASCII) is vacuously true, so when event.charactersIgnoringModifiers is nil (some synthetic events, special keys), normalizedCharacters returns "" and never reaches the character(forKeyCode:) fallback. This is intentional — all callers already have independent keyCode-based fallbacks for this case — but the asymmetry (non-ASCII triggers the lookup, empty string does not) is a subtle trap for future callers who may not expect "" when a valid keyCode is present.
A clarifying comment or a guard would improve long-term readability:
| static func normalizedCharacters(for event: NSEvent) -> String { | |
| let raw = (event.charactersIgnoringModifiers ?? "").lowercased() | |
| if raw.allSatisfy(\.isASCII) { return raw } | |
| if let layoutChar = character(forKeyCode: event.keyCode, modifierFlags: []) { | |
| return layoutChar | |
| } | |
| return raw | |
| } | |
| static func normalizedCharacters(for event: NSEvent) -> String { | |
| let raw = (event.charactersIgnoringModifiers ?? "").lowercased() | |
| // Fast path: ASCII (or empty/nil) events need no normalization. | |
| // Note: nil charactersIgnoringModifiers yields "" here intentionally; | |
| // callers that need a keyCode-based lookup for nil events should call | |
| // character(forKeyCode:modifierFlags:) directly. | |
| if raw.allSatisfy(\.isASCII) { return raw } | |
| if let layoutChar = character(forKeyCode: event.keyCode, modifierFlags: []) { | |
| return layoutChar | |
| } | |
| return raw | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Thank you for the contribution! |
…put-keyboard-shortcuts Fix keyboard shortcuts not working with Korean input mode
3 participants
Summary
KeyboardLayout.character(forKeyCode:modifierFlags:)assumed CJK input sources lackkTISPropertyUnicodeKeyLayoutData, but Korean 두벌식 has it —UCKeyTranslatereturned Hangul characters and the ASCII fallback was never reachedhandleCustomShortcutcompared rawevent.charactersIgnoringModifiers(which returns Korean chars like ㅅ for T) against Latin shortcut keysevent.keyCode(hardware scan codes) which are input-method-invariantChanges
KeyboardLayout.swift:character(forKeyCode:modifierFlags:)now checks if the result is ASCII before accepting; non-ASCII results fall through toTISCopyCurrentASCIICapableKeyboardInputSource(). AddednormalizedCharacters(for:)helper for reusable event character normalization.AppDelegate.swift:handleCustomShortcutusesKeyboardLayout.normalizedCharacters(for:)instead of rawevent.charactersIgnoringModifiers— fixes Cmd+1-9 workspace switching, Ctrl+1-9 surface selection, omnibar Cmd/Ctrl+N/P, command palette shortcuts, and allmatchShortcut-based shortcuts.BrowserPanelView.swift: Omnibar key handler uses normalized characters for Cmd/Ctrl+N/P navigation.BrowserPopupWindowController.swift: Popup Cmd+W close handler uses normalized characters.Test plan
🤖 Generated with Claude Code
Summary by cubic
Fixes character-based shortcuts failing when Korean 두벌식 or other non‑Latin input sources are active. Shortcuts now normalize to ASCII so Cmd+T, Cmd+D, Cmd+1–9, Ctrl+N/P, and Cmd+W work regardless of input method.
Written for commit 8cd9cd9. Summary will update on new commits.
Summary by CodeRabbit
Release Notes