Fix macOS system tray menu real-time updates using NSMenuDelegate #4828
Conversation
Implements NSMenuDelegate to enable real-time menu updates while the system tray menu is open on macOS. Previously, menu.Update() calls would not refresh the displayed menu until it was closed and reopened. Changes: - Added MenuDelegate class implementing NSMenuDelegate protocol - Implemented menuNeedsUpdate: and menuWillOpen: delegate methods - Added nsMenuDelegate field to macosSystemTray struct - Updated setMenu() to create and attach delegate to NSMenu - Updated run() to set up delegate during initialization Fixes #4630 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds a macOS NSMenuDelegate implementation and wires it to the Go system tray so menus receive lifecycle callbacks for real-time in-place updates; introduces delegate creation/attachment C functions and Go-side delegate pointer and callbacks for onMenuOpen/onMenuClose. Changes
Sequence DiagramsequenceDiagram
participant Go as Go code
participant Tray as macosSystemTray
participant ObjC as MenuDelegate (Obj‑C)
participant NSMenu as NSMenu (Cocoa)
Note over Go,Tray: Initialization
Go->>Tray: run() create NSStatusItem & NSMenu
Tray->>ObjC: createMenuDelegate(menuPtr, trayID)
ObjC-->>Tray: delegate pointer
Tray->>NSMenu: setMenuDelegate(nsMenu, delegate)
Note over NSMenu,ObjC: User opens menu
NSMenu->>ObjC: menuWillOpen:(menu)
ObjC->>Go: systrayMenuOpenCallback(trayID)
Go->>Tray: onMenuOpen handler
Note over NSMenu,ObjC: While open - updates
Go->>Tray: setMenu/updateMenuInPlace() (main thread)
Tray->>NSMenu: update items (preserve IDs)
NSMenu->>ObjC: menuNeedsUpdate:(menu)
ObjC->>Go: trigger update callback
Note over NSMenu,ObjC: Menu closed
NSMenu->>ObjC: menuDidClose:(menu)
ObjC->>Go: systrayMenuCloseCallback(trayID)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
v3/pkg/application/systemtray_darwin.go (1)
114-118: Delegate'smenuPtrmay become stale on subsequent menu changes.The delegate is created once (when
nsMenuDelegate == nil) with the currents.nsMenupointer. IfsetMenuis called again with a different menu, the delegate retains the oldmenuPtrreference. While the current delegate implementation doesn't usemenuPtr, this could cause issues if future changes rely on it.Consider updating
menuPtron every call or documenting this limitation:🔎 Suggested fix
// Set up the delegate if not already done if s.nsMenuDelegate == nil { s.nsMenuDelegate = C.createMenuDelegate(s.nsMenu, C.long(s.id)) + } else { + // Update the delegate's menuPtr if menu changed + C.updateMenuDelegatePtr(s.nsMenuDelegate, s.nsMenu) } C.setMenuDelegate(s.nsMenu, s.nsMenuDelegate)This would require adding an
updateMenuDelegatePtrfunction in the Objective-C layer.v3/pkg/application/systemtray_darwin.m (1)
262-267: Consider delegate lifecycle management.The
MenuDelegateis allocated with[[MenuDelegate alloc] init]but there's no corresponding release when the system tray is destroyed. While the delegate likely lives for the application lifetime, if system trays can be dynamically created/destroyed, this could leak memory.If needed, store the delegate reference and release it in
systemTrayDestroy:// In systemTrayDestroy, if delegate is tracked: // [delegate release];
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v3/UNRELEASED_CHANGELOG.mdv3/pkg/application/systemtray_darwin.gov3/pkg/application/systemtray_darwin.hv3/pkg/application/systemtray_darwin.m
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-24T22:41:18.566Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
Applied to files:
v3/pkg/application/systemtray_darwin.go
🪛 LanguageTool
v3/UNRELEASED_CHANGELOG.md
[style] ~26-~26: This phrase is redundant (‘OS’ stands for ‘operating system’). Use simply “macOS”.
Context: ... --> ## Fixed - Fix macOS system tray menu real-time updates using NSMen...
(ACRONYM_TAUTOLOGY)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (go)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
v3/UNRELEASED_CHANGELOG.md (1)
26-26: LGTM!The changelog entry clearly describes the fix and references the correct issue number. Note: The static analysis warning about "macOS" is a false positive—"macOS" is Apple's official branding and is correct here.
v3/pkg/application/systemtray_darwin.go (2)
48-48: LGTM!The new
nsMenuDelegatefield appropriately holds the Objective-C delegate reference usingunsafe.Pointerfor CGo interop.
188-190: LGTM!The delegate setup during initial
run()is correctly placed aftermenu.Update()ensures thensMenupointer is valid.v3/pkg/application/systemtray_darwin.h (2)
10-13: LGTM!The
MenuDelegateinterface declaration correctly adoptsNSMenuDelegateprotocol with appropriate properties for CGo interop.
30-31: LGTM!Function declarations are well-defined and match the implementation signatures.
v3/pkg/application/systemtray_darwin.m (2)
19-32: Clarify the real-time update mechanism.Both delegate methods are effectively no-ops. The comment says "The Go side will handle the actual menu rebuilding through menu.Update()" but there's no callback to Go from these methods.
Could you clarify: does simply having a delegate attached cause macOS to refresh the menu display when the underlying
NSMenuitems change? If the intent is formenuNeedsUpdate:to trigger Go-side updates, a callback would be needed.
270-274: LGTM!The
setMenuDelegatefunction correctly casts and assigns the delegate to theNSMenu.
Deploying wails with
|
| Latest commit: |
0cd2bef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d3ae43ca.wails.pages.dev |
| Branch Preview URL: | https://fix-systemtray-menu-realtime.wails.pages.dev |
Adds NSMenuDelegate methods menuWillOpen: and menuDidClose: to call the existing onMenuOpen and onMenuClose callbacks on SystemTray, bringing macOS to parity with Windows and Linux implementations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
|
In #4719, I reported two behaviors:
|
Previously, updating a systray menu while it was open would break click handlers because Menu.Update() destroyed and recreated all menu items with new IDs. The visible menu still referenced old IDs, causing "MenuItem #X not found" errors when clicked. This fix modifies macosMenu.update() to update existing menu items in-place using the existing C setter functions (setMenuItemLabel, setMenuItemDisabled, etc.) instead of clearing and rebuilding the entire menu. This preserves the menuItemID baked into each NSMenuItem. Fixes #4719 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@v3/pkg/application/systemtray_darwin.m`:
- Around line 268-279: The MenuDelegate allocated in createMenuDelegate is never
released; add a new C function (e.g., releaseMenuDelegate(void* delegate)) that
calls [(MenuDelegate *)delegate release] (or CFBridgingRelease equivalent) to
free the object, update setMenuDelegate usage to expect that delegates are
managed externally, and call this releaseMenuDelegate from the Go destroy() path
and from the setMenu(nil) branch where menus are cleared so the retained
MenuDelegate instances are properly freed.
In `@v3/UNRELEASED_CHANGELOG.md`:
- Line 26: The changelog entry "Fix macOS system tray menu real-time updates
using NSMenuDelegate" references the wrong issue number (`#4630`); verify the PR
description and, if it indeed fixes issue `#4719`, update that changelog line to
reference `#4719` instead of `#4630` and ensure any related cross-references in the
same changelog are consistent.
🧹 Nitpick comments (1)
v3/pkg/application/menu_darwin.go (1)
93-120: In-place update doesn't handle menu structure changes.The implementation correctly updates existing item properties, but items added after initial menu creation (where
item.impl == nil) are silently skipped. This limitation is documented in the comment, but it could lead to confusion if users expect dynamically added items to appear.Additionally, separator items don't have an
implset (they're added directly viaC.addMenuSeparator), so any separator-related changes would also be skipped.Consider documenting this limitation in the public API or logging a warning when items are skipped.
| void* createMenuDelegate(void* menuPtr, long trayID) { | ||
| MenuDelegate *delegate = [[MenuDelegate alloc] init]; | ||
| delegate.menuPtr = menuPtr; | ||
| delegate.trayID = trayID; | ||
| return (void*)delegate; | ||
| } | ||
|
|
||
| // Set the delegate on a menu | ||
| void setMenuDelegate(void* nsMenu, void* delegate) { | ||
| NSMenu *menu = (NSMenu *)nsMenu; | ||
| MenuDelegate *menuDelegate = (MenuDelegate *)delegate; | ||
| [menu setDelegate:menuDelegate]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd v3 && git ls-files | grep -E "(systemtray|tray)" | head -20Repository: wailsapp/wails
Length of output: 778
🏁 Script executed:
wc -l v3/pkg/application/systemtray_darwin.mRepository: wailsapp/wails
Length of output: 100
🏁 Script executed:
cat -n v3/pkg/application/systemtray_darwin.m | sed -n '260,290p'Repository: wailsapp/wails
Length of output: 839
🏁 Script executed:
# Search for all uses of createMenuDelegate, setMenuDelegate, MenuDelegate
rg -n 'createMenuDelegate|setMenuDelegate|MenuDelegate|destroyMenuDelegate' v3/pkg/application/Repository: wailsapp/wails
Length of output: 1441
🏁 Script executed:
# Check if there's a Go file that calls these functions
fd '\.go$' v3/pkg/application/ | head -20Repository: wailsapp/wails
Length of output: 864
🏁 Script executed:
cat -n v3/pkg/application/systemtray_darwin.go | sed -n '45,55p'Repository: wailsapp/wails
Length of output: 429
🏁 Script executed:
cat -n v3/pkg/application/systemtray_darwin.go | sed -n '130,220p'Repository: wailsapp/wails
Length of output: 3354
🏁 Script executed:
# Check if there's any cleanup or Close method
rg -n 'Close|Cleanup|dealloc|release' v3/pkg/application/systemtray_darwin.goRepository: wailsapp/wails
Length of output: 227
🏁 Script executed:
# Check the full struct and any methods
ast-grep --pattern 'type $_ struct {
$$$
nsMenuDelegate $_
$$$
}'Repository: wailsapp/wails
Length of output: 1045
🏁 Script executed:
# Check if delegate is ever reassigned
cat -n v3/pkg/application/systemtray_darwin.go | grep -A 5 -B 5 'nsMenuDelegate'Repository: wailsapp/wails
Length of output: 1390
🏁 Script executed:
# Search for Close, Hide, Stop, or cleanup methods for macosSystemTray
rg -n 'func.*macosSystemTray.*\)' v3/pkg/application/systemtray_darwin.go | head -30Repository: wailsapp/wails
Length of output: 919
🏁 Script executed:
# Check the full file for any cleanup/deallocation
cat -n v3/pkg/application/systemtray_darwin.go | tail -100Repository: wailsapp/wails
Length of output: 3325
🏁 Script executed:
# Check if there's a SystemTray interface that defines required methods
rg -n 'type SystemTray' v3/pkg/application/system_tray_manager.goRepository: wailsapp/wails
Length of output: 91
🏁 Script executed:
# Look for Close/Hide/Stop method on macosSystemTray
rg -A 10 'func \(s \*macosSystemTray\) (Close|Hide|Stop|destroy)' v3/pkg/application/systemtray_darwin.goRepository: wailsapp/wails
Length of output: 575
🏁 Script executed:
# Check systemTrayDestroy implementation in the .m file
rg -A 20 'void systemTrayDestroy' v3/pkg/application/systemtray_darwin.mRepository: wailsapp/wails
Length of output: 1095
🏁 Script executed:
# Check if there are any other NSMenu delegate patterns in the codebase
rg -n 'setDelegate' v3/pkg/application/Repository: wailsapp/wails
Length of output: 554
🏁 Script executed:
# Check MenuDelegate dealloc or other cleanup
rg -B 5 -A 15 '@implementation MenuDelegate' v3/pkg/application/systemtray_darwin.mRepository: wailsapp/wails
Length of output: 561
🏁 Script executed:
# Check if MenuDelegate has a dealloc method
rg -A 10 'dealloc' v3/pkg/application/systemtray_darwin.mRepository: wailsapp/wails
Length of output: 40
🏁 Script executed:
# Look at the complete systemtray_darwin.m to see full MenuDelegate
cat -n v3/pkg/application/systemtray_darwin.m | sed -n '1,40p'Repository: wailsapp/wails
Length of output: 1308
🏁 Script executed:
# Check if there's any reference counting or memory management in the header
cat -n v3/pkg/application/systemtray_darwin.hRepository: wailsapp/wails
Length of output: 1567
🏁 Script executed:
# Check if there are multiple delegate creations and assignments in run()
cat -n v3/pkg/application/systemtray_darwin.go | sed -n '192,215p'Repository: wailsapp/wails
Length of output: 1028
🏁 Script executed:
# Verify: Does NSMenu retain delegate in Cocoa documentation context?
# Check how other delegates are managed in the codebase
rg -B 5 -A 5 'appDelegate|windowDelegate' v3/pkg/application/systemtray_darwin.goRepository: wailsapp/wails
Length of output: 40
🏁 Script executed:
# Look at application_darwin.go to see how appDelegate is managed
rg -B 3 -A 10 'appDelegate.*release' v3/pkg/application/application_darwin.goRepository: wailsapp/wails
Length of output: 312
🏁 Script executed:
# Check if setMenu is called after initial run
rg -n 'setMenu' v3/pkg/application/systemtray_darwin.goRepository: wailsapp/wails
Length of output: 214
🏁 Script executed:
# Verify the flow: does setMenu get called multiple times or only once per tray?
cat -n v3/pkg/application/systemtray_darwin.go | sed -n '123,142p'Repository: wailsapp/wails
Length of output: 697
🏁 Script executed:
# Final check: Confirm NSMenu delegate is weak reference by checking if any release happens
rg -n 'MenuDelegate.*release|release.*MenuDelegate' v3/pkg/application/Repository: wailsapp/wails
Length of output: 40
🏁 Script executed:
# Also check if there's any autorelease or cleanup in setMenu for old delegates
cat -n v3/pkg/application/systemtray_darwin.go | sed -n '206,215p'Repository: wailsapp/wails
Length of output: 433
Add memory cleanup for MenuDelegate to prevent leaks.
createMenuDelegate allocates a retained MenuDelegate object, but destroy() and setMenu(nil) never release it. NSMenu holds only a weak reference to its delegate, so the Go-side code is responsible for cleanup. Add a release function and call it in the destroy path and when menus are cleared.
Suggested implementation
+// Release a menu delegate
+void destroyMenuDelegate(void* delegate) {
+ if (delegate != NULL) {
+ MenuDelegate *menuDelegate = (MenuDelegate *)delegate;
+ [menuDelegate release];
+ }
+}Then in systemtray_darwin.go destroy():
func (s *macosSystemTray) destroy() {
+ if s.nsMenuDelegate != nil {
+ C.destroyMenuDelegate(s.nsMenuDelegate)
+ s.nsMenuDelegate = nil
+ }
C.systemTrayDestroy(s.nsStatusItem)
}And in setMenu() when clearing:
func (s *macosSystemTray) setMenu(menu *Menu) {
s.menu = menu
if menu == nil {
+ if s.nsMenuDelegate != nil {
+ C.destroyMenuDelegate(s.nsMenuDelegate)
+ s.nsMenuDelegate = nil
+ }
s.nsMenu = nil
return
}🤖 Prompt for AI Agents
In `@v3/pkg/application/systemtray_darwin.m` around lines 268 - 279, The
MenuDelegate allocated in createMenuDelegate is never released; add a new C
function (e.g., releaseMenuDelegate(void* delegate)) that calls [(MenuDelegate
*)delegate release] (or CFBridgingRelease equivalent) to free the object, update
setMenuDelegate usage to expect that delegates are managed externally, and call
this releaseMenuDelegate from the Go destroy() path and from the setMenu(nil)
branch where menus are cleared so the retained MenuDelegate instances are
properly freed.
|
|
||
| ## Fixed | ||
| <!-- Bug fixes --> | ||
| - Fix macOS system tray menu real-time updates using NSMenuDelegate (#4630) |
There was a problem hiding this comment.
Incorrect issue number referenced.
The changelog references #4630, but the PR description states this fixes issue #4719. Please verify and correct the issue number.
Proposed fix (if `#4719` is correct)
-- Fix macOS system tray menu real-time updates using NSMenuDelegate (`#4630`)
+- Fix macOS system tray menu real-time updates using NSMenuDelegate (`#4719`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Fix macOS system tray menu real-time updates using NSMenuDelegate (#4630) | |
| - Fix macOS system tray menu real-time updates using NSMenuDelegate (`#4719`) |
🧰 Tools
🪛 LanguageTool
[style] ~26-~26: This phrase is redundant (‘OS’ stands for ‘operating system’). Use simply “macOS”.
Context: ... --> ## Fixed - Fix macOS system tray menu real-time updates using NSMen...
(ACRONYM_TAUTOLOGY)
🤖 Prompt for AI Agents
In `@v3/UNRELEASED_CHANGELOG.md` at line 26, The changelog entry "Fix macOS system
tray menu real-time updates using NSMenuDelegate" references the wrong issue
number (`#4630`); verify the PR description and, if it indeed fixes issue `#4719`,
update that changelog line to reference `#4719` instead of `#4630` and ensure any
related cross-references in the same changelog are consistent.
Summary
Implements NSMenuDelegate to enable real-time menu updates while the system tray menu is open on macOS. Previously,
menu.Update()calls would not refresh the displayed menu until it was closed and reopened.Also implements
onMenuOpenandonMenuClosecallbacks for macOS, bringing it to parity with Windows and Linux.Changes
MenuDelegateclass implementingNSMenuDelegateprotocolmenuNeedsUpdate:,menuWillOpen:, andmenuDidClose:delegate methodsnsMenuDelegatefield tomacosSystemTraystructsetMenu()to create and attach delegate to NSMenurun()to set up delegate during initializationsystrayMenuOpenCallbackandsystrayMenuCloseCallbackHow it works
MenuDelegateis attached to the NSMenu when menu is setmenu.Update()is called, NSMenu is rebuilt in-placemenuNeedsUpdate:before display and periodicallymenuWillOpen:triggersonMenuOpencallbackmenuDidClose:triggersonMenuClosecallbackPlatform parity
Fixes #4719
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.