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

Notification Support for Briar Desktop Flatpak #11

Open
willow446 opened this issue Oct 24, 2022 · 9 comments
Open

Notification Support for Briar Desktop Flatpak #11

willow446 opened this issue Oct 24, 2022 · 9 comments

Comments

@willow446
Copy link

Hello! When running the flatpak version of Briar Desktop, the notification feature added in 0.3.1-beta doesn't appear to work on Fedora 36. When running the 0.3.1-beta .jar version of Briar Desktop, it is able to properly connect to libnotify and send notifications, which leads me to believe this is an issue specific to the flatpak version.

@Mikaela
Copy link

Mikaela commented Oct 24, 2022

I think the flatpak is missing permission --talk-name=org.freedesktop.Notifications (maybe --talk-name=org.kde.StatusNotifierWatcher as im.riot.Riot says that is required) and should probably somehow bundle libnotify.

When comparing to other flatpaks (by locate libnotify|grep flatpak), multiple others pop up such as chat.delta.desktop, com.visualstudio.code, org.claws_mail.Claws-Mail, org.videolan.VLC and more after running sudo updatedb, the files libnotify.so, libnotify.so.4 and libnotify.so.4.0.0 are present.

VLC includes it in modules[], but their file is very long and I am not confident enough of that being the right way to attempt opening a PR

https://github.com/flathub/org.videolan.VLC/blob/f61c033fccf7c834c8776c6d40a7212000d9366b/org.videolan.VLC.json#L754-L767

Mikaela added a commit to Mikaela/org.briarproject.Briar that referenced this issue Feb 14, 2023
@A6GibKm
Copy link

A6GibKm commented Feb 17, 2023

No, --talk-name=org.freedesktop.Notifications is not needed with libnotify > 0.8, but yes it needs to be bundled for notifications to work. Note that additionally the desktop file needs a line X-GNOME-UsesNotifications=true to work in GNOME.

EDIT: All of this is assuming the app actually uses libnotify to send notifications.

Mikaela added a commit to Mikaela/org.briarproject.Briar that referenced this issue Feb 17, 2023
@Mikaela
Copy link

Mikaela commented Feb 18, 2023

Upstream issue with potential to resolve this: Send notifications directly through dbus instead of using libnotify? (#487)

@A6GibKm
Copy link

A6GibKm commented Feb 18, 2023

There is nothing wrong using libnotify, its how they load it from a specific .so file thats problematic. This issue should be fixed on flathub side, they way they load it is unfortunate but not an issue in practice.

@sebkur
Copy link

sebkur commented Feb 18, 2023

I still wonder if there's a way to know which exact version of the .so file to load on the app's side so that we could implement this in a way that makes modifications on the flathub side unnecessary.

@A6GibKm
Copy link

A6GibKm commented Feb 18, 2023

Modifications on flathub side are necessary, there are two modifications needed and only one is actionable upstreams:

  • Package libnotify: FDO does not provide libnotify so it has to be packaged here.

  • Add a flag to the desktop file: What upstreams could do here is ship the desktop file themselves and add the X-GNOME-UsesNotifications=trueline which is missing in the one shipped here, desktop files are a linux standard and flatpak apps are encouraged to upstream these.

Regarding the .so file, .4 is the latest one and given the history of libnotify, its maintenance, and its purpose, it is unlikely that it will ever need a new version. I personally don't know the kotlin situation, but in other languages we would use a language binding and use whatever the system has installed.

EDIT: The way the .so is loaded is not optimal, but is unrelated to this issue.

@muelli muelli closed this as completed in f406a20 Feb 18, 2023
@A6GibKm
Copy link

A6GibKm commented Feb 18, 2023

In the MR neither of

  - --talk-name=org.freedesktop.Notifications
  - --talk-name=org.kde.StatusNotifierWatcher

are necessary, please remove them. It also missed adding X-GNOME-UsesNotifications=true to the desktop file.

@muelli muelli reopened this Feb 19, 2023
@MTRNord
Copy link
Contributor

MTRNord commented Feb 20, 2023

In the MR neither of

  - --talk-name=org.freedesktop.Notifications
  - --talk-name=org.kde.StatusNotifierWatcher

are necessary, please remove them. It also missed adding X-GNOME-UsesNotifications=true to the desktop file.

Could you share some details on why that's not needed? I didn't find good explanations so far for that. The X-GNOME-UsesNotifications=true seems logical but tbh kinda unnecessary, looking at the gnome docs for it. (as in, why does gnome need it if everyone else does not)

@A6GibKm
Copy link

A6GibKm commented Feb 20, 2023

Could you share some details on why that's not needed?

libnotify used to be a library to interact with the org.freedesktop.Notifications DBus interface, now it talks with the portal instead if sandboxed. All flatpak apps can talk to the portals so not permission is needed. I don't know whats org.kde.StatusNotifierWatcher, but libnotify does not use it and a code search in gitlab reveals that briar is also not calling it, at least not directly.

The X-GNOME-UsesNotifications=true seems logical but tbh kinda unnecessary, looking at the gnome docs for it. (as in, why does gnome need it if everyone else does not)

So when using the notification portal, the user can request to not display notifications in a per-app basis or the DE might reject a notification request from an app (for example if an app which is unknown to the system requests to display a notification, the request will be ignored in GNOME), for what I recall GNOME takes this variable into consideration. It is also displayed as metainfo in gnome-software and we use this variable to determine whether to show the app in GNOME Settings permission panel. If you test that it works in GNOME, then I guess you can remove it I guess.

Please see https://docs.gtk.org/gio/class.Notification.html and https://developer.gnome.org/documentation/tutorials/notifications.html for more information.

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 a pull request may close this issue.

6 participants