fix(macos): pop up status item menu via NSStatusItem (fixes #251)#318
Open
TuYv wants to merge 1 commit into
Open
fix(macos): pop up status item menu via NSStatusItem (fixes #251)#318TuYv wants to merge 1 commit into
TuYv wants to merge 1 commit into
Conversation
…s#251 The previous implementation showed the menu by calling `button.performClick(None)`, which synthesises a click with no NSEvent context. AppKit's status-item popup logic relies on `[NSApp currentEvent]` to discover the active screen/Space; when the synthesised click resolves to the wrong context — specifically on a secondary display while a full-screen Space is active there — the popup silently no-ops. Drive the popup directly through `NSStatusItem.popUpStatusItemMenu`, which positions the menu off the status item's own button frame. This makes the menu appear reliably in every monitor/Space combination, including secondary displays whose active Space hosts a full-screen application. `popUpStatusItemMenu:` was deprecated in macOS 10.10 (Apple recommends binding the menu via `statusItem.menu` and letting AppKit handle the click), but the symbol is functionally intact on every supported macOS version and is the standard escape hatch when the custom subview already intercepts mouse events. The `on_tray_click` site also needs to release its `RefCell::borrow()` before entering the modal popup: a menu item action selected inside the modal can re-enter `TrayIcon::set_menu`, which calls `borrow_mut()` on the same RefCell. We clone the `Retained<NSMenu>` (an ObjC retain bump) and drop the borrow first so the modal cannot deadlock with itself. Fixes tauri-apps#251.
4 tasks
3 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
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.
fix(macos): pop up status item menu via
NSStatusItem(fixes #251)What
Stops the tray menu from silently failing to open on macOS when the click happens on a secondary display while a full-screen Space is active there. Drives the popup directly through
NSStatusItem.popUpStatusItemMenuinstead of synthesising a button click.Symptoms (matches #251)
Reported in #251, confirmed across multiple users:
The user comment tauri-apps/tray-icon#251 (comment by @andrewdavidmackenzie) captures this matrix exactly. macOS 14/15, both Intel and Apple Silicon, with and without "Displays have separate Spaces".
Root cause
The current implementation uses
button.performClick(None)to trigger the popup (in bothshow_menu()atsrc/platform_impl/macos/mod.rs:258andon_tray_clickatsrc/platform_impl/macos/mod.rs:515).performClick:simulates a press/release on the button without producing or attaching anNSEvent.NSStatusItem's internal popup logic then resolves the active screen/Space via[NSApp currentEvent]. On a secondary display whose active Space hosts a full-screen application, the most recent NSEvent in scope is bound to a different NSWindow (the full-screen window) than the status bar that received the real click; AppKit's popup positioning resolves to the wrong context and no-ops without warning or log output.This regression was introduced in PR #69
refactor(macos): rewrite impl to fix missing click issues(commit54fc7de, 2023-08-03, first released intray-icon@0.7.5). Before that PR, clicks reached theNSStatusBarButtondirectly viasendActionOn:and AppKit drove the popup with the realNSEvent. After the PR, a customTrayTargetsubview interceptsmouseDown:to surfaceClick/Enter/Moveevents to the Rust caller — so the real event is consumed by the subview andperformClick(None)is the only thing left to drive the popup.The bug is present in every release from 0.7.5 through HEAD.
Fix
Two call sites, same swap: replace
performClick(None)withNSStatusItem.popUpStatusItemMenu(menu). That method positions the menu off the status item's own button frame and does not need anNSEventto discover the active screen/Space.popUpStatusItemMenu:was deprecated in macOS 10.10 in favour of binding the menu viastatusItem.menuand letting AppKit handle the click. We cannot rely on that path here because theTrayTargetsubview already interceptsmouseDown:, so the native button-click → menu route is short-circuited. The symbol is functionally intact on every supported macOS version (10.12+) and is the standard escape hatch when the click path is overridden.Reentrancy: drop the
RefCell::borrow()before the modal popuppopUpStatusItemMenu:is modal — it does not return until the menu is dismissed. A menu item action selected inside that modal can re-enterTrayIcon::set_menu, which callsmenu.borrow_mut()on the sameRefCell(src/platform_impl/macos/mod.rs:137). Holding aborrow()across the modal would panic withalready borrowed: BorrowMutErrorfor the very common case "user clicks an item whose handler rebuilds the menu".on_tray_clickclones theRetained<NSMenu>(an ObjC retain bump — single atomic increment, not a deep copy) and drops theRefCellborrow at the end of the same expression, so the innerNSMenustays alive across the modal while theRefCellis free forset_menuto reborrow:The
show_menu()path readsself.attrs.menu(noRefCellinvolved) so it does not need the clone-and-drop dance.Verification
cargo checkclean on macOS; no new warnings introduced (thepopUpStatusItemMenudeprecation is suppressed locally with#[allow(deprecated)]and a comment explaining why).tray-icon@0.21.3with this patch applied through[patch.crates-io]). After the patch, the menu pops up reliably on a secondary display while the same display hosts a full-screen Space; previously it would silently no-op there. No regression on the other three (primary/secondary × desktop/full-screen) configurations.Click/Enter/Leave/Moveevent surface is unchanged;menu_on_left_click/menu_on_right_clicksemantics are preserved.Notes for reviewers
popUpMenuPositioningItem:atLocation:inView:first — it works too, but it needs theNSMenuItemfeature flag added toobjc2-app-kitand is slightly fussier about the view/location coordinate space.popUpStatusItemMenu:is the smallest possible change while being functionally equivalent for the status-bar use case.TrayTargetsubview'smouseDown:interception and forward tosuper.mouseDown(event)so AppKit's native click → popup path is restored. That is a much larger change and would alter the timing and ordering of theClickevents surfaced to Rust callers, so I am keeping it out of scope here.Closes #251.