Electron apps update icons by sending newIconThemePath; gtk-sni-tray continues to look in old path

[cumber]

Ever since I migrated to using SNI tray with indicators, I've had problems with Slack and Signal not updating their icons (they both add graphical flourishes to indicate things like whether there are unread messages or not). When I start them up, the icon in Taffybar correctly shows whatever state they're in at that time, and then never changes.

I noticed that Taffybar's stderr emits messages about failing to get pixbufs, with iconThemePath set to folders like /tmp/.org.chromium.Chromium.<randomstring> that don't exist, exactly when the icon should be changing (new message recieved, or I click on the last unread message, etc). So I did a bit more digging.

In dbus-monitor, when I start up Slack (with my Taffybar SNI tray running), I can see this section:

method call time=1530493694.000590 sender=:1.66 -> destination=:1.71 serial=31 path=/org/ayatana/NotificationItem/Slack1; interface=org.freedesktop.DBus.Properties; member=Get
   string "org.kde.StatusNotifierItem"
   string "IconPixmap"
error time=1530493694.000937 sender=:1.71 -> destination=:1.66 error_name=org.freedesktop.DBus.Error.InvalidArgs reply_serial=31
   string "No such property 'IconPixmap'"
method call time=1530493694.001166 sender=:1.66 -> destination=:1.71 serial=32 path=/org/ayatana/NotificationItem/Slack1; interface=org.freedesktop.DBus.Properties; member=Get
   string "org.kde.StatusNotifierItem"
   string "IconName"
method return time=1530493694.060064 sender=:1.71 -> destination=:1.66 serial=21 reply_serial=32
   variant       string "Slack1_2"
method call time=1530493694.060298 sender=:1.66 -> destination=:1.71 serial=33 path=/org/ayatana/NotificationItem/Slack1; interface=org.freedesktop.DBus.Properties; member=Get
   string "org.kde.StatusNotifierItem"
   string "IconThemePath"
method return time=1530493694.065481 sender=:1.71 -> destination=:1.66 serial=22 reply_serial=33
   variant       string "/tmp/.org.chromium.Chromium.Vke7A3"

:1.66 seems to be Taffybar and :1.71 seems to be Slack. Taffybar asks for a direct pixbuf, which doesn't exist, so then it asks for the icon name and theme path. Sure enough, the theme path mentioned contains a file for the icon name mentioned:

% ls /tmp/.org.chromium.Chromium.Vke7A3
Slack1_2.png

And everything works at this point. The icon is displayed correctly. After a message comes in and the icon should be updated (but actually remains unchanged), Taffybar's log says:

Failed to get pixbuf for ItemInfo {itemServiceName = BusName ":1.71", itemServicePath = ObjectPath "/org/ayatana/NotificationItem/Slack1", iconTitle = "slack", iconName = "Slack1_4", iconThemePath = Just "/tmp/.org.chromium.Chromium.Vke7A3", iconPixmaps = [], menuPath = Just (ObjectPath "/org/ayatana/NotificationItem/Slack1/Menu")}

It's still using /tmp/.org.chromium.Chromium.Vke7A3 as the icon theme path, which no longer exists (Electron cleaned up the tmp folder I guess):

% ls /tmp/.org.chromium.Chromium.Vke7A3
ls: cannot access '/tmp/.org.chromium.Chromium.Vke7A3': No such file or directory

At the same time, dbus-monitor shows this sequence:

signal time=1530494718.862148 sender=:1.71 -> destination=(null destination) serial=42 path=/org/ayatana/NotificationItem/Slack1; interface=org.kde.StatusNotifierItem; member=NewIconThemePath
   string "/tmp/.org.chromium.Chromium.zLsFh7"
signal time=1530494718.862196 sender=:1.71 -> destination=(null destination) serial=43 path=/org/ayatana/NotificationItem/Slack1; interface=org.kde.StatusNotifierItem; member=NewIcon
method call time=1530494718.862513 sender=:1.66 -> destination=:1.71 serial=46 path=/org/ayatana/NotificationItem/Slack1; interface=org.freedesktop.DBus.Properties; member=Get
   string "org.kde.StatusNotifierItem"
   string "IconName"
method return time=1530494718.862868 sender=:1.71 -> destination=:1.66 serial=44 reply_serial=46
   variant       string "Slack1_4"
method call time=1530494718.863173 sender=:1.66 -> destination=:1.71 serial=47 path=/org/ayatana/NotificationItem/Slack1; interface=org.freedesktop.DBus.Properties; member=Get
   string "org.kde.StatusNotifierItem"
   string "IconPixmap"
error time=1530494718.863299 sender=:1.71 -> destination=:1.66 error_name=org.freedesktop.DBus.Error.InvalidArgs reply_serial=47
   string "No such property 'IconPixmap'"

Slack sent a broadcast NewIconThemePath message. And the Slack1_4 icon does exist in that path:

% ls /tmp/.org.chromium.Chromium.zLsFh7                                                                                                                           
Slack1_4.png

As further events happen, the Taffybar log continues to show "Failed to get pixbuf for ItemInfo" messages containing the same iconThemePath, but it seems like Slack generates a new temp folder every time it updates its icon. So It seems that gtk-sni-tray isn't responding to the NewIconThemePath messages.

The Signal desktop app seems to behave exactly the same way. Both are based on the Electron framework, which uses Chromium for the front end, so I presume they're using common code to update their icons which generates the /tmp/.org.chromium.Chromium.*/ paths. This leads me to expect that this will be a problem for Electron-based apps in general.

