fix: use native NSStatusItem for the macOS tray menu

client/platforms/macos/macosstatusicon.h

@@ -5,8 +5,10 @@
 #ifndef MACOSSTATUSICON_H
 #define MACOSSTATUSICON_H
 
-#include <QMenu>
 #include <QObject>
+#include <QString>
+
+class QMenu;
 
 class MacOSStatusIcon final : public QObject {
   Q_OBJECT
@@ -16,12 +18,12 @@ class MacOSStatusIcon final : public QObject {
   explicit MacOSStatusIcon(QObject* parent);
   ~MacOSStatusIcon();
 
- public:
-  void setIcon(const QString& iconUrl);
-  void setIndicatorColor(const QColor& indicatorColor);
-  void setMenu(NSMenu* statusBarMenu);
-  void setToolTip(const QString& tooltip);
+  void setIcon(const QString& iconPath);
+  void setMenu(QMenu* menu);
   void showMessage(const QString& title, const QString& message);
+
+ private:
+  void* m_statusItem = nullptr;
 };
 
 #endif  // MACOSSTATUSICON_H

client/platforms/macos/macosstatusicon.mm

@@ -4,201 +4,64 @@
 
 #include "macosstatusicon.h"
 #include "leakdetector.h"
-#include "logger.h"
+
+#include <QMenu>
 
 #import <Cocoa/Cocoa.h>
 #import <UserNotifications/UserNotifications.h>
 #import <QResource>
 
-/**
- * Creates a NSStatusItem with that can hold an icon. Additionally a NSView is
- * set as a subview to the button item of the status item. The view serves as
- * an indicator that can be displayed in color eventhough the icon is set as a
- * template. In that way we give the system control over it’s effective
- * appearance.
- */
-@interface MacOSStatusIconDelegate : NSObject
-@property(assign) NSStatusItem* statusItem;
-@property(assign) NSView* statusIndicator;
-
-- (void)setIcon:(NSData*)imageData;
-- (void)setIndicator;
-- (void)setIndicatorColor:(NSColor*)color;
-- (void)setMenu:(NSMenu*)statusBarMenu;
-- (void)setToolTip:(NSString*)tooltip;
-@end
-
-@implementation MacOSStatusIconDelegate
-/**
- * Initializes and sets the status item and indicator objects.
- *
- * @return An instance of MacOSStatusIconDelegate.
- */
-- (id)init {
-  self = [super init];
-
-  // Create status item
-  self.statusItem =
-      [[[NSStatusBar systemStatusBar] statusItemWithLength:NSSquareStatusItemLength] retain];
-  self.statusItem.visible = true;
-  // Add the indicator as a subview
-  [self setIndicator];
-
-  return self;
-}
-
-/**
- * Sets the image for the status icon.
- *
- * @param iconPath The data for the icon image.
- */
-- (void)setIcon:(NSData*)imageData {
-  NSImage* image = [[NSImage alloc] initWithData:imageData];
-  [image setTemplate:true];
-
-  [self.statusItem.button setImage:image];
-  [image release];
-}
-
-/**
- * Adds status indicator as a subview to the status item button.
- */
-- (void)setIndicator {
-  float viewHeight = NSHeight([self.statusItem.button bounds]);
-  float dotSize = viewHeight * 0.35;
-  float dotOrigin = (viewHeight - dotSize) * 0.8;
-
-  NSView* dot = [[NSView alloc] initWithFrame:NSMakeRect(dotOrigin, dotOrigin, dotSize, dotSize)];
-  self.statusIndicator = dot;
-  self.statusIndicator.wantsLayer = true;
-  self.statusIndicator.layer.cornerRadius = dotSize * 0.5;
-
-  [self.statusItem.button addSubview:self.statusIndicator];
-  [dot release];
-}
-
-/**
- * Sets the color if the indicator.
- *
- * @param color The indicator background color.
- */
-- (void)setIndicatorColor:(NSColor*)color {
-  if (self.statusIndicator) {
-    self.statusIndicator.layer.backgroundColor = color.CGColor;
-  }
-}
-
-/**
- * Sets the status bar menu to the status item.
- *
- * @param statusBarMenu The menu object that is passed from QT.
- */
-- (void)setMenu:(NSMenu*)statusBarMenu {
-  [self.statusItem setMenu:statusBarMenu];
-}
-
-/**
- * Sets the tooltip string for the status item.
- *
- * @param tooltip The tooltip string.
- */
-- (void)setToolTip:(NSString*)tooltip {
-  [self.statusItem.button setToolTip:tooltip];
-}
-@end
-
-namespace {
-Logger logger("MacOSStatusIcon");
-
-MacOSStatusIconDelegate* m_statusBarIcon = nullptr;
-}
-
 MacOSStatusIcon::MacOSStatusIcon(QObject* parent) : QObject(parent) {
   MZ_COUNT_CTOR(MacOSStatusIcon);
 
-  logger.debug() << "Register delegate";
-  Q_ASSERT(!m_statusBarIcon);
-
-  m_statusBarIcon = [[MacOSStatusIconDelegate alloc] init];
+  NSStatusItem* item = [[NSStatusBar systemStatusBar] statusItemWithLength:NSSquareStatusItemLength];
+  item.visible = YES;
+  m_statusItem = [item retain];
 }
 
 MacOSStatusIcon::~MacOSStatusIcon() {
   MZ_COUNT_DTOR(MacOSStatusIcon);
 
-  logger.debug() << "Remove delegate";
-  Q_ASSERT(m_statusBarIcon);
-
-  [static_cast<MacOSStatusIconDelegate*>(m_statusBarIcon) dealloc];
-  m_statusBarIcon = nullptr;
+  NSStatusItem* item = static_cast<NSStatusItem*>(m_statusItem);
+  item.menu = nil;
+  [[NSStatusBar systemStatusBar] removeStatusItem:item];
+  [item release];
+  m_statusItem = nullptr;
 }
 
 void MacOSStatusIcon::setIcon(const QString& iconPath) {
-  logger.debug() << "Set icon" << iconPath;
-
-  QResource imageResource = QResource(iconPath);
-  Q_ASSERT(imageResource.isValid());
-
-  [m_statusBarIcon setIcon:imageResource.uncompressedData().toNSData()];
-}
-
-void MacOSStatusIcon::setIndicatorColor(const QColor& indicatorColor) {
-  logger.debug() << "Set indicator color";
-
-  if (!indicatorColor.isValid()) {
-    [m_statusBarIcon setIndicatorColor:[NSColor clearColor]];
+  QResource resource(iconPath);
+  if (!resource.isValid()) {
     return;
   }
 
-  NSColor* color = [NSColor colorWithCalibratedRed:indicatorColor.red() / 255.0f
-                                             green:indicatorColor.green() / 255.0f
-                                              blue:indicatorColor.blue() / 255.0f
-                                             alpha:indicatorColor.alpha() / 255.0f];
-  [m_statusBarIcon setIndicatorColor:color];
-}
-
-void MacOSStatusIcon::setMenu(NSMenu* statusBarMenu) {
-  logger.debug() << "Set menu";
-  [m_statusBarIcon setMenu:statusBarMenu];
+  NSImage* image = [[NSImage alloc] initWithData:resource.uncompressedData().toNSData()];
+  CGFloat side = [[NSStatusBar systemStatusBar] thickness] - 4.0;
+  image.size = NSMakeSize(side, side);
+  static_cast<NSStatusItem*>(m_statusItem).button.image = image;
+  [image release];
 }
 
-void MacOSStatusIcon::setToolTip(const QString& tooltip) {
-  logger.debug() << "Set tooltip";
-  [m_statusBarIcon setToolTip:tooltip.toNSString()];
+void MacOSStatusIcon::setMenu(QMenu* menu) {
+  static_cast<NSStatusItem*>(m_statusItem).menu = menu ? menu->toNSMenu() : nil;
 }
 
 void MacOSStatusIcon::showMessage(const QString& title, const QString& message) {
-  logger.debug() << "Show message";
-
   UNUserNotificationCenter* center = [UNUserNotificationCenter currentNotificationCenter];
-
-  // This is a no-op is authorization has been granted.
   [center requestAuthorizationWithOptions:(UNAuthorizationOptionSound | UNAuthorizationOptionAlert |
                                            UNAuthorizationOptionBadge)
-                        completionHandler:^(BOOL granted, NSError* _Nullable error) {
-                          if (error) {
-                            // Note: This error may happen if the application is not signed.
-                            NSLog(@"Error asking for permission to send notifications %@", error);
-                            return;
-                          }
+                        completionHandler:^(BOOL, NSError*){
                         }];
 
   UNMutableNotificationContent* content = [[UNMutableNotificationContent alloc] init];
-
-  content.title = [title.toNSString() autorelease];
-  content.body = [message.toNSString() autorelease];
+  content.title = title.toNSString();
+  content.body = message.toNSString();
   content.sound = [UNNotificationSound defaultSound];
 
-  UNTimeIntervalNotificationTrigger* trigger =
-      [UNTimeIntervalNotificationTrigger triggerWithTimeInterval:1 repeats:NO];
-
   UNNotificationRequest* request = [UNNotificationRequest requestWithIdentifier:@"amneziavpn"
                                                                         content:content
-                                                                        trigger:trigger];
+                                                                        trigger:nil];
+  [content release];
 
-  [center addNotificationRequest:request
-           withCompletionHandler:^(NSError* _Nullable error) {
-             if (error) {
-               logger.error() << "Local Notification failed" << error;
-             }
-           }];
+  [center addNotificationRequest:request withCompletionHandler:nil];
 }

client/ui/utils/systemTrayNotificationHandler.cpp

@@ -7,7 +7,7 @@
 
 
 #ifdef Q_OS_MAC
-#  include "platforms/macos/macosutils.h"
+#  include "platforms/macos/macosstatusicon.h"
 #endif
 
 #include <QApplication>
@@ -18,13 +18,11 @@
 #include "version.h"
 
 SystemTrayNotificationHandler::SystemTrayNotificationHandler(QObject* parent) :
-    NotificationHandler(parent),
-    m_systemTrayIcon(parent)
-
+    NotificationHandler(parent)
+#ifndef Q_OS_MAC
+    , m_systemTrayIcon(parent)
+#endif
 {
-    m_systemTrayIcon.show();
-    connect(&m_systemTrayIcon, &QSystemTrayIcon::activated, this, &SystemTrayNotificationHandler::onTrayActivated);
-
     m_trayActionShow =  m_menu.addAction(QIcon(":/images/tray/application.png"), tr("Show") + " " + APPLICATION_NAME, this, [this](){
         emit raiseRequested();
     });
@@ -44,11 +42,26 @@ SystemTrayNotificationHandler::SystemTrayNotificationHandler(QObject* parent) :
                                        this,
                                        [&](){ qApp->quit(); });
 
+#ifdef Q_OS_MAC
+    // QSystemTrayIcon::setContextMenu crashes on macOS 14+: its menu-tracking
+    // observer reads -[NSEvent clickCount] off a non-mouse event. Own the
+    // NSStatusItem and attach the native NSMenu instead.
+    m_statusIcon = new MacOSStatusIcon(this);
+    m_statusIcon->setMenu(&m_menu);
+#else
+    m_systemTrayIcon.show();
+    connect(&m_systemTrayIcon, &QSystemTrayIcon::activated, this,
+            &SystemTrayNotificationHandler::onTrayActivated);
     m_systemTrayIcon.setContextMenu(&m_menu);
+#endif
     setTrayState(Vpn::ConnectionState::Disconnected);
 }
 
 SystemTrayNotificationHandler::~SystemTrayNotificationHandler() {
+#ifdef Q_OS_MAC
+    delete m_statusIcon;  // before m_menu: the status item references its NSMenu
+    m_statusIcon = nullptr;
+#endif
 }
 
 void SystemTrayNotificationHandler::setConnectionState(Vpn::ConnectionState state)
@@ -73,20 +86,20 @@ void SystemTrayNotificationHandler::updateWebsiteUrl(const QString &newWebsiteUr
 
 void SystemTrayNotificationHandler::setTrayIcon(const QString &iconPath)
 {
+#ifdef Q_OS_MAC
+    m_statusIcon->setIcon(iconPath);
+#else
     QIcon trayIconMask(QPixmap(iconPath).scaled(128,128));
-#ifndef Q_OS_MAC
     trayIconMask.setIsMask(true);
-#endif
     m_systemTrayIcon.setIcon(trayIconMask);
+#endif
 }
 
 void SystemTrayNotificationHandler::onTrayActivated(QSystemTrayIcon::ActivationReason reason)
 {
-#ifndef Q_OS_MAC
     if(reason == QSystemTrayIcon::DoubleClick || reason == QSystemTrayIcon::Trigger) {
         emit raiseRequested();
     }
-#endif
 }
 
 void SystemTrayNotificationHandler::setTrayState(Vpn::ConnectionState state)
@@ -152,8 +165,13 @@ void SystemTrayNotificationHandler::notify(NotificationHandler::Message type,
                                            int timerMsec) {
   Q_UNUSED(type);
 
+#ifdef Q_OS_MAC
+  Q_UNUSED(timerMsec);
+  m_statusIcon->showMessage(title, message);
+#else
   QIcon icon(ConnectedTrayIconName);
   m_systemTrayIcon.showMessage(title, message, icon, timerMsec);
+#endif
 }
 
 void SystemTrayNotificationHandler::showHideWindow() {