Waybar: Tray icons fail to update

[Berulacks]

Thanks for all the great work with waybar, loving it so far! Had a small issue to report with the latest version:

While tray icons work fine when first launched, after they get visually updated (e.g. when a notification is received in Slack and the icon normally switches to the Slack icon with a red notification-dot on it) the icon fails. The error I receive is:
Exception: Icon 'Slack1_5' not present in theme Faba-Mono-Dark

I wouldn't expect it to normally look for those icons in the system theme - is this desired/normal behaviour?

Here, for example, is how my bar looks before I check a message:

Here it is after reading all messages in Slack:

Started happening with 0.5.1, was fine earlier; I'm also on Arch Linux if that information helps.

Edit: Switching icon themes does not fix the issue.
Edit 2: Looks like this check (line 224) of item.cpp is what's failing:
if (std::filesystem::exists(icon_name)) {
causing updateImage to fall back to:
image.set(getIconByName(icon_name, icon_size)); which obviously doesn't work because Slack1_5 isn't present in any icon pack

[Alexays]

Thanks for the debug but icon_name isnt a path so it's normal that it fails on this condition :/

[Berulacks]

Yeah, I was wondering how Slack1_5 would evaluate to a path :D Does this mean the issue is with the programs (electron apps) in question?

[Alexays]

Can you try to modify this line (line 216) of item.cpp with image.set_from_icon_name(icon_name, Gtk::ICON_SIZE_MENU);

[Berulacks]

Can you try to modify this line (line 216) of item.cpp with image.set_from_icon_name(icon_name, Gtk::ICON_SIZE_MENU);

Doesn't seem to make a difference. Still getting the following on icon change:

(same error in log)

Edit: I tried set_from_resource and just set - interestingly enough the set_from_* calls always go through without error. If I pop a return statement right after set_pixel_size nothing changes about the behaviour. (I've never played around with this glib stuff before, so I don't know what's the correct behaviour here)

[alebastr]

Looks similar to what I observed with Skype: Electron apps are updating both IconName and IconThemePath to a randomly generated unique values. sni/item.cpp currently does not handle IconThemePath changes.

If you want quick and dirty fix, move following code https://github.com/Alexays/Waybar/blob/24684ca71b7856af888051465e2ec022b48a98a7/src/modules/sni/item.cpp#L48-L51 into the corresponding branch of SNI::Item::setProperty

Now, there is a reason why I did not include this in my PR: there is a single instance of a theme and every update will append to it's path list. I.e. theme object will uncontrollably grow and there's possibility of conflicting icons.
The right way of doing this would be to create an instance of theme for each item that has IconThemePath and replace custom path instead of adding it to the list.

[Berulacks]

Oh god, yeah. You weren't kidding. The icon search path starts filling up with values like:

/tmp/.org.chromium.Chromium.wCVmnp /tmp/.org.chromium.Chromium.wCVmnp /tmp/.org.chromium.Chromium.oJFnze /tmp/.org.chromium.Chromium.oJFnze /tmp/.org.chromium.Chromium.LZU1WM /tmp/.org.chromium.Chromium.LZU1WM

Fun! I'll look into seeing if I can play around with a new theme instance for items with their own IconThemPath, as per your suggestion. Thanks for the interim workaround, though, @alebastr - my tray is happy again.

[Berulacks]

So, I got it working by adding a Glib::RefPtr<Gtk::IconTheme> icon_theme; property to the class, and using that instead of getting the default theme every time we need to perform an operation. I then call the following function every time the IconThemePath property is set:

void
waybar::modules::SNI::Item::updateIconSearchPaths(std::string icon_theme_path) {

    auto search_path = this->icon_theme->get_search_path();
    bool theme_present_in_search_path = std::find(search_path.begin(), search_path.end(), icon_theme_path) != search_path.end();

    if ( !theme_present_in_search_path )
    {
        std::cout << "Path " << icon_theme_path << " is not present in Icon Search Path! Appending..." << std::endl;
        this->icon_theme->append_search_path(icon_theme_path);
    }
}

Two issues, though:

1. Even though we're using a new instance of the current theme, it will still grow constantly as new icons are added - would probably make sense to prune it every once in a while. I could also iterate through the search path and remove any directories that no longer exist?
2. This means that the icon_theme won't respond to updates from the user's end. E.g. if we switch to a them that has a tray icon we won't be able to show that. I wonder if setProperty ever gets called when the IconTheme changes? (Edit: Because I set this->icon_theme using get_default() in the constructor)

[Alexays]

@Berulacks If you have added Glib::RefPtr<Gtk::IconTheme> icon_theme to the class SNI::Item, there is no need to do an append_search_path since we will always have the right path for this icon and we will have only one icon per Item instance, so you can do a set_search_path :)

[Berulacks]

@Berulacks If you have added Glib::RefPtr<Gtk::IconTheme> icon_theme to the class SNI::Item, there is no need to do an append_search_path since we will always have the right path for this icon and we will have only one icon per Item instance, so you can do a set_search_path :)

D'oh. That makes sense :D Having said that, though, I didn't notice that Glib::RefPtr<Gtk::IconTheme> icon_theme was a smart pointer :D Anyone know how to dereference it? Or make a copy of it? Glib docs are... sparse, to say the least

Edit: I was under the impression that if the IconTheme was shared between Items that simply setting the search paths could cause issues with Items that come without their own Icon Search Paths - does that ever happen? :thinking: This Glib stuff really has me scratching my head.

Edit 2: Ah, looks like it was a deliberate design decision from the Glib team to not allow us to dereference RefPtrs. That makes sense, I guess. If that's the case I still think using set_search_path is dangerous: What happens if this IconTheme is instance is used somewhere else? Does Gtk::IconTheme::get_default() create a copy? And could there be tray Items that don't come with their own icon search path property?

[Alexays]

Fixed in 78067462be4f82e93416c1ce98025a8d33289d12 and 57c99dc52650374ac6d1b4f22ad00efb5bd64be7