Created attachment 8605 Fixes slack's issue The Slack "desktop" app, which is an electron application actually doesn't set the _NET_WM_ICON to the window resulting in the fallback icon being rendered and this is annoying. So this bug is not in xfwm4 but the fix is to make xfwm4 understand that there's an increasing trend to create these web apps as desktop apps wannabe which if I could I would completely avoid using. The fix will read the WM_CLASS property and extract the instance name from it, then use GtkIconTheme to determine the icon that should be used in place of the missing icon. It appears to have worked and the patch is simple. I wrote this patch because I was happy to finally have a gtk2 free desktop and that was the only thing bothering me (and the same problem with the panel)
Comment on attachment 8605 Fixes slack's issue Thanks for the patch, I'll post my comments below: I'd rather have a "git format-patch" with the usual format pointing back to the bug (check existing git log to see what I mean) with a sign-off by, commit author, etc. >diff -Naur xfwm4-4.13.2/src/display.c xfwm4-4.13.2.slack-icon-bux-fixed/src/display.c >--- xfwm4-4.13.2/src/display.c 2019-05-15 15:35:15.000000000 -0400 >+++ xfwm4-4.13.2.slack-icon-bux-fixed/src/display.c 2019-05-30 16:58:24.654061503 -0400 >@@ -160,6 +160,7 @@ > "_NET_WM_WINDOW_TYPE_UTILITY", > "_NET_WM_WINDOW_TYPE_NOTIFICATION", > "_NET_WORKAREA", >+ "WM_CLASS", > "MANAGER", > "PIXMAP", > "SM_CLIENT_ID", >diff -Naur xfwm4-4.13.2/src/display.h xfwm4-4.13.2.slack-icon-bux-fixed/src/display.h >--- xfwm4-4.13.2/src/display.h 2019-05-12 12:45:20.000000000 -0400 >+++ xfwm4-4.13.2.slack-icon-bux-fixed/src/display.h 2019-05-30 16:52:52.369257999 -0400 >@@ -252,6 +252,7 @@ > NET_WM_WINDOW_TYPE_UTILITY, > NET_WM_WINDOW_TYPE_NOTIFICATION, > NET_WORKAREA, >+ WM_CLASS, It's not needed, it's not used in the patch anywhere, as you use XGetClassHint() which takes care of using the right atom name already. > MANAGER, > PIXMAP,indenttaion > SM_CLIENT_ID, >diff -Naur xfwm4-4.13.2/src/hints.c xfwm4-4.13.2.slack-icon-bux-fixed/src/hints.c >--- xfwm4-4.13.2/src/hints.c 2019-05-15 16:44:58.000000000 -0400 >+++ xfwm4-4.13.2.slack-icon-bux-fixed/src/hints.c 2019-05-30 18:25:31.705949898 -0400 >@@ -456,6 +456,7 @@ > atoms[i++] = display_info->atoms[NET_WM_WINDOW_TYPE_TOOLBAR]; > atoms[i++] = display_info->atoms[NET_WM_WINDOW_TYPE_UTILITY]; > atoms[i++] = display_info->atoms[NET_WORKAREA]; >+ atoms[i++] = display_info->atoms[WM_CLASS]; Ditto, not needed. > /* GTK specific hints */ > atoms[i++] = display_info->atoms[GTK_FRAME_EXTENTS]; > atoms[i++] = display_info->atoms[GTK_HIDE_TITLEBAR_WHEN_MAXIMIZED]; >@@ -852,6 +853,23 @@ > } > > gboolean >+getWindowClass (DisplayInfo *display_info, Window w, gchar **name, gchar **class) >+{ >+ XClassHint hint; >+ int status; >+ status = XGetClassHint (display_info->dpy, w, &hint); The client window might have vanished, better enclose this within an error trap push/pop and check for status as well (see for similar code in the same file for an example) >+ if (status != 0) { nit: code style. >+ *class = g_strdup (hint.res_class); >+ *name = g_strdup (hint.res_name); Would be worth checking for a NULL param here, so we don't need dummy vars if we don't care about the resource name or class name. >+ // Now free the x11 resource nit: code style. >+ XFree (hint.res_class); >+ XFree (hint.res_name); >+ return TRUE; >+ } >+ return FALSE; >+} We already have that info from clientFrame(), so either reuse the data we have or change the exiting code in clientFrame() to use that new function. >+ >+gboolean > getUTF8StringList (DisplayInfo *display_info, Window w, int atom_id, gchar ***str_p, guint *n_items) > { > char *xstr, *ptr; >diff -Naur xfwm4-4.13.2/src/hints.h xfwm4-4.13.2.slack-icon-bux-fixed/src/hints.h >--- xfwm4-4.13.2/src/hints.h 2019-05-12 12:45:20.000000000 -0400 >+++ xfwm4-4.13.2.slack-icon-bux-fixed/src/hints.h 2019-05-30 17:33:28.019601707 -0400 >@@ -298,4 +298,6 @@ > char **); > #endif > >+gboolean getWindowClass (DisplayInfo *, Window, gchar **, gchar **); nit: code style, each param on a separate line. >+ > #endif /* INC_HINTS_H */ >diff -Naur xfwm4-4.13.2/src/icons.c xfwm4-4.13.2.slack-icon-bux-fixed/src/icons.c >--- xfwm4-4.13.2/src/icons.c 2019-05-12 12:45:20.000000000 -0400 >+++ xfwm4-4.13.2.slack-icon-bux-fixed/src/icons.c 2019-05-30 18:44:15.217305112 -0400 >@@ -535,7 +535,22 @@ > return icon; > } > } >- >+ else >+ { >+ gchar *name; >+ gchar *class; If getWindowClass() would checkf ro NULL params, we wouldn't not need for class here. >+ if (getWindowClass (screen_info->display_info, window, &name, &class)) >+ { >+ GtkIconTheme *theme = gtk_icon_theme_get_default (); >+ GdkPixbuf *icon = gtk_icon_theme_load_icon (theme, name, width, 0, NULL); >+ g_free (name);It's >+ g_free (class); >+ if (icon) >+ { >+ return icon; >+ } That all block of code would be better in its own function called from getAppIcon() for clarity. The icon should should be scaled to the desired size as well. >+ } >+ } > return default_icon_at_size (screen_info->gscr, width, height); > } >
Iharob Al Asimi referenced this bugreport in commit 84a3b1455a96f73da83c1c0a3184f5d896086a0b icons: Fallback to resource class name for icons https://git.xfce.org/xfce/xfwm4/commit?id=84a3b1455a96f73da83c1c0a3184f5d896086a0b
Thanks for your patch, I've pushed a much simpler version.
I am glad to help and indeed, this is way simpler.
As mentioned in Bug 15511, comment 9, the icon in tabwin list mode is larger than expected.
Can you please tell me which app is having that problem, slack isn't and I don't use any other electron based app.
Never mind, I just saw that it was insomnia. I'll download it and see if it happens.
(In reply to Iharob Al Asimi from comment #7) > Never mind, I just saw that it was insomnia. I'll download it and see if it > happens. Were you able to reproduce the issue?
No, I will install insomnia again today and take some screenshots.