[window] Prefer GTK icon theme over NET_WM_ICON

[Kafva]

I have noticed that the window mode does not pick up certain icons from the icon theme set in gtk-3.0.ini. Looking at window.c I can see that this is to be expected since the class name that is used to look up the icon name is not guaranteed to be correct.

However, currently the NETWM icon seems to override any GTK icons that are found, this patch is meant to address this.

Before/after patch using:

[Settings]
gtk-icon-theme-name=Papirus-Dark

# rofi -show window

(theme)

[DaveDavenport]

I don't think this patch implements correct behavior.
The icon the application explicitly sets for itself should get priority.
(some) Applications use this to relay extra information to the user.

[Kafva]

Oh ye, I assume you mean like an email app setting "X unread messages" in the corner of the icon?
I would personally prefer that the icons always try to follow the theme but I guess that behaviour is worth preserving for other use cases.

[DaveDavenport]

maybe get the applications to use the correct icon theme instead?

[Kafva]

Hmmm ye, I guess the source of the problem isn't really rofi but how my environment sets (or does not set) the application icons that are sent. Definitely seems better to fix that instead if that is the case.

[Kafva]

I tried my patch in a different window manager (awesomewm instead of bspwm) but saw the same behaviour as shown in my prior screenshots.

Were you implying earlier that each individual application (kitty, obs etc.) would need to be patched to provide an icon from the active GTK theme (in queries for their NET_WM_ICON) or is this something that the WM/DE has control over/should handle?

[DaveDavenport]

I have no idea what a window manager has to do with what icon an application set for itself.

I don't know how applications exactly do this, I guess it depends on the application.

But I am not going to change rofi default behavior, that seems correct, because a user finds it 'ugly'.

[Kafva]

Ye, the window manager is probably unrelated 😅

My main gripe with the current behaviour is that the drun mode does show icons from the GTK theme, so it feels inconsistent that another mode shows different icons.

But I am fine with using my fork instead if this is not desirable as the default.

[DaveDavenport]

I don't think you read my reply right. I am not going to change the default behaviour, this does not rule out an config option to change the default behaviour.

[Kafva]

Oh ok, I guess I will reopen the issue then.

[DaveDavenport]

I think gtk icons is not the right name, -window-prefer-icon-theme is better.

[DaveDavenport]

It is not 'gtk theme'. It is the icon theme set, this is not directly dependent on gtk.
Rofi f.e. has it own set icon-theme option, and this will be used when set.

[Kafva]

I changed it to 'icon theme', does it need to be more verbose in some other way?

[DaveDavenport]

Is this logic correct? when config.window_prefer_icon_theme is not set, it will never fall back to the 'class icon' when the application has no net_wm_icon set.
It now always requests this icon, even when not used. This seems wasteful.

[Kafva]

Tried to address this in 4bd7d3b

[DaveDavenport]

Going over the patch, there is a small possible glitching issue. It might show the application set by the application, then the one from the theme.
To fix this an extension needs to be made to rofi_icon_fetcher. I will look at this.

[DaveDavenport]

Pushed the extension to rofi_icon_fetcher and directly implemented this request (this exposed another small issue, so decided to commit the whole).

Can you test?

[DaveDavenport]

   You can prefer the icon theme above the window set icon with the 'prefer-icon-theme' setting:

          configuration {
            window {
                prefer-icon-theme: true;
            }
          }

   or pass -window-prefer-icon-theme true on command line.

[Kafva]

Ye, tested the new option via CLI and from the config file, both seem to work as expected 🙂

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.