Skip to content
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

Remove permissions to unbreak the tray icon #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guihkx
Copy link

@guihkx guihkx commented Jan 2, 2024

NOTE: This is not the proper fix by any means; It's only a workaround.

This PR removes both --talk-name=org.kde.StatusNotifierWatcher and --socket=wayland permissions to unbreak the tray icon functionality.

The root cause likely needs to be addressed in this third-party library that Spotube uses to set up the tray icon.

Also see antler119/system_tray#67 (comment) for more details.


Using dbus-monitor, we can figure out how Spotube is setting the tray icon:

$ dbus-monitor
...
   array [
      dict entry(
         string "Id"
         variant             string "cac90a20-a943-11ee-86a7-d9e05156d7eb"
      )
      dict entry(
         string "Category"
         variant             string "ApplicationStatus"
      )
      dict entry(
         string "Status"
         variant             string "Active"
      )
      dict entry(
         string "IconName"
         variant             string "/app/spotube/data/flutter_assets/assets/spotube-logo.png"
      )
...

The problem here is that the path for the icon file in the IconName property does not exist outside of the Flatpak sandbox, and therefore the icon file cannot be found or set.

By removing the --talk-name=org.kde.StatusNotifierWatcher permission, however, xdg-desktop-portal takes care of this problem for us automagically.

At least on KDE, instead of forwarding the literal path of the icon, only the pixmap data of the icon is sent, through the IconPixmap option:

$ dbus-monitor
...
   array [
      dict entry(
         string "Category"
         variant             string "ApplicationStatus"
      )
      dict entry(
         string "IconPixmap"
         variant             array [
               struct {
                  int32 16
                  int32 16
                  array of bytes [
                     ff 00 00 00 ff 00 00 00 ff 00 00 00 ff 00 00 00 ff 00 00
                     [ ... output shortened for readability ... ]
                  ]
               }
            ]
      )
...

Although this fixes things on X11 sessions, that's still not enough for Wayland, so that's why it's needed to also remove the Wayland permission so that XWayland is used instead.

Fixes KRTirtho/spotube#541

@flathubbot
Copy link
Contributor

Started test build 90988

@flathubbot
Copy link
Contributor

Build 90988 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/73671/com.github.KRTirtho.Spotube.flatpakref

@callegar
Copy link

callegar commented Jan 3, 2024

With this test build I am still not getting the system tray icon. The main difference is that I am not even getting the empty space. So the situation is actually worsened. Seen on Manjaro/Flatpak/KDE.

@guihkx
Copy link
Author

guihkx commented Jan 3, 2024

I'm not sure why that's happening for you, but I'll say that I tested this on Arch Linux, with three different DEs, and the results are consistent.

The stable build fails to display the icon on all DEs, but the test build (i.e., the build from this pull request) actually displays it:

KDE

kde.mp4

GNOME

gnome.mp4

XFCE

xfce.mp4

@callegar
Copy link

callegar commented Jan 4, 2024

When I start the application with flatpak run, I get the following:

(process:2): GLib-GObject-CRITICAL **: 10:12:55.529: g_object_new_is_valid_property: object class 'MyApplication' has no property named 'com.github.KRTirtho.Spotube'

(spotube:2): Gdk-CRITICAL **: 10:12:57.562: gdk_window_get_state: assertion 'GDK_IS_WINDOW (window)' failed

** (spotube:2): CRITICAL **: 10:12:58.285: Failed to read XDG desktop portal settings: GDBus.Error:org.freedesktop.portal.Error.NotFound: Requested setting not found

** (spotube:2): CRITICAL **: 10:12:58.287: Failed to read XDG desktop portal settings: GDBus.Error:org.freedesktop.portal.Error.NotFound: Requested setting not found

** (spotube:2): CRITICAL **: 10:12:58.288: Failed to read XDG desktop portal settings: GDBus.Error:org.freedesktop.portal.Error.NotFound: Requested setting not found

** (spotube:2): CRITICAL **: 10:12:58.289: Failed to read XDG desktop portal settings: GDBus.Error:org.freedesktop.portal.Error.NotFound: Requested setting not found

** (spotube:2): CRITICAL **: 10:12:58.289: Failed to read XDG desktop portal settings: GDBus.Error:org.freedesktop.portal.Error.NotFound: Requested setting not found
package:media_kit_libs_linux registered.
flutter: media_kit: WARNING: package:media_kit_native_event_loop not found.
flutter: Configure AudioServiceLinux.
method call InitSystemTray
SystemTray::set_system_tray_info title: , icon_path: /app/spotube/data/flutter_assets/assets/spotube-logo.png, toolTip: (null)
method call CreateContextMenu
value_to_menu_item type:label, label:Show/Hide
value_to_menu_item type:label, label:Play/Pause
value_to_menu_item type:label, label:Next
value_to_menu_item type:label, label:Previous
value_to_menu_item type:label, label:Quit
method call SetContextMenu
method call DestroySystemTray
method call InitSystemTray
SystemTray::set_system_tray_info title: , icon_path: /app/spotube/data/flutter_assets/assets/spotube-logo.png, toolTip: (null)
method call CreateContextMenu
value_to_menu_item type:label, label:Show/Hide
value_to_menu_item type:label, label:Play/Pause
value_to_menu_item type:label, label:Next
value_to_menu_item type:label, label:Previous
value_to_menu_item type:label, label:Quit
flutter: setQueue() has not been implemented.
flutter: Get org.mpris.MediaPlayer2.Player.Volume not implemented
flutter: GetPosition(): 0:00:00.000000

(spotube:2): Gtk-CRITICAL **: 10:12:59.834: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
method call SetContextMenu
method call DestroySystemTray
sh: line 1: xdg-mime: command not found

(spotube:2): Gtk-CRITICAL **: 10:13:00.575: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
method call InitSystemTray

(spotube:2): Gtk-CRITICAL **: 10:13:00.578: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
SystemTray::set_system_tray_info title: , icon_path: /app/spotube/data/flutter_assets/assets/spotube-logo.png, toolTip: (null)
method call DestroySystemTray

(spotube:2): Gtk-CRITICAL **: 10:13:00.582: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
method call InitSystemTray

(spotube:2): Gtk-CRITICAL **: 10:13:00.584: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
SystemTray::set_system_tray_info title: , icon_path: /app/spotube/data/flutter_assets/assets/spotube-logo.png, toolTip: (null)
method call DestroySystemTray

(spotube:2): Gtk-CRITICAL **: 10:13:00.589: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
method call InitSystemTray

(spotube:2): Gtk-CRITICAL **: 10:13:00.591: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
SystemTray::set_system_tray_info title: , icon_path: /app/spotube/data/flutter_assets/assets/spotube-logo.png, toolTip: (null)
method call CreateContextMenu

Apparently, I am getting a path to the icon, not the icon data.

@callegar
Copy link

callegar commented Jan 4, 2024

Using xdg-desktop-portal-kde 5.27.10-1

@guihkx
Copy link
Author

guihkx commented Jan 4, 2024

Apparently, I am getting a path to the icon, not the icon data.

If you're saying that based only on the output of the program, that's definitely not it.

If you watch any of the videos I posted, I also get a similar output. The "conversion" is done by the current xdg-desktop-portal implementation (in your case, it should be xdg-desktop-portal-kde).

Can you verify that you don't have any permission overrides that might be interfering with this? You can check with:

flatpak override --user --show com.github.KRTirtho.Spotube

And also global overrides:

flatpak override --user --show

Also, can you verify that the KDE portal service is running correctly?

systemctl --user status plasma-xdg-desktop-portal-kde.service

@callegar
Copy link

callegar commented Jan 4, 2024

Trying to show overrides gives nothing.

Checking the portal service give:

systemctl --user status plasma-xdg-desktop-portal-kde.service
● plasma-xdg-desktop-portal-kde.service - Xdg Desktop Portal For KDE
Loaded: loaded (/usr/lib/systemd/user/plasma-xdg-desktop-portal-kde.service; static)
Active: active (running) since Thu 2024-01-04 10:16:25 CET; 22min ago
Main PID: 3082 (xdg-desktop-por)
Tasks: 9 (limit: 37549)
Memory: 24.4M
CPU: 271ms
CGroup: /user.slice/user-1000.slice/user@1000.service/session.slice/plasma-xdg-desktop-portal-kde.service
└─3082 /usr/lib/xdg-desktop-portal-kde

Jan 04 10:33:53 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported
Jan 04 10:33:53 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported
Jan 04 10:33:53 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported
Jan 04 10:33:53 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.a11y.interface" is not supported
Jan 04 10:33:53 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported
Jan 04 10:34:59 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported
Jan 04 10:34:59 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported
Jan 04 10:34:59 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported
Jan 04 10:34:59 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.a11y.interface" is not supported
Jan 04 10:34:59 zorrykid xdg-desktop-portal-kde[3082]: xdp-kde-settings: Namespace "org.gnome.desktop.interface" is not supported

@guihkx
Copy link
Author

guihkx commented Jan 4, 2024

Damn, everything seems in order... :/

One last thing I can think of, and this is a long shot, but maybe the icon is being automatically hidden here:

image

Or manually hidden here:

image

If it's neither of those, then I'd perhaps try to debug this by leaving dbus-monitor running while toggling the 'Show system tray icon' option in Spotube's settings and see what kind of information comes from it.

@callegar
Copy link

callegar commented Jan 4, 2024

Not hidden. In fact, not appearing at all in "Entries".
One question: should some other portal be installed too, together with the kde one?

@callegar
Copy link

callegar commented Jan 4, 2024

Tried dbus-monitor. Weird enough I cannot seem to be able to grep neither IconPixmap nor IconName in the output.

@callegar
Copy link

callegar commented Jan 4, 2024

Wanted to test on another machine but now getting a 404 from https://dl.flathub.org/build-repo/73671/com.github.KRTirtho.Spotube.flatpakref

@guihkx
Copy link
Author

guihkx commented Jan 4, 2024

One question: should some other portal be installed too, together with the kde one?

Nope. You should only need one implementation.

Tried dbus-monitor. Weird enough I cannot seem to be able to grep neither IconPixmap nor IconName in the output.

I'm at a loss then, lol

Wanted to test on another machine but now getting a 404

I'll trigger a rebuild.

bot, build

@flathubbot
Copy link
Contributor

Queued test build for com.github.KRTirtho.Spotube.

@flathubbot
Copy link
Contributor

Started test build 91492

@flathubbot
Copy link
Contributor

Build 91492 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/74174/com.github.KRTirtho.Spotube.flatpakref

@callegar
Copy link

callegar commented Jan 4, 2024

Same on the other host (that is also KDE/Wayland/Manjaro, though).
Installed the non-flatpak generic binary that works fine.

@guihkx
Copy link
Author

guihkx commented Jan 4, 2024

Can you test it on a X11 session just out of curiosity? I didn't test on Wayland since I have a really old NVIDIA GPU.

@flathubbot
Copy link
Contributor

Queued test build for com.github.KRTirtho.Spotube.

@flathubbot
Copy link
Contributor

Started test build 91760

@flathubbot
Copy link
Contributor

Build 91760 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/74449/com.github.KRTirtho.Spotube.flatpakref

This permission allows us to communicate directly with the
org.kde.StatusNotifierWatcher interface, but this is actually a bad
idea.

Previously, when the app was about to set the tray icon, the
org.kde.StatusNotifierWatcher interface would receive the following
information:

$ dbus-monitor
...
   array [
      dict entry(
         string "Id"
         variant             string "cac90a20-a943-11ee-86a7-d9e05156d7eb"
      )
      dict entry(
         string "Category"
         variant             string "ApplicationStatus"
      )
      dict entry(
         string "Status"
         variant             string "Active"
      )
      dict entry(
         string "IconName"
         variant             string "/app/spotube/data/flutter_assets/assets/spotube-logo.png"
      )
...

The problem here is that the path for the icon file in the "IconName"
property does not exist outside of the Flatpak sandbox, and therefore
the icon file cannot be found or set.

By removing this permission, however, the current xdg-desktop-portal
implementation should take care of this problem for us automagically.

At least on KDE, instead of forwarding the literal path of the icon,
only the pixmap data of the icon is sent, through the "IconPixmap"
option:

$ dbus-monitor
...
   array [
      dict entry(
         string "Category"
         variant             string "ApplicationStatus"
      )
      dict entry(
         string "IconPixmap"
         variant             array [
               struct {
                  int32 16
                  int32 16
                  array of bytes [
                     ff 00 00 00 ff 00 00 00 ff 00 00 00 ff 00 00 00 ff 00 00
                     [ ... output shortened for readability ... ]
                  ]
               }
            ]
      )
...

Unfortunately, for reasons unbeknownst to me, simply removing the
org.kde.StatusNotifierWatcher permission is still not enough to make
the tray icon appear on Wayland sessions.

However, removing the Wayland permission to make the app run through
XWayland, did the trick.
@flathubbot
Copy link
Contributor

Started test build 92931

@flathubbot
Copy link
Contributor

Build 92931 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/75613/com.github.KRTirtho.Spotube.flatpakref

@guihkx
Copy link
Author

guihkx commented Jan 12, 2024

I have set up a QEMU virtual machine to test this change on Wayland, and in fact the tray icon didn't show up at all on any of the Wayland sessions.

I'm not sure why that is, but for now, the best solution I found is to disable Wayland support in the app, so that it runs through XWayland instead...

@C-512L
Copy link

C-512L commented Jan 24, 2024

Maybe an approach like bbhtt/net.sourceforge.liferea@ec88547 would be better for fixing this.

@guihkx
Copy link
Author

guihkx commented Jan 24, 2024

Maybe an approach like bbhtt/net.sourceforge.liferea@ec88547 would be better for fixing this.

Sure, but there's currently no Flutter SDK on Flathub, and so the application can't be easily patched to be built from source.

Not only that, but the tray icon is set up by this third-party library (and arguably incorrectly as well), so we'd have to directly patch the library.

For now, I believe the solution I found is better than having an invisible tray icon.

@C-512L
Copy link

C-512L commented Jan 25, 2024

Maybe an approach like bbhtt/net.sourceforge.liferea@ec88547 would be better for fixing this.

Sure, but there's currently no Flutter SDK on Flathub, and so the application can't be easily patched to be built from source.

Not only that, but the tray icon is set up by this third-party library (and arguably incorrectly as well), so we'd have to directly patch the library.

For now, I believe the solution I found is better than having an invisible tray icon.

Another option could be to contribute it upstream as a workaround and put it behind an environment variable for detection (i.e FLATPAK=1), which I have seen a few apps do it.

@ninja-
Copy link

ninja- commented Feb 22, 2024

on gnome, removing org.kde.StatusNotifierWatcher with flatseal results in no tray icon at all

@guihkx
Copy link
Author

guihkx commented Feb 22, 2024

on gnome, removing org.kde.StatusNotifierWatcher with flatseal results in no tray icon at all

Did you remove the Wayland permission as well?

@guihkx guihkx changed the title Remove permission that breaks the tray icon Remove permissions to unbreak the tray icon Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tray icon has no image and is improperly captalized
5 participants