From a0d983e4b53432b236499cf76638ead4dd3d2c46 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 7 Aug 2025 19:25:50 +0200 Subject: [PATCH] feat: allow macOS tray to maintain position (#47838) * feat: allow macOS tray to maintain position * refactor: just use guid * test: fixup tests * docs: clarify UUID format --- docs/api/tray.md | 14 +++++++- shell/browser/api/electron_api_tray.cc | 22 ++++++++---- shell/browser/api/electron_api_tray.h | 6 ++-- shell/browser/ui/tray_icon.cc | 2 ++ shell/browser/ui/tray_icon.h | 4 ++- shell/browser/ui/tray_icon_cocoa.h | 1 + shell/browser/ui/tray_icon_cocoa.mm | 11 +++++- shell/browser/ui/tray_icon_linux.cc | 2 +- shell/browser/ui/tray_icon_win.cc | 2 +- shell/browser/ui/win/notify_icon_host.cc | 29 +++++++++++----- shell/browser/ui/win/notify_icon_host.h | 2 +- shell/common/gin_converters/guid_converter.h | 36 +++++++++++++++----- spec/api-tray-spec.ts | 4 +-- 13 files changed, 101 insertions(+), 34 deletions(-) diff --git a/docs/api/tray.md b/docs/api/tray.md index dd62da0e176a..0c7c2e95ac0b 100644 --- a/docs/api/tray.md +++ b/docs/api/tray.md @@ -79,7 +79,15 @@ app.whenReady().then(() => { ### `new Tray(image, [guid])` * `image` ([NativeImage](native-image.md) | string) -* `guid` string (optional) _Windows_ - Assigns a GUID to the tray icon. If the executable is signed and the signature contains an organization in the subject line then the GUID is permanently associated with that signature. OS level settings like the position of the tray icon in the system tray will persist even if the path to the executable changes. If the executable is not code-signed then the GUID is permanently associated with the path to the executable. Changing the path to the executable will break the creation of the tray icon and a new GUID must be used. However, it is highly recommended to use the GUID parameter only in conjunction with code-signed executable. If an App defines multiple tray icons then each icon must use a separate GUID. +* `guid` string (optional) _Windows_ _macOS_ - A unique string used to identify the tray icon. Must adhere to [UUID](https://en.wikipedia.org/wiki/Universally_unique_identifier) format. + +**Windows** + +On Windows, if the executable is signed and the signature contains an organization in the subject line then the GUID is permanently associated with that signature. OS level settings like the position of the tray icon in the system tray will persist even if the path to the executable changes. If the executable is not code-signed then the GUID is permanently associated with the path to the executable. Changing the path to the executable will break the creation of the tray icon and a new GUID must be used. However, it is highly recommended to use the GUID parameter only in conjunction with code-signed executable. If an App defines multiple tray icons then each icon must use a separate GUID. + +**MacOS** + +On macOS, the `guid` is a string used to uniquely identify the tray icon and allow it to retain its position between relaunches. Using the same string for a new tray item will create it in the same position as the previous tray item to use the string. Creates a new tray icon associated with the `image`. @@ -327,6 +335,10 @@ Returns [`Rectangle`](structures/rectangle.md) The `bounds` of this tray icon as `Object`. +#### `tray.getGUID()` _macOS_ _Windows_ + +Returns `string | null` - The GUID used to uniquely identify the tray icon and allow it to retain its position between relaunches, or null if none is set. + #### `tray.isDestroyed()` Returns `boolean` - Whether the tray icon is destroyed. diff --git a/shell/browser/api/electron_api_tray.cc b/shell/browser/api/electron_api_tray.cc index c8494e4b7337..11c462cdf573 100644 --- a/shell/browser/api/electron_api_tray.cc +++ b/shell/browser/api/electron_api_tray.cc @@ -52,10 +52,12 @@ gin::DeprecatedWrapperInfo Tray::kWrapperInfo = {gin::kEmbedderNativeGin}; Tray::Tray(v8::Isolate* isolate, v8::Local image, - std::optional guid) - : tray_icon_(TrayIcon::Create(guid)) { + std::optional guid) + : guid_(guid), tray_icon_(TrayIcon::Create(guid)) { SetImage(isolate, image); tray_icon_->AddObserver(this); + if (guid.has_value()) + tray_icon_->SetAutoSaveName(guid.value().AsLowercaseString()); } Tray::~Tray() = default; @@ -63,19 +65,17 @@ Tray::~Tray() = default; // static gin_helper::Handle Tray::New(gin_helper::ErrorThrower thrower, v8::Local image, - std::optional guid, + std::optional guid, gin::Arguments* args) { if (!Browser::Get()->is_ready()) { thrower.ThrowError("Cannot create Tray before app is ready"); return {}; } -#if BUILDFLAG(IS_WIN) if (!guid.has_value() && args->Length() > 1) { - thrower.ThrowError("Invalid GUID format"); + thrower.ThrowError("Invalid GUID format - GUID must be a string"); return {}; } -#endif // Error thrown by us will be dropped when entering V8. // Make sure to abort early and propagate the error to JS. @@ -392,6 +392,15 @@ gfx::Rect Tray::GetBounds() { return tray_icon_->GetBounds(); } +v8::Local Tray::GetGUID() { + if (!CheckAlive()) + return {}; + auto* isolate = JavascriptEnvironment::GetIsolate(); + if (!guid_) + return v8::Null(isolate); + return gin::ConvertToV8(isolate, guid_.value()); +} + bool Tray::CheckAlive() { if (!tray_icon_) { v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); @@ -424,6 +433,7 @@ void Tray::FillObjectTemplate(v8::Isolate* isolate, .SetMethod("closeContextMenu", &Tray::CloseContextMenu) .SetMethod("setContextMenu", &Tray::SetContextMenu) .SetMethod("getBounds", &Tray::GetBounds) + .SetMethod("getGUID", &Tray::GetGUID) .Build(); } diff --git a/shell/browser/api/electron_api_tray.h b/shell/browser/api/electron_api_tray.h index 94128348f0dd..a0ef6b97cb36 100644 --- a/shell/browser/api/electron_api_tray.h +++ b/shell/browser/api/electron_api_tray.h @@ -45,7 +45,7 @@ class Tray final : public gin_helper::DeprecatedWrappable, // gin_helper::Constructible static gin_helper::Handle New(gin_helper::ErrorThrower thrower, v8::Local image, - std::optional guid, + std::optional guid, gin::Arguments* args); static void FillObjectTemplate(v8::Isolate*, v8::Local); @@ -65,7 +65,7 @@ class Tray final : public gin_helper::DeprecatedWrappable, private: Tray(v8::Isolate* isolate, v8::Local image, - std::optional guid); + std::optional guid); ~Tray() override; // TrayIconObserver: @@ -111,10 +111,12 @@ class Tray final : public gin_helper::DeprecatedWrappable, void SetContextMenu(gin_helper::ErrorThrower thrower, v8::Local arg); gfx::Rect GetBounds(); + v8::Local GetGUID(); bool CheckAlive(); v8::Global menu_; + std::optional guid_; std::unique_ptr tray_icon_; }; diff --git a/shell/browser/ui/tray_icon.cc b/shell/browser/ui/tray_icon.cc index acb1b2fe5c19..ed36cb2f18e1 100644 --- a/shell/browser/ui/tray_icon.cc +++ b/shell/browser/ui/tray_icon.cc @@ -16,6 +16,8 @@ gfx::Rect TrayIcon::GetBounds() { return {}; } +void TrayIcon::SetAutoSaveName(const std::string& name) {} + void TrayIcon::NotifyClicked(const gfx::Rect& bounds, const gfx::Point& location, int modifiers) { diff --git a/shell/browser/ui/tray_icon.h b/shell/browser/ui/tray_icon.h index 057f2d5e9cc6..ad4566105f55 100644 --- a/shell/browser/ui/tray_icon.h +++ b/shell/browser/ui/tray_icon.h @@ -18,7 +18,7 @@ namespace electron { class TrayIcon { public: - static TrayIcon* Create(std::optional guid); + static TrayIcon* Create(std::optional guid); #if BUILDFLAG(IS_WIN) using ImageType = HICON; @@ -99,6 +99,8 @@ class TrayIcon { // Returns the bounds of tray icon. virtual gfx::Rect GetBounds(); + virtual void SetAutoSaveName(const std::string& name); + void AddObserver(TrayIconObserver* obs) { observers_.AddObserver(obs); } void RemoveObserver(TrayIconObserver* obs) { observers_.RemoveObserver(obs); } diff --git a/shell/browser/ui/tray_icon_cocoa.h b/shell/browser/ui/tray_icon_cocoa.h index 9d8e7599eb52..9c85f492571e 100644 --- a/shell/browser/ui/tray_icon_cocoa.h +++ b/shell/browser/ui/tray_icon_cocoa.h @@ -35,6 +35,7 @@ class TrayIconCocoa : public TrayIcon { void CloseContextMenu() override; void SetContextMenu(raw_ptr menu_model) override; gfx::Rect GetBounds() override; + void SetAutoSaveName(const std::string& name) override; base::WeakPtr GetWeakPtr() { return weak_factory_.GetWeakPtr(); diff --git a/shell/browser/ui/tray_icon_cocoa.mm b/shell/browser/ui/tray_icon_cocoa.mm index 0cc6bf4eae65..b267ac026474 100644 --- a/shell/browser/ui/tray_icon_cocoa.mm +++ b/shell/browser/ui/tray_icon_cocoa.mm @@ -11,6 +11,7 @@ #include "base/message_loop/message_pump_apple.h" #include "base/strings/sys_string_conversions.h" #include "base/task/current_thread.h" +#include "base/uuid.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "shell/browser/ui/cocoa/NSString+ANSI.h" @@ -68,6 +69,10 @@ [self setFrame:[statusItem_ button].frame]; } +- (void)setAutosaveName:(NSString*)name { + statusItem_.autosaveName = name; +} + - (void)updateTrackingAreas { // Use NSTrackingArea for listening to mouseEnter, mouseExit, and mouseMove // events. @@ -420,8 +425,12 @@ gfx::Rect TrayIconCocoa::GetBounds() { return gfx::ScreenRectFromNSRect([status_item_view_ window].frame); } +void TrayIconCocoa::SetAutoSaveName(const std::string& name) { + [status_item_view_ setAutosaveName:base::SysUTF8ToNSString(name)]; +} + // static -TrayIcon* TrayIcon::Create(std::optional guid) { +TrayIcon* TrayIcon::Create(std::optional guid) { return new TrayIconCocoa; } diff --git a/shell/browser/ui/tray_icon_linux.cc b/shell/browser/ui/tray_icon_linux.cc index 79107dbb6efb..50b03937babd 100644 --- a/shell/browser/ui/tray_icon_linux.cc +++ b/shell/browser/ui/tray_icon_linux.cc @@ -112,7 +112,7 @@ ui::StatusIconLinux* TrayIconLinux::GetStatusIcon() { } // static -TrayIcon* TrayIcon::Create(std::optional guid) { +TrayIcon* TrayIcon::Create(std::optional guid) { return new TrayIconLinux; } diff --git a/shell/browser/ui/tray_icon_win.cc b/shell/browser/ui/tray_icon_win.cc index 328e83a9d977..9153748f5ea6 100644 --- a/shell/browser/ui/tray_icon_win.cc +++ b/shell/browser/ui/tray_icon_win.cc @@ -8,7 +8,7 @@ namespace electron { // static -TrayIcon* TrayIcon::Create(std::optional guid) { +TrayIcon* TrayIcon::Create(std::optional guid) { static NotifyIconHost host; return host.CreateNotifyIcon(guid); } diff --git a/shell/browser/ui/win/notify_icon_host.cc b/shell/browser/ui/win/notify_icon_host.cc index 2d0263472a9b..2895175e7292 100644 --- a/shell/browser/ui/win/notify_icon_host.cc +++ b/shell/browser/ui/win/notify_icon_host.cc @@ -190,21 +190,32 @@ NotifyIconHost::~NotifyIconHost() { delete ptr; } -NotifyIcon* NotifyIconHost::CreateNotifyIcon(std::optional guid) { - if (guid.has_value()) { - for (NotifyIcons::const_iterator i(notify_icons_.begin()); - i != notify_icons_.end(); ++i) { - auto* current_win_icon = static_cast(*i); - if (current_win_icon->guid() == guid.value()) { - LOG(WARNING) - << "Guid already in use. Existing tray entry will be replaced."; +NotifyIcon* NotifyIconHost::CreateNotifyIcon(std::optional guid) { + std::string guid_str = + guid.has_value() ? guid.value().AsLowercaseString() : ""; + UUID uid = GUID_NULL; + if (!guid_str.empty()) { + if (guid_str[0] == '{' && guid_str[guid_str.length() - 1] == '}') { + guid_str = guid_str.substr(1, guid_str.length() - 2); + } + + unsigned char* uid_cstr = (unsigned char*)guid_str.c_str(); + RPC_STATUS result = UuidFromStringA(uid_cstr, &uid); + if (result != RPC_S_INVALID_STRING_UUID) { + for (NotifyIcons::const_iterator i(notify_icons_.begin()); + i != notify_icons_.end(); ++i) { + auto* current_win_icon = static_cast(*i); + if (current_win_icon->guid() == uid) { + LOG(WARNING) + << "Guid already in use. Existing tray entry will be replaced."; + } } } } auto* notify_icon = new NotifyIcon(this, NextIconId(), window_, kNotifyIconMessage, - guid.has_value() ? guid.value() : GUID_DEFAULT); + uid == GUID_NULL ? GUID_DEFAULT : uid); notify_icons_.push_back(notify_icon); return notify_icon; diff --git a/shell/browser/ui/win/notify_icon_host.h b/shell/browser/ui/win/notify_icon_host.h index b58da039e1d6..68adcfdaa911 100644 --- a/shell/browser/ui/win/notify_icon_host.h +++ b/shell/browser/ui/win/notify_icon_host.h @@ -27,7 +27,7 @@ class NotifyIconHost { NotifyIconHost(const NotifyIconHost&) = delete; NotifyIconHost& operator=(const NotifyIconHost&) = delete; - NotifyIcon* CreateNotifyIcon(std::optional guid); + NotifyIcon* CreateNotifyIcon(std::optional guid); void Remove(NotifyIcon* notify_icon); private: diff --git a/shell/common/gin_converters/guid_converter.h b/shell/common/gin_converters/guid_converter.h index 88dd8cf79262..a2c771efebd2 100644 --- a/shell/common/gin_converters/guid_converter.h +++ b/shell/common/gin_converters/guid_converter.h @@ -7,6 +7,8 @@ #include +#include "base/strings/string_util.h" +#include "base/uuid.h" #include "gin/converter.h" #if BUILDFLAG(IS_WIN) @@ -36,18 +38,40 @@ typedef struct { namespace gin { +template <> +struct Converter { + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + base::Uuid* out) { + std::string guid; + if (!gin::ConvertFromV8(isolate, val, &guid)) + return false; + + base::Uuid parsed = base::Uuid::ParseLowercase(base::ToLowerASCII(guid)); + if (!parsed.is_valid()) + return false; + + *out = parsed; + return true; + } + + static v8::Local ToV8(v8::Isolate* isolate, base::Uuid val) { + const std::string guid = val.AsLowercaseString(); + return gin::ConvertToV8(isolate, guid); + } +}; + +#if BUILDFLAG(IS_WIN) template <> struct Converter { static bool FromV8(v8::Isolate* isolate, v8::Local val, UUID* out) { -#if BUILDFLAG(IS_WIN) std::string guid; if (!gin::ConvertFromV8(isolate, val, &guid)) return false; UUID uid; - if (!guid.empty()) { if (guid[0] == '{' && guid[guid.length() - 1] == '}') { guid = guid.substr(1, guid.length() - 2); @@ -62,12 +86,8 @@ struct Converter { } } return false; -#else - return false; -#endif } static v8::Local ToV8(v8::Isolate* isolate, UUID val) { -#if BUILDFLAG(IS_WIN) const GUID GUID_NULL = {}; if (val == GUID_NULL) { return v8::String::Empty(isolate); @@ -75,11 +95,9 @@ struct Converter { std::wstring uid = base::win::WStringFromGUID(val); return StringToV8(isolate, base::SysWideToUTF8(uid)); } -#else - return v8::Undefined(isolate); -#endif } }; +#endif } // namespace gin diff --git a/spec/api-tray-spec.ts b/spec/api-tray-spec.ts index f56ba1e695b3..b1f0147bef12 100644 --- a/spec/api-tray-spec.ts +++ b/spec/api-tray-spec.ts @@ -30,13 +30,13 @@ describe('tray module', () => { }).to.throw(/Failed to load image from path (.+)/); }); - ifit(process.platform === 'win32')('throws a descriptive error if an invalid guid is given', () => { + ifit(process.platform !== 'linux')('throws a descriptive error if an invalid guid is given', () => { expect(() => { tray = new Tray(nativeImage.createEmpty(), 'I am not a guid'); }).to.throw('Invalid GUID format'); }); - ifit(process.platform === 'win32')('accepts a valid guid', () => { + ifit(process.platform !== 'linux')('accepts a valid guid', () => { expect(() => { tray = new Tray(nativeImage.createEmpty(), '0019A433-3526-48BA-A66C-676742C0FEFB'); }).to.not.throw();