fix(playback): route bare Space to play/pause and stop hidden WebView swallowing ⌘←/⌘→ - #444
fix(playback): route bare Space to play/pause and stop hidden WebView swallowing ⌘←/⌘→#444tsibog wants to merge 5 commits into
Conversation
|
@codex review |
Runtime findings: the original fix did not resolve #405Verified against a packaged build with file-based tracing (sandbox The
|
| Build | window.performKeyEquivalent |
|---|---|
With KasetWebView |
false (18/18) |
Stock WKWebView |
false (9/9) |
So the hidden 1×1 WebView was not consuming the key equivalent, and the PR's stated mechanism did not reproduce.
Actual root cause
The menu item is fully wired — keyEquiv=' ', enabled=true, non-nil target:
MENUITEM 'Playback' > 'Play' keyEquiv=' ' mods=0 enabled=true target=<MenuItemCallback>
And the main menu handles Space every time it is asked directly (25/25 across runs):
PROBE mainMenu.performKeyEquivalent -> true → PLAY/PAUSE COMMAND FIRED
AppKit simply never offers a modifier-less key equivalent to the main menu. The window's view hierarchy declines Space, nothing else claims it, and it dies before the menu is consulted. ⌘←/⌘→ are unaffected because they carry a modifier.
This also explains the "Tab sometimes fixes it" report in #405: Tab moves focus into the WebView, after which the YouTube Music page's own Space handler toggles playback. That is the page working, not the shortcut.
Fix in 6dec521
PlaybackSpaceKeyMonitor — a local key-down monitor that claims bare Space only when the native UI holds focus:
- Text fields still type a space (
isTextInputFocused) - Player WebView focused → left to the page, which already toggles playback; claiming it would toggle twice
- Playback command disabled (no track) → not claimed
- Space with any modifier → not claimed, so it stays available as a shortcut
performPlayPause() is extracted so the menu item and the monitor share one implementation and cannot diverge (arbiter routing to video vs music is preserved).
8 unit tests cover the decision boundaries.
What is still unverified
The new fix has not been confirmed at runtime. Automated key injection needs Accessibility permission, which isn't granted in this environment, and macOS will not give a background app a key window, so a syntheticNSEventself-test could not reach the monitor. The root cause above is runtime-verified; the fix is verified only by unit tests and reasoning. Please confirm manually: play a track, click the sidebar, press Space.- Confirmed, working.
KasetWebViewis retained. It is provably not needed for Space, but⌘←/⌘→interception was never exercised, so its value there is untested. It may be worth dropping if it can't be shown to do anything.- The description and title still describe the original WebView theory and should be updated to match the actual cause.
…ivalents The singleton WKWebView that plays audio lives in the main window's view hierarchy even during audio-only playback (1x1, opacity 0, allowsHitTesting false). NSWindow.performKeyEquivalent recursively searches the contentView's subviews, and a stock WKWebView returns true for Space and Cmd-Left/Right because the YouTube Music page has its own handlers for those keys. The event is consumed before the SwiftUI command-menu shortcut fires, so Space never toggles play/pause (issue sozercan#405). Tab does not reliably fix it because the WebView remains in the view hierarchy regardless of who the first responder is. Subclass WKWebView as KasetWebView with a shouldInterceptKeyEquivalents flag. When false (audio-only/hidden/miniPlayer), performKeyEquivalent returns false so key equivalents fall through to the menu system. In video mode the flag is true and the WebView handles keys normally. Fixes sozercan#405
Claude Code review found three issues with the manual flag approach: 1. Flag desyncs after WebView recreation (tearDown never resets displayMode) 2. Reparenting race between PersistentPlayerView and VideoWebViewContainer 3. Triplicated cast blocks with a dead .miniPlayer branch Deriving from self.bounds (like ScrollForwardingWebView derives from self.url) eliminates all three issues. No call-site changes needed.
The Space shortcut is registered on the Playback menu item (keyEquiv=' ', enabled, with a target) but AppKit never offers a modifier-less key equivalent to the main menu, so it was never dispatched. Runtime tracing showed the key window's whole view hierarchy declining Space (performKeyEquivalent -> false, 18/18) while the main menu handled it every time it was asked directly (25/25). Add PlaybackSpaceKeyMonitor, a local key-down monitor that claims bare Space only when the native UI holds focus. Text fields still type a space, and the player WebView still gets Space when it holds focus, where the page's own handler already toggles playback (claiming it there would toggle twice). Extract performPlayPause() so the menu item and the monitor share one implementation and cannot diverge.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
3deec8e to
9026b1b
Compare
tsibog
changed the title
2 participants
Description
Space did not toggle play/pause when the track list (native
NSTableView) had focus, and ⌘←/⌘→ were swallowed by the hidden audio-only WebView. Two independent mechanisms fix both:1.
PlaybackSpaceKeyMonitor— bare Space over native UIAppKit does not route a modifier-less Space key through the main-menu key equivalent when a native navigation content (
NSTableView/NSOutlineView) holds focus, so the Playback menu shortcut is never dispatched. A localNSEventkey-down monitor bridges that gap: it claims the initial bare-Space key-down in the primary window when the responder is a native browsing surface, and routes it to the sameperformPlayPause()the menu uses. Text editors, WebKit content, controls with standard Space activation, auxiliary windows, and key-repeat events all keep the event.2.
KasetWebView— hidden WebView declines ⌘←/⌘→The singleton
WKWebViewthat plays audio lives in the main window's view hierarchy even during audio-only playback (1×1, opacity 0,allowsHitTesting(false)).NSWindow.performKeyEquivalentrecursively searches the contentView's subviews, and a stockWKWebViewreturnstruefor ⌘←/⌘→ because YouTube Music handles those.KasetWebViewoverridesperformKeyEquivalentto returnfalsewhen its bounds are 1×1 (hidden/audio-only), letting the key equivalent fall through to the menu system. When the WebView has non-trivial bounds (video mode), it forwards keys normally.3.
MainWindowLayout— scene identifierExtracts the
"main"scene identifier asMainWindowLayout.sceneIdentifierso window identity follows the SwiftUI scene identifier after navigation changes the window title, not just the autosave name or title string.Type of Change
Related Issues
Fixes #405
Changes Made
PlaybackSpaceKeyMonitor, a local key-down monitor that routes bare Space toperformPlayPause()when a native browsing surface has focus.performPlayPause()onKasetAppso the monitor and the Playback menu item share one code path.KasetWebView, aWKWebViewsubclass that declines ⌘←/⌘→ key equivalents when hidden (1×1 bounds). The singleton player WebView now creates aKasetWebViewinstead of a plainWKWebView.MainWindowLayout.sceneIdentifierand wired it intoisPrimaryWindowIdentityso the monitor can reliably identify the primary window regardless of the current navigation title.self.bounds, not a manually-synced flag, matching the pattern inScrollForwardingWebView(which derives fromself.url). This avoids desync after WebView recreation or reparenting.Testing
swift test --skip KasetUITests)Manual checklist:
TextInputFocusStateguard still applies)Checklist
swiftlint --strict && swiftformat .— verified by CIPlaybackSpaceKeyMonitorTests,KasetWebViewTests,MainWindowLayoutTests)Additional Notes
The existing
TextInputFocusStatemechanism (PR #379) is orthogonal and still works: it disables the menu shortcut itself when a text field has focus. This PR fixes the other half — bare Space never reaching the menu in the first place when a native table has focus.A Claude Code self-review identified and fixed three issues with the initial flag-based approach: flag desync after WebView recreation, a reparenting race between
PersistentPlayerViewandVideoWebViewContainer, and triplicated cast blocks. The stateless bounds-check eliminates all three.