-
Notifications
You must be signed in to change notification settings - Fork 15k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: reimplement Tray with StatusIconLinuxDbus on Linux #36333
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0aeb3e1
fix: reimplement Tray with StatusIconLinuxDbus on Linux
zcbenz a9ada8d
fix: re-register status item when status watcher is recreated
zcbenz 08ee81a
fix: add missing status_icon_ check
zcbenz 38f8c7e
Merged in branch 'main' to fix mergability
electron-patch-conflict-fixer[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -27,17 +27,14 @@ app.whenReady().then(() => { | |||||||||||||||||||
|
||||||||||||||||||||
__Platform Considerations__ | ||||||||||||||||||||
|
||||||||||||||||||||
If you want to keep exact same behaviors on all platforms, you should not | ||||||||||||||||||||
rely on the `click` event; instead, always attach a context menu to the tray icon. | ||||||||||||||||||||
|
||||||||||||||||||||
__Linux__ | ||||||||||||||||||||
|
||||||||||||||||||||
* On Linux distributions that only have app indicator support, you have to | ||||||||||||||||||||
install `libappindicator1` to make the tray icon work. | ||||||||||||||||||||
* The app indicator will be used if it is supported, otherwise | ||||||||||||||||||||
`GtkStatusIcon` will be used instead. | ||||||||||||||||||||
* App indicator will only be shown when it has a context menu. | ||||||||||||||||||||
* The `click` event is ignored when using the app indicator. | ||||||||||||||||||||
* Tray icon requires support of [StatusNotifierItem](https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/) | ||||||||||||||||||||
in user's desktop environment. | ||||||||||||||||||||
* The `click` event is emitted when the tray icon receives activation from | ||||||||||||||||||||
user, however the StatusNotifierItem spec does not specify which action would | ||||||||||||||||||||
cause an activation, for some environments it is left mouse click, but for | ||||||||||||||||||||
some it might be double left mouse click. | ||||||||||||||||||||
Comment on lines
+34
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
* In order for changes made to individual `MenuItem`s to take effect, | ||||||||||||||||||||
you have to call `setContextMenu` again. For example: | ||||||||||||||||||||
|
||||||||||||||||||||
|
@@ -92,6 +89,9 @@ Returns: | |||||||||||||||||||
|
||||||||||||||||||||
Emitted when the tray icon is clicked. | ||||||||||||||||||||
|
||||||||||||||||||||
Note that on Linux this event is emitted when the tray icon receives an | ||||||||||||||||||||
activation, which might not necessarily be left mouse click. | ||||||||||||||||||||
Comment on lines
+92
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
||||||||||||||||||||
#### Event: 'right-click' _macOS_ _Windows_ | ||||||||||||||||||||
|
||||||||||||||||||||
Returns: | ||||||||||||||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Cheng Zhao <zcbenz@gmail.com> | ||
Date: Tue, 15 Nov 2022 09:38:25 +0900 | ||
Subject: Re-register status item when owner of status watcher is changed | ||
|
||
https://chromium-review.googlesource.com/c/chromium/src/+/4022621 | ||
|
||
diff --git a/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.cc b/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.cc | ||
index f3c9dfa9ca33496a9c45cd0c780d3d629aeb4663..387b59a1015b51690810b90a4ac65df862b337f3 100644 | ||
--- a/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.cc | ||
+++ b/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.cc | ||
@@ -381,6 +381,13 @@ void StatusIconLinuxDbus::OnInitialized(bool success) { | ||
return; | ||
} | ||
|
||
+ watcher_->SetNameOwnerChangedCallback( | ||
+ base::BindRepeating(&StatusIconLinuxDbus::NameOwnerChangedReceived, | ||
+ weak_factory_.GetWeakPtr())); | ||
+ RegisterStatusNotifierItem(); | ||
+} | ||
+ | ||
+void StatusIconLinuxDbus::RegisterStatusNotifierItem() { | ||
dbus::MethodCall method_call(kInterfaceStatusNotifierWatcher, | ||
kMethodRegisterStatusNotifierItem); | ||
dbus::MessageWriter writer(&method_call); | ||
@@ -396,6 +403,14 @@ void StatusIconLinuxDbus::OnRegistered(dbus::Response* response) { | ||
delegate_->OnImplInitializationFailed(); | ||
} | ||
|
||
+void StatusIconLinuxDbus::NameOwnerChangedReceived( | ||
+ const std::string& old_owner, | ||
+ const std::string& new_owner) { | ||
+ // Re-register the item when the StatusNotifierWatcher has a new owner. | ||
+ if (!new_owner.empty()) | ||
+ RegisterStatusNotifierItem(); | ||
+} | ||
+ | ||
void StatusIconLinuxDbus::OnActivate( | ||
dbus::MethodCall* method_call, | ||
dbus::ExportedObject::ResponseSender sender) { | ||
diff --git a/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.h b/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.h | ||
index e7628de42f980fa3535cab9dfffd0deab30f8812..eae1c332a0972aefb8843cac947aeb2f4c48d360 100644 | ||
--- a/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.h | ||
+++ b/chrome/browser/ui/views/status_icons/status_icon_linux_dbus.h | ||
@@ -74,10 +74,16 @@ class StatusIconLinuxDbus : public ui::StatusIconLinux, | ||
const std::string& method_name, | ||
bool success); | ||
void OnInitialized(bool success); | ||
+ void RegisterStatusNotifierItem(); | ||
|
||
// Step 5: register the StatusNotifierItem with the StatusNotifierWatcher. | ||
void OnRegistered(dbus::Response* response); | ||
|
||
+ // Called when the name owner of StatusNotifierWatcher has changed, which | ||
+ // can happen when lock/unlock screen. | ||
+ void NameOwnerChangedReceived(const std::string& old_owner, | ||
+ const std::string& new_owner); | ||
+ | ||
// DBus methods. | ||
// Action -> KDE behavior: | ||
// Left-click -> Activate |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.