flathub / com.github.KRTirtho.Spotube

https://flathub.org/apps/details/com.github.KRTirtho.Spotube
1 stars 8 forks source link

Remove permissions to unbreak the tray icon #51

Closed guihkx closed 2 months ago

guihkx commented 8 months ago

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 https://github.com/antler119/system_tray/issues/67#issuecomment-1879840114 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 https://github.com/KRTirtho/spotube/issues/541

flathubbot commented 8 months ago

Started test build 90988

flathubbot commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

https://github.com/flathub/com.github.KRTirtho.Spotube/assets/626206/f1143d60-259b-421d-9c22-7d442cbb5292

GNOME

https://github.com/flathub/com.github.KRTirtho.Spotube/assets/626206/8a81187a-bc08-4588-97ba-4532b523b47e

XFCE

https://github.com/flathub/com.github.KRTirtho.Spotube/assets/626206/2b67c9db-442d-485f-b6c2-2ffe479ccf26

callegar commented 8 months ago

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 commented 8 months ago

Using xdg-desktop-portal-kde 5.27.10-1

guihkx commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

callegar commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

flathubbot commented 8 months ago

Started test build 91492

flathubbot commented 8 months ago

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 commented 8 months ago

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

guihkx commented 8 months ago

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 commented 8 months ago

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

flathubbot commented 8 months ago

Started test build 91760

flathubbot commented 8 months ago

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
flathubbot commented 8 months ago

Started test build 92931

flathubbot commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

Maybe an approach like https://github.com/bbhtt/net.sourceforge.liferea/commit/ec88547402142230e8690c7ad67217035bbaa1fb would be better for fixing this.

guihkx commented 7 months ago

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 commented 7 months ago

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- commented 6 months ago

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

guihkx commented 6 months ago

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

Did you remove the Wayland permission as well?