From e5c62b6346662298c4b303a1b07734b142c45fa6 Mon Sep 17 00:00:00 2001 From: Alexei Date: Mon, 22 Jun 2026 01:44:04 +0300 Subject: [PATCH] fix: use native NSStatusItem for the macOS tray menu On macOS 14+ the status-item menu is tracked out of process, so Qt's QCocoaSystemTrayIcon menu-tracking observer (installed by setContextMenu) fires asynchronously and calls emitActivated(), which reads -[NSEvent clickCount] when NSApp.currentEvent is no longer a mouse event. clickCount then raises NSInternalInconsistencyException, crashing the app the first time the tray menu is opened. Stop using QSystemTrayIcon on macOS and own the NSStatusItem ourselves (MacOSStatusIcon), attaching the menu's backing native NSMenu via QMenu::toNSMenu(). AppKit shows the menu natively - with menu-bar highlight and correct anchoring - without registering the crashing observer, and Qt keeps the NSMenu in sync with the QActions so the existing menu logic (enable/disable, translations) is reused. The icon stays colored per state and notifications go through UNUserNotificationCenter. Windows and Linux keep QSystemTrayIcon unchanged. This also wires up the previously-unused MacOSStatusIcon and removes the orphaned neNotificationHandler.h. Co-Authored-By: Claude Opus 4.8 (1M context) --- client/platforms/macos/macosstatusicon.h | 14 +- client/platforms/macos/macosstatusicon.mm | 187 +++--------------- client/ui/utils/neNotificationHandler.h | 36 ---- .../utils/systemTrayNotificationHandler.cpp | 40 ++-- .../ui/utils/systemTrayNotificationHandler.h | 6 + 5 files changed, 68 insertions(+), 215 deletions(-) delete mode 100644 client/ui/utils/neNotificationHandler.h diff --git a/client/platforms/macos/macosstatusicon.h b/client/platforms/macos/macosstatusicon.h index 3a69dff4ca..bfb8efe717 100644 --- a/client/platforms/macos/macosstatusicon.h +++ b/client/platforms/macos/macosstatusicon.h @@ -5,8 +5,10 @@ #ifndef MACOSSTATUSICON_H #define MACOSSTATUSICON_H -#include #include +#include + +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 diff --git a/client/platforms/macos/macosstatusicon.mm b/client/platforms/macos/macosstatusicon.mm index cea8439c50..cbe5d027e4 100644 --- a/client/platforms/macos/macosstatusicon.mm +++ b/client/platforms/macos/macosstatusicon.mm @@ -4,201 +4,64 @@ #include "macosstatusicon.h" #include "leakdetector.h" -#include "logger.h" + +#include #import #import #import -/** - * 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(m_statusBarIcon) dealloc]; - m_statusBarIcon = nullptr; + NSStatusItem* item = static_cast(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(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(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]; } diff --git a/client/ui/utils/neNotificationHandler.h b/client/ui/utils/neNotificationHandler.h deleted file mode 100644 index b53eda9865..0000000000 --- a/client/ui/utils/neNotificationHandler.h +++ /dev/null @@ -1,36 +0,0 @@ -#ifndef NENOTIFICATIONHANDLER_H -#define NENOTIFICATIONHANDLER_H - -#include "notificationHandler.h" -#include -#include - -class MacOSStatusIcon; - -class NEStatusBarNotificationHandler : public NotificationHandler { - Q_OBJECT -public: - explicit NEStatusBarNotificationHandler(QObject* parent); - ~NEStatusBarNotificationHandler() override; - - void setConnectionState(Vpn::ConnectionState state) override; - void onTranslationsUpdated() override; - -protected: - void notify(Message type, const QString& title, - const QString& message, int timerMsec) override; - -private: - void buildMenu(); - - QMenu m_menu; - MacOSStatusIcon* m_statusIcon; - - QAction* m_actionShow; - QAction* m_actionConnect; - QAction* m_actionDisconnect; - QAction* m_actionVisitWebsite; - QAction* m_actionQuit; -}; - -#endif // NENOTIFICATIONHANDLER_H diff --git a/client/ui/utils/systemTrayNotificationHandler.cpp b/client/ui/utils/systemTrayNotificationHandler.cpp index 8cbcbe67e8..9158cb6223 100644 --- a/client/ui/utils/systemTrayNotificationHandler.cpp +++ b/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 @@ -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() { diff --git a/client/ui/utils/systemTrayNotificationHandler.h b/client/ui/utils/systemTrayNotificationHandler.h index 309b1d64b0..8bb0fe1323 100644 --- a/client/ui/utils/systemTrayNotificationHandler.h +++ b/client/ui/utils/systemTrayNotificationHandler.h @@ -10,6 +10,8 @@ #include #include +class MacOSStatusIcon; + class SystemTrayNotificationHandler : public NotificationHandler { Q_OBJECT @@ -38,7 +40,11 @@ public slots: private: QMenu m_menu; +#ifdef Q_OS_MAC + MacOSStatusIcon* m_statusIcon = nullptr; +#else QSystemTrayIcon m_systemTrayIcon; +#endif QAction* m_trayActionShow = nullptr; QAction* m_trayActionConnect = nullptr;