[colonelpanic8]

@cumber Great job investigating! I have noticed/was aware of this issue, but I haven't gotten around fixing it. The fix is relatively simple though. So I can probably crank it out today.

It actually needs to occur in https://github.com/IvanMalison/status-notifier-item

[colonelpanic8]

@cumber I can't seem to get any electron apps to show an SNI icon at all (I know I used to be able to get discord to show up in Arch, but I can't seem to get it working in NixOS).

Any chance you can test whether or not my changes in https://github.com/taffybar/gtk-sni-tray/tree/handleIconPathUpdate have resolved the issue. Note that there are no actual changes to the gtk-sni-tray code (just to the sha that is used for status-notifier-item) so if you are not building using stack, you will need to figure out how to build against the right version of status-notifier-item.

[cumber]

Oh, yeah, there's multiple layers of things wrong with Slack/Signal's tray icons on my system. :) Nix builds them without appindicator available at build time (I've got an overlay hack; need to file a pull request with nixpkgs) and you need to start them with XDG_CURRENT_DESKTOP=Unity.

I rebuilt overriding status-notifier-item (I'm building using nix, so no need to use the gtk-sni-branch just to get the stack hash). And it does seem to be working now! The icons are updated when I expect them to be.

I still see lots of messages like this in Taffybar's log:

Failed to get pixbuf for ItemInfo {itemServiceName = BusName ":1.74", itemServicePath = ObjectPath "/org/ayatana/NotificationItem/Signal1", iconTitle = "signal-desktop", iconName = "Signal1_3", iconThemePath = Just "/tmp/.org.chromium.Chromium.oPeZUu", iconPixmaps = [], menuPath = Just (ObjectPath "/org/ayatana/NotificationItem/Signal1/Menu")}
Failed to get pixbuf for ItemInfo {itemServiceName = BusName ":1.74", itemServicePath = ObjectPath "/org/ayatana/NotificationItem/Signal1", iconTitle = "signal-desktop", iconName = "Signal1_4", iconThemePath = Just "/tmp/.org.chromium.Chromium.eToBuE", iconPixmaps = [], menuPath = Just (ObjectPath "/org/ayatana/NotificationItem/Signal1/Menu")}
Failed to get pixbuf for ItemInfo {itemServiceName = BusName ":1.74", itemServicePath = ObjectPath "/org/ayatana/NotificationItem/Signal1", iconTitle = "signal-desktop", iconName = "Signal1_4", iconThemePath = Just "/tmp/.org.chromium.Chromium.eToBuE", iconPixmaps = [], menuPath = Just (ObjectPath "/org/ayatana/NotificationItem/Signal1/Menu")}

Now it seems that the iconThemePath folder generally exists, but the icon named doesn't exist in that folder. At a guess, you're getting the NewIconThemePath and processing that immediately, which causes it to look for the previous icon in the new folder, which fails (Electron seems to also renumber the icon file every time it changes, as well as putting it in a new temp folder) so it holds the previous icon, until the NewIcon message comes through with the new icon name and then the icon is updated.

If that's what's going on, it seems totally reasonable behaviour on your part, even though it results in scary errors that don't really matter being reported. I'm not sure what you would do about that... treat NewThemePath as updating where you would look when the icon changes, but don't actually update the icon until a NewIcon is received?? I can just as easily imagine usage patterns where that behaviour is wrong and the current one is correct. :/

(Also getting ahead of myself, since that was just a guess)

[colonelpanic8]

If that's what's going on, it seems totally reasonable behaviour on your part, even though it results in scary errors that don't really matter being reported. I'm not sure what you would do about that... treat NewThemePath as updating where you would look when the icon changes, but don't actually update the icon until a NewIcon is received?? I can just as easily imagine usage patterns where that behaviour is wrong and the current one is correct. :/

I think the easiest thing to do would be to simply ALSO update the IconName whenever the theme path is updated and vice versa.

[colonelpanic8]

@cumber I think I'm going to leave the behavior as is. The warnings about the failure to get pixbuf for Item Info are well understood, but honestly the way electron provides icons is out of spec anyway.

[cumber]

Yeah, to do what they're doing correctly would require that they can atomically send both messages at once (or does the spec provide for an effective NewIconThemeAndIconName message?). It does seem a very weird way to go about providing dynamic icons.

I'm totally fine with that call. The icon updates actually work now, regardless of the "false" errors. Thanks for the quick fix again!

[colonelpanic8]

@cumber Mind sharing your derivation for slack?

[cumber]

Sure thing! https://gist.github.com/cumber/e2dedb807d1a8aec7d4d6ab34f11e378

It's literally just the whole derivation copied from nixpkgs, which is a pain when they update. I'm going to get round to filing a pull request any day now, I just need to figure out if/how to make it conditional (and whether it should default to supporting indicators or not).

[colonelpanic8]

@cumber Do you know about overrideAttrs? Easy way to change parts of a derivation without changing the whole thing.

[cumber]

Yesish. I get a bit confused the difference between overrideDerivation and overrideAttrs, and in this case that combines with my confusion about buildInputs vs nativeBuildInputs (there's some weird thing where they switch places at one point in the layers of helpers used to make derivations? or they used to?).

[colonelpanic8]

You almost always want overrideAttrs, not overrideDerivation. overrideDerivation directly overrides the properties in the derivation file, while overrideAttrs overides the properties of the nix object that is then processed to make that file.

I'm not as sure re nativeBuildInputs, but I thought that mostly had to do with platform dependent stuff, which I don't think is going to be relevant in this (or most) cases.