ErikReider / SwayNotificationCenter

A simple GTK based notification daemon for SwayWM
GNU General Public License v3.0
1.25k stars 62 forks source link

[Feature] Use `desktop-entry` if present to display icon #447

Closed punk-dev-robot closed 1 month ago

punk-dev-robot commented 2 months ago

Hi, I recently switched to swaync and noticed I don't see almost any app icons on my system. After some investigation, it looks like app_icon is usually empty, but desktop_entry is present.

From the specification: "desktop-entry" STRING This specifies the name of the desktop filename representing the calling program. This should be the same as the prefix used for the application's .desktop file. An example would be "rhythmbox" from "rhythmbox.desktop". This can be used by the daemon to retrieve the correct icon for the application, for logging purposes, etc.

Here is an example notification from Slack:

category:                (null)
image_data:              false
expire_timeout:          -1
replaces_id:             0
desktop_entry:           Slack
body:                   What do you know, it works?
app_icon:
action_icons:            false
time:                   1719756849
hints:
        sender-pid: int64 9071
        urgency: byte 0x01
        desktop-entry: 'Slack'
applied_id:              1
image_path:              (null)
inline-reply:            (null)
app_name:                Slack
summary:                 New message from Slackbot
icon_data:               false
resident:                false
default_action:          Name: View, Id: default
actions:
urgency:                 Normal
punk-dev-robot commented 1 month ago

So after some more investigation I noticed that it is implemented but doesn't always work for me. With the example above, both desktop-entry and app_name are set to Slack, but desktop file provided by application is slack.desktop (lowercase).

If I copy / symlink Slack.desktop to slack.desktop the icon shows. But it's not ideal as it causes duplicate entries in launchers. I wonder if you would consider some other fix, maybe:

willnorris commented 1 month ago

I'm seeing identical behavior, both with Slack as well as Firefox. Firefox is a little weird because as best as I can tell, its notifications really should have a lowercase "firefox", but I'm seeing them with uppercase "Firefox" which is causing the same problem.

I just wrote about it last night (along with the same temporary fix you mention) at https://willnorris.com/til/linux/missing-notify-icons/. One thing I note there is if you exclude the Exec line from your extra desktop-entry, then it shouldn't create duplicates in your launcher.

I don't see this behavior with fnott, but I don't actually see in their source code where they are handling desktop-entry in the notification... perhaps they aren't. I'm not actually sure that treating desktop-entry as case sensitive is necessarily wrong... the spec defines it as "the name of the desktop filename representing the calling program" which I would read as being the exact desktop filename, case and all. But maybe checking the lowercase version of the filename is a reasonable fallback if the non-lowercase version fails to find a desktop file?

ErikReider commented 1 month ago

We already look for the desktop entry, but swaync can't do anything about misbehaving applications that don't provide the correct “.desktop” filename. In the past Discord provided "Discord" as the desktop-entry, but they've fixed it and now provide "com.discordapp.Discord".

I don't really think that there's anything that can be done except for fixing the root issue at the application level…

ErikReider commented 1 month ago

d5cac981 adds lower-case support