fix(application): let registered accelerators win over the webview on… - #5902
Open
julianstorer wants to merge 2 commits into
Open
fix(application): let registered accelerators win over the webview on…#5902julianstorer wants to merge 2 commits into
julianstorer wants to merge 2 commits into
Conversation
… macOS On macOS the WKWebView is first responder and swallows most key events before they reach NSWindow's keyDown:. Common combos like Ctrl+Tab never get there, so an app-registered KeyBinding for them silently never fires. Override performKeyEquivalent: on WebviewWindow, which runs before the responder chain and before NSMenu's own key-equivalent dispatch. It calls into Go with the same accelerator string keyDown: builds, looks it up via the existing processKeyBinding path, and returns YES only on a real match. On a miss it falls through to super, so main-menu key equivalents and every other Cocoa default keep working — only accelerators the app explicitly registered are claimed. The Go side is a synchronous sibling of processWindowKeyDownEvent rather than a channel send, because performKeyEquivalent: needs the answer immediately to decide whether to consume the event. It's a single map read under RLock plus the same goroutine dispatch processKeyBinding already does, so the threading matches the keyDown: path. The modifier-string assembly is pulled out of keyDown: into acceleratorStringFromEvent: so both callers produce identical strings.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughmacOS keyboard equivalents now build accelerator strings and call a synchronous Go callback. The callback validates the window and accelerator, invokes ChangesmacOS key-equivalent processing
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
27 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On macOS the
WKWebViewis first responder and swallows most key events beforethey reach
NSWindow'skeyDown:. Common combos like Ctrl+Tab never get there,so an app-registered
KeyBindingfor them silently never fires.This overrides
performKeyEquivalent:onWebviewWindow, which runs before theresponder chain and before
NSMenu's own key-equivalent dispatch. It calls intoGo with the same accelerator string
keyDown:builds, looks it up via theexisting
processKeyBindingpath, and returnsYESonly on a real match. On amiss it falls through to
super, so main-menu key equivalents and every otherCocoa default keep working — only accelerators the app explicitly registered are
claimed. The modifier-string assembly is factored out of
keyDown:intoacceleratorStringFromEvent:so both callers produce identical strings.Type of change
How Has This Been Tested?
Registered a KeyBinding for an accelerator the webview otherwise consumes (e.g.
Ctrl+Tab) and confirmed the binding now fires, while unregistered combos and
main-menu shortcuts behave unchanged.
Summary by CodeRabbit