GTK: Set WMCLASS in a way docks notice while still solving display issues on XFCE.

GTK: Set WMCLASS in a way docks notice while still solving display issues on XFCE.

BUG=20587
TEST=Check xprop on the main window.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=82581

[Elliot Glaysher, 2011-04-21 20:44:43]

trevi55: review for functionality
evan: stamp it since he isn't a reviewer the commit queue would recognize.

It looks like some of the confusion here was caused by me noticing that XFCE
displays the third parameter as a display name. Our wmclass for apps aren't user
displayable strings and xfwm4 doesn't seem to really do any advanced window
grouping so I think it's safe to hack the xfwm case so that there's a pretty
string?

(I *still* don't really think I understand what's going on or what's expected.)

[Evan Martin, 2011-04-21 20:54:33]

I don't like the XFCE hack.  Can you experiment/trace a bit more?

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
File chrome/browser/ui/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
chrome/browser/ui/gtk/browser_window_gtk.cc:336: // a dock or application based
behaviour so do what looks good.
Relevant docs:
http://developer.gimp.org/api/2.0/gtk/GtkWindow.html#gtk-window-set-wmclass says
"class" and "name" hints.

In the GTK source, these make their way into:

  GdkWindowAttr attributes;
  ...
  attributes.wmclass_name = window->wmclass_name;
  attributes.wmclass_class = window->wmclass_class;

Plumbs through to X,
          class_hint = XAllocClassHint ();
          class_hint->res_name = attributes->wmclass_name;
          class_hint->res_class = attributes->wmclass_class;
          XSetClassHint (xdisplay, xid, class_hint);

Which uses this structure
http://tronche.com/gui/x/xlib/ICC/client-to-window-manager/wm-class.html#XCla...

Which says that it's setting WM_CLASS, not WM_NAME.  Perhaps we need to set
WM_NAME explicitly so XFCE doesn't get confused?

[Evan Martin, 2011-04-21 20:55:08]

+derat, because he loves underspecified ICCCM snafu

[Evan Martin, 2011-04-21 20:56:45]

E.g. gnome-terminal has WM_NAME that is my shell prompt, but its WM_CLASS is
gnome-terminal.  Does XFCE display "gnome-terminal" when it's run?

[Marco Trevisan (Treviño), 2011-04-21 21:02:29]

You can also summarize the wmclass both as wmclass_name and wmclass_class if
XFCE needs it...

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
File chrome/browser/ui/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
chrome/browser/ui/gtk/browser_window_gtk.cc:333:
base::nix::DESKTOP_ENVIRONMENT_XFCE) {
If you prefer you can also mix these two cases into the single (as it was at the
beginninng):

gtk_window_set_wmclass(window_, wmclassname.c_str(), wmclassname.c_str());

[Daniel Erat, 2011-04-21 21:10:28]

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
File chrome/browser/ui/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
chrome/browser/ui/gtk/browser_window_gtk.cc:336: // a dock or application based
behaviour so do what looks good.
On 2011/04/21 20:54:34, Evan Martin wrote:
> Relevant docs:
> http://developer.gimp.org/api/2.0/gtk/GtkWindow.html#gtk-window-set-wmclass
says
> "class" and "name" hints.
> 
> In the GTK source, these make their way into:
> 
>   GdkWindowAttr attributes;
>   ...
>   attributes.wmclass_name = window->wmclass_name;
>   attributes.wmclass_class = window->wmclass_class;
> 
> Plumbs through to X,
>           class_hint = XAllocClassHint ();
>           class_hint->res_name = attributes->wmclass_name;
>           class_hint->res_class = attributes->wmclass_class;
>           XSetClassHint (xdisplay, xid, class_hint);
> 
> Which uses this structure
>
http://tronche.com/gui/x/xlib/ICC/client-to-window-manager/wm-class.html#XCla...
> 
> Which says that it's setting WM_CLASS, not WM_NAME.  Perhaps we need to set
> WM_NAME explicitly so XFCE doesn't get confused?

(Sorry if I'm stating stuff that you guys already know.)

WM_NAME is the original ICCCM property containing the title that the window
manager should display for a window: http://tronche.com/gui/x/icccm/sec-4.html. 
It is stupid (doesn't specify encoding).

_NET_WM_NAME is the UTF-8 replacement from EWMH:
http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#id2551374

We should (and do, as far as I can see) set both properties.  The "name" hint
referred to in the GTK docs is one half of the WM_CLASS property, not WM_NAME --
see XSetClassHint(3).

I have no idea why a WM would display WM_CLASS as a window's title if
_NET_WM_NAME or WM_NAME is supplied, though.  That sounds wrong.

[Elliot Glaysher, 2011-04-21 21:23:31]

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
File chrome/browser/ui/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
chrome/browser/ui/gtk/browser_window_gtk.cc:336: // a dock or application based
behaviour so do what looks good.
On 2011/04/21 20:54:34, Evan Martin wrote:
> Relevant docs:
> http://developer.gimp.org/api/2.0/gtk/GtkWindow.html#gtk-window-set-wmclass
says
> "class" and "name" hints.
> 
> In the GTK source, these make their way into:
> 
>   GdkWindowAttr attributes;
>   ...
>   attributes.wmclass_name = window->wmclass_name;
>   attributes.wmclass_class = window->wmclass_class;
> 
> Plumbs through to X,
>           class_hint = XAllocClassHint ();
>           class_hint->res_name = attributes->wmclass_name;
>           class_hint->res_class = attributes->wmclass_class;
>           XSetClassHint (xdisplay, xid, class_hint);
> 
> Which uses this structure
>
http://tronche.com/gui/x/xlib/ICC/client-to-window-manager/wm-class.html#XCla...
> 
> Which says that it's setting WM_CLASS, not WM_NAME.  Perhaps we need to set
> WM_NAME explicitly so XFCE doesn't get confused?

It's not setting the window title; xfce's alt-tab switcher displays wm_class /
icon / wm_name as you alt-tab through them. WM_NAME is being set correctly and
is being displayed.

[Elliot Glaysher, 2011-04-21 21:25:17]

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
File chrome/browser/ui/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/6879127/diff/1/chrome/browser/ui/gtk/browser_w...
chrome/browser/ui/gtk/browser_window_gtk.cc:333:
base::nix::DESKTOP_ENVIRONMENT_XFCE) {
On 2011/04/21 21:02:29, Marco Trevisan (Treviño) wrote:
> If you prefer you can also mix these two cases into the single (as it was at
the
> beginninng):
> 
> gtk_window_set_wmclass(window_, wmclassname.c_str(), wmclassname.c_str());

I don't think that helps. The whole point is that wmclassname should never be
presented to the user. XFCE takes wmclass_class and displays it to the user so I
don't want to change the program name there.

[Marco Trevisan (Treviño), 2011-04-21 21:31:26]

On 2011/04/21 21:25:17, Elliot Glaysher wrote:
> I don't think that helps. The whole point is that wmclassname should never be
> presented to the user. XFCE takes wmclass_class and displays it to the user so
I
> don't want to change the program name there.

Ah, ok I didn't know how XFCE was working... I was just saying that to summarize
the code. However other WM needs a class to be set in the proper way now you're
doing it right, at least for libwnck (=> BAMF) based WM (like Unity is)

[Evan Martin, 2011-04-21 21:40:58]

On 2011/04/21 21:31:26, Marco Trevisan (Treviño) wrote:
> Ah, ok I didn't know how XFCE was working... I was just saying that to
summarize
> the code. However other WM needs a class to be set in the proper way now
you're
> doing it right, at least for libwnck (=> BAMF) based WM (like Unity is)

I think we all agree that it needs a class.  :)  What we're trying to figure out
is *which* class setting is used where.

Looking at:
http://tronche.com/gui/x/icccm/sec-4.html#s-4.1.2.5
it seems the two strings are "instance" and "class".

I think Elliot is saying that XFCE displays the "class" (in addition to the real
window title) in some circumstances.
And I think Marco is saying that libwnck needs the "class" to be different to
work.


I think the intended use case for the two strings is when they have there is
that variants of xterm would use something like
("rxvt", "xterm")

But testing locally, if I run 'uxterm' (an xterm variant) I get
WM_CLASS(STRING) = "xterm", "UXTerm"

(!!!)

Elliot, what does uxterm show up as in your alt-tab list?

[Evan Martin, 2011-04-21 21:42:11]

uxterm -name test
shows up as
WM_CLASS(STRING) = "test", "UXTerm"

So I guess it must just use the name 'xterm' by default because that is the
binary that is run or something?  Elliot, can you test that one too?

[Elliot Glaysher, 2011-04-21 21:52:06]

On Thu, Apr 21, 2011 at 2:40 PM,  <evan@chromium.org> wrote:
> But testing locally, if I run 'uxterm' (an xterm variant) I get
> WM_CLASS(STRING) = "xterm", "UXTerm"
>
> Elliot, what does uxterm show up as in your alt-tab list?

Shows up as "UXTERM" (xfce uppercases the class and appears to replace
'_' and '.' with spaces).

For example

xterm -class "one_two.three-four"

shows:

WM_CLASS(STRING) = "xterm", "one_two.three-four"

but comes up in the alt-tab as:

"ONE TWO THREE FOUR" / icon / [my terminal title string]

Setting the -name has no effect on the alt-tab string, which is why I
first assumed that that was where our custom app class should go
there.

> http://codereview.chromium.org/6879127/
>

[Marco Trevisan (Treviño), 2011-04-21 21:54:24]

On 2011/04/21 21:40:58, Evan Martin wrote:
> Looking at:
> http://tronche.com/gui/x/icccm/sec-4.html#s-4.1.2.5
> it seems the two strings are "instance" and "class".
> 
> I think Elliot is saying that XFCE displays the "class" (in addition to the
real
> window title) in some circumstances.
> And I think Marco is saying that libwnck needs the "class" to be different to
> work.

Indeed. Also if I agree that reading the docs, it should use the "instance"
part, but in the real world this doesn't seem to happen.

> I think the intended use case for the two strings is when they have there is
> that variants of xterm would use something like
> ("rxvt", "xterm")
> 
> But testing locally, if I run 'uxterm' (an xterm variant) I get
> WM_CLASS(STRING) = "xterm", "UXTerm"
> 
> (!!!)

This is strange... Since reading the docs the "instance" part seems that should
indicate the binary that generates it (when not overridden by env variable or
arg), while the class part should be more related to the content (that's why
libwnck is using it I guess)...

[Evan Martin, 2011-04-21 22:01:02]

OK, final idea: since we're making a site-specific browser, perhaps it's more
appropriate to put GMAIL.COM in the xcfe alt-tab list?  I know it's a stretch...

[Elliot Glaysher, 2011-04-21 22:14:52]

On Thu, Apr 21, 2011 at 3:01 PM,  <evan@chromium.org> wrote:
> OK, final idea: since we're making a site-specific browser, perhaps it's
> more
> appropriate to put GMAIL.COM in the xcfe alt-tab list?  I know it's a
> stretch...

This would require the major replumbing so we kept the original URL
around that I was talking about in the bug. This is what I tried to do
earlier and reverted. :(

I'm not crazy about using a transformation of the internal appname as
the wmclass, but it's the only piece of data that we reliably have
before the window is created, otherwise I'd love it if we could just
display the real app Name= that we stuff in the .desktop file. The
transformation of more complex URLs doesn't look good. (i.e.
"mail.google.com__mail_u_0", which is what triggered me trying to not
show that string in XFCE).

And what about extension based apps like TweetDeck? That doesn't have
a URL past "chrome-extensions://sha1" and there appname is
"_ext__sha1__". That's a good chunk of the apps in the app store. :(

> http://codereview.chromium.org/6879127/
>

[Evan Martin, 2011-04-21 22:43:53]

LGTM

OK, I think it is fine to work around XFCE like you did then.  Thanks for
trying.

[Evan Martin, 2011-04-21 22:43:53]

LGTM

OK, I think it is fine to work around XFCE like you did then.  Thanks for
trying.