[oc7o]

What

Adds a native macOS implementation of the TrayMenuController SPI, backed by NSStatusItem/NSMenu, replacing the AWT-based tray menu on macOS.

Why

The AWT tray menu has long-standing rendering and behavior issues on macOS (cryptomator/cryptomator#1977); e.g. clicking a submenu closes the whole tray menu. A native AppKit menu fixes these.

How

• MacTrayMenuController (Java) implements TrayMenuController and is registered via provides in module-info.
• A JNI backend (org_cryptomator_macos_tray_MacTrayMenuController_Native.m) drives NSStatusItem and NSMenu directly, supporting:
  • action items, submenus, and separators
  • tray icon set/update (template image)
  • a "before open" menu listener (menuNeedsUpdate:)
• All AppKit calls run on the main thread; a menu rebuild happens within a single main thread hop, so concurrent rebuilds cannot interleave.
• The root menu lives for the library's lifetime, so the menu can be built before or after the icon is shown.
• Follows the SPI contract: TrayMenuException if the icon cannot be added, IllegalStateException for icon updates/listeners before the icon exists. Callback exceptions are logged via slf4j.
• Native source added to the Xcode project.

Testing

• Unit test: the controller is loadable via TrayMenuController.get().
• Manual: built Cryptomator against this snapshot on macOS 27 (arm64) and checked icon, menu, submenus (stay open), actions, vault lock/unlock state refresh before opening, and quit.

Notes

• macOS only; no behavior change on other platforms.
• defaultAction is ignored: clicking a macOS status item always opens its menu.
• Unrelated to this PR: building with Xcode 27 requires raising MACOSX_DEPLOYMENT_TARGET (currently 10.13; Xcode 27 supports 12.0+).

🤖 Generated with Claude Code

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4409ca25-ec83-4643-9872-0f26a687428e

📥 Commits

Reviewing files that changed from the base of the PR and between cd9c435 and 34372bc.

📒 Files selected for processing: 5

• src/main/headers/org_cryptomator_macos_tray_MacTrayMenuController_Native.h
• src/main/java/org/cryptomator/macos/tray/MacTrayMenuController.java
• src/main/native/Integrations.xcodeproj/project.pbxproj
• src/main/native/org_cryptomator_macos_tray_MacTrayMenuController_Native.m
• src/test/java/org/cryptomator/macos/tray/MacTrayMenuControllerTest.java

🚧 Files skipped from review as they are similar to previous changes: 1

• src/main/native/Integrations.xcodeproj/project.pbxproj

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

Adds a macOS tray menu controller that displays and updates tray icons, builds menus with actions, separators, and submenus, and registers a before-open listener. The Objective-C JNI implementation performs native operations on the main thread and invokes Java callbacks. The module registers the controller as a TrayMenuController provider, and the Xcode project includes the native source.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Suggested reviewers: overheadhunter

Merge Risk: ⚪ Minimal · up to 34372

The macOS tray menu is ready to merge after normal checks. A malformed menu item can leave the displayed menu partially updated, but ordinary rebuild failures are reported to callers.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 3 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: a native macOS TrayMenuController implementation.
Description check		The description is directly related to the changeset and explains the native AppKit implementation, JNI backend, SPI registration, testing, and scope.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 3 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/java/org/cryptomator/macos/tray/MacTrayMenuController.java`:
- Around line 49-53: The loadPng method can return null if the provided
iconLoader never invokes the TrayIconLoader.PngData consumer; after calling
iconLoader.accept(...) check holder[0] and if it is null return an empty byte[]
(or otherwise non-null sentinel) instead of null to avoid passing a null array
to native code that calls GetArrayLength; update loadPng (and its single-use
holder/consumer logic) to ensure it always returns a non-null byte[].
- Around line 29-31: The native implementation drops the Java Runnable passed
from MacTrayMenuController.showTrayIcon (Native.showTrayIcon) so clicks do
nothing; update
Java_org_cryptomator_macos_tray_MacTrayMenuController_00024Native_showTrayIcon
to store the passed defaultAction (Runnable) in a global/reference table, set
gStatusItem.button.target and action to a native selector that, when invoked on
click, looks up that stored Runnable and calls its run method via JNI; ensure
you retain a global JNI globalRef to the Runnable and release it when removing
the tray icon or on shutdown, and use the existing gStatusItem.button,
Java_org_cryptomator_macos_tray_MacTrayMenuController_00024Native_showTrayIcon
symbol names to locate where to add the storage, selector hookup, and JNI
invocation.

In `@src/main/native/org_cryptomator_macos_tray_MacTrayMenuController_Native.m`:
- Around line 89-114: The native showTrayIcon currently ignores the
defaultAction jobject; capture and store it as a JNI global reference (e.g.,
create a static jobject gDefaultAction via (*env)->NewGlobalRef(env,
defaultAction)) so it can be invoked later, and ensure you ReleaseGlobalRef when
replacing/removing the tray (cleanup). Also wire gStatusItem.button.action and
.target to an Objective‑C selector (e.g., -handleDefaultAction:) implemented in
the same file that obtains a JNIEnv, finds the Java callback method (or
Runnable.run) and calls it on gDefaultAction; keep gRootMenu and gMenuDelegate
behavior but change menu presentation so left-click triggers the selector
(invoke defaultAction) and show the menu manually (popUpMenu:forEvent: or on
right/Ctrl-click) if you want both menu and defaultAction to work.
- Around line 22-27: callRunnable currently invokes Runnable.run via
CallVoidMethod but never checks for a pending Java exception; update
callRunnable to, immediately after CallVoidMethod, check for exceptions with
ExceptionCheck/ExceptionOccurred, clear the pending exception, rethrow the same
exception back into the JVM with Throw (and delete any local refs), so that a
Java exception from Runnable.run is not left pending and is propagated
correctly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a07322f-1f0f-4204-97eb-ef3ef09071e0

📥 Commits

Reviewing files that changed from the base of the PR and between d560150 and 5e1b580.

📒 Files selected for processing (6)

• .gitignore
• src/main/headers/org_cryptomator_macos_tray_MacTrayMenuController_Native.h
• src/main/java/module-info.java
• src/main/java/org/cryptomator/macos/tray/MacTrayMenuController.java
• src/main/native/Integrations.xcodeproj/project.pbxproj
• src/main/native/org_cryptomator_macos_tray_MacTrayMenuController_Native.m

[oc7o]

@overheadhunter
Are the fixes alright?

[oc7o]

What about this PR?

[overheadhunter]

Sorry, lost this PR out of sight.

Two general remarks concerning the ObjC implementation:

1. In JNI calls, add @autoreleasepool {...} where you use autoreleased object (like [NSData dataWithBytes:...])
2. all the tray menu APIs must be called on the main thread. Add some check like if (NSThread.isMainThread) { ... } else { dispatch_sync(dispatch_get_main_queue(), ...) }. The calling java thread should be the main thread, but this not guaranteed by the API contract.

[oc7o]

@overheadhunter

Thank you for taking the time to review this PR! This is really great for learning (since I'm at my current job in a Jr. position I take all the feedback and learning opportunities I can get 🙏)

No worries I got time, but I thought I did something wrong 🥲

Regarding 1: I added the @autoreleasepool {...} for showTrayIcon, updateTrayIcon, addActionItem, addSeparator, addSubMenuItem

Towards 2: I added this helper function runOnMainThread to guarantee that the code passed to it is executed on the main thread.

🐙

[overheadhunter]

we're mostly there, but I believe not all the autoreleasepools are really needed. I'm not an ObjC expert any more, so please double check when an object is allocated/retained/released/autoreleased. (this is worth learning about 😉)

[oc7o]

@overheadhunter Thanks, and you were right: I hadn't actually checked ownership; I'd just wrapped everything in pools. 🙈

The rule I follow now (the file is compiled with ARC): objects returned by alloc/new/copy/mutableCopy are owned (+1) and released by ARC at the end of their scope, so they never go through an autorelease pool. Only +0 returns, typically from convenience constructors, can be autoreleased.

So instead of adding pools on the calling Java thread, I now convert Java strings/arrays with initWithUTF8String: / initWithBytes:length: (helpers marked NS_RETURNS_RETAINED), so nothing on the Java thread is autoreleased anymore. Only two pools remain, each with a comment:

• showTrayIcon: systemStatusBar / statusItemWithLength:
• addSeparator: separatorItem

All other main-thread work already runs within the run loop's / main queue's pool.

While double-checking, I also fixed a few more things:

• Root menu lifecycle: the root menu is now created once. Before, building the menu before showTrayIcon returned a dangling submenu handle.
• Atomic rebuild: updateTrayMenu rebuilds the whole menu within a single main-thread hop, so concurrent rebuilds can't interleave. I deliberately avoided a Java lock here, since combined with dispatch_sync it could deadlock.
• JNI robustness: getEnv now uses GetEnv first (like SKYAppearanceObserver) and only attaches as a daemon when detached. NULL strings, arrays and actions are guarded, and it uses JNI_VERSION_10 like the rest of the library.
• SPI contract: showTrayIcon throws TrayMenuException if the icon can't be added, and updateTrayIcon/onBeforeOpenMenu throw IllegalStateException before the icon exists. Callback exceptions are logged via slf4j instead of ExceptionDescribe.
• Before-open listener: it now runs from menuNeedsUpdate: instead of menuWillOpen:, since the listener may rebuild the menu and AppKit only allows structural changes there.
• Cleanup: removed JNI_OnUnload, added an SPI loading test and the standard file header, and dropped the unrelated .gitignore change.

I smoke-tested it by building Cryptomator against this snapshot (macOS 27, arm64): icon, menu, submenus (they stay open now), actions, the vault state refresh when opening the menu, and quit all work.

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.