Last Comment Bug 496653 - Command line option --class <WM_CLASS> does not work, proposed MOZ_WM_CLASS workaround
: Command line option --class <WM_CLASS> does not work, proposed MOZ_WM_CLASS w...
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: All Linux
: -- major with 4 votes (vote)
: mozilla14
Assigned To: Martin Stránský
:
Mentors:
Depends on: 776325
Blocks: 29856 737791
  Show dependency treegraph
 
Reported: 2009-06-05 15:45 PDT by reikred
Modified: 2012-08-16 22:26 PDT (History)
10 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch for Bug 496653, relative to firefox release 10.0.2 source code (1.98 KB, patch)
2012-03-22 00:38 PDT, reikred
no flags Details | Diff | Review
patch with gdk_get_program_class() (3.73 KB, patch)
2012-03-25 23:30 PDT, Martin Stránský
karlt: review+
Details | Diff | Review
wmclass v2 (7.71 KB, patch)
2012-03-26 07:05 PDT, Martin Stránský
karlt: review+
Details | Diff | Review
patch for check-in (7.88 KB, patch)
2012-03-27 01:16 PDT, Martin Stránský
stransky: review+
Details | Diff | Review

Description reikred 2009-06-05 15:45:46 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.4) Gecko/2008120411 Minefield/3.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.4) Gecko/2008120411 Minefield/3.0.4

In Firefox 1.x and 2.x, one could use the argument --class BLAH to force a user-defined value into the WM_CLASS property of the X11 windows created by the browser. In Firefox 3.x, --class does nothing.



Reproducible: Always

Steps to Reproduce:
firefox -class Firefox5

Actual Results:  
xprop will show that WM_CLASS of any window is still Gecko or Minefield or some such.


Expected Results:  
It should have said WM_CLASS was "Firefox5", as I asked for that value to be set.

A simple workaround can be made so that I can use an environment variable MOZ_WM_CLASS to force the value. The diff is

### diff -w nsXULWindow.cpp old/nsXULWindow.cpp.ORG
1340,1342d1339
<   char* env_moz_wm_class= getenv("MOZ_WM_CLASS");
<   if (env_moz_wm_class != NULL) windowType.AssignASCII(env_moz_wm_class);
< 

I may try to get this workaround into the source tree, but since I am not a well known entity in Mozilla circles I'm not sure I have the clout to get it approved.

An even better fix would be to get "--class WM_CLASS" working again, but I just could not figure out how to do it.

Example of suing the workaround fix:

env MOZ_WM_CLASS=Firefox5 firefox
Comment 1 reikred 2009-06-05 15:47:24 PDT
The diff was generated from Firefox version 3.0.10 source tree.
Comment 2 Antti Kaihola 2010-02-26 20:19:19 PST
The -class/--class option still doesn't work in a Firefox 3.6.2 nightly build from this week:

Mozilla/5.0 (X11; U; Linux i686; fi-FI; rv:1.9.2.2pre) Gecko/20100223 Ubuntu/10.04 (lucid) Namoroka/3.6.2pre
Comment 3 reikred 2010-10-05 17:46:47 PDT
The problem still exists in Firefox 4b6.
Comment 4 reikred 2010-10-06 12:15:21 PDT
Unfortunately the file .../mozilla/xpfe/appshell/src/nsXULWindow.cpp has changed so much between 3.0.10 and 4b6 that the MOZ_WM_CLASS hack described above has to be modified.

But isn't it about time we got a real fix for this? What do people know about how WM_CLASS is supposed to get set in firefox 4?

There is this interesting bug

https://bugzilla.redhat.com/show_bug.cgi?id=445543#c12

Looks to me that /usr/share/applications/mozilla-firefox.desktop line
StartupWMClass=Firefox-bin (or whatever)
does not work either.
Comment 5 reikred 2010-10-06 12:32:13 PDT
Another WM_CLASS bug, related to the StartupWMClass=Firefox-bin (or whataver value) setting in the firefox.desktop file being ignored.
Comment 6 reikred 2010-10-07 19:23:59 PDT
(In reply to comment #5)
> Another WM_CLASS bug, related to the StartupWMClass=Firefox-bin (or whataver
> value) setting in the firefox.desktop file being ignored.

https://bugzilla.mozilla.org/show_bug.cgi?id=494586
Comment 7 Tyler Downer [:Tyler] 2011-05-15 14:23:07 PDT
Reporter, are you still seeing this issue with Firefox 4.0.1 or later in safe mode or a fresh profile? If not, please close. These links can help you in your testing.
http://support.mozilla.com/kb/Safe+Mode
http://support.mozilla.com/kb/Managing+profiles
Comment 8 Marc-Aurèle DARCHE 2011-05-16 00:50:38 PDT
I'm not the reporter but I can confirm that what is reported here is accurate and that this ticket state could be set to NEW:

* The fix to have the "--class WM_CLASS" option has not been done
* A potential workaround using an environment variable "MOZ_WM_CLASS" has not been made either. But note that having the "--class WM_CLASS" option is really what we would like to have

Thank you.
Comment 9 Tyler Downer [:Tyler] 2011-05-30 17:08:25 PDT
No reply, INCOMPLETE. Please retest with Firefox 4 or later and a new profile (http://support.mozilla.com/kb/Managing+profiles). Also, ensure you have the most up to date graphics drivers, operating system updates, and plugin versions (flash, java, etc). If you continue to see this issue with the newest Firefox and a new profile, then please comment on this bug.
Comment 10 Henrik Skupin (:whimboo) 2011-05-31 06:03:21 PDT
Tyler, I assume you closed this bug by accident. If that's the case please adjust your query for mass-closing bugs.

Benjamin, is this still a supported feature or have we removed the --class command line option?
Comment 11 Benjamin Smedberg [:bsmedberg] 2011-05-31 06:06:43 PDT
Unless --class is a feature of GTK/X11 itself, I don't think it's worth supporting.
Comment 12 Marc-Aurèle DARCHE 2011-05-31 06:16:21 PDT
Thanks a lot Henrik.

And yes Benjamin, this is a feature of GTK/X11 itself.

The gtk_window_set_wmclass method is an example of how to achieve this, but, as stated in the linked doc below, the "wmclass" should be not be modified afterward but be set at the right value the first time. This example was just to show you that the wmclass concept is very well known and supported by window managers in the X11/X.org world:
http://developer.gnome.org/gtk/2.24/GtkWindow.html#gtk-window-set-wmclass
Comment 13 Benjamin Smedberg [:bsmedberg] 2011-05-31 06:17:46 PDT
I mean: --display is a standard command line argument eaten by gtk_init. If --class is also a standard argument, then we should support it. If it's not, I don't see any reason to add support for it.
Comment 14 Marc-Aurèle DARCHE 2011-05-31 06:21:33 PDT
(In reply to comment #13)
> I mean: --display is a standard command line argument eaten by gtk_init. If
> --class is also a standard argument, then we should support it. If it's not,
> I don't see any reason to add support for it.

OK understood.

Could at least this bug be reopened and even assigned to be or something similar? And I'll try to give a definitive answer regarding a --class option and try to provide a patch.
Comment 15 Benjamin Smedberg [:bsmedberg] 2011-05-31 07:01:17 PDT
No. Currently there is no bug to fix, AIUI.
Comment 16 Marc-Aurèle DARCHE 2011-05-31 07:06:27 PDT
(In reply to comment #15)
> No. Currently there is no bug to fix, AIUI.

As stated in the description: "In Firefox 1.x and 2.x, one could use the argument --class BLAH to force a user-defined value into the WM_CLASS property of the X11 windows created by the browser. In Firefox 3.x, --class does nothing." Doesn't it make it a regression, hence a bug to fix?
Comment 17 Benjamin Smedberg [:bsmedberg] 2011-05-31 07:52:09 PDT
It may be a change we made (accidentally or on purpose) during code refactoring. It is not a feature I wish to support, which is why this is resolved WONTFIX.
Comment 18 reikred 2011-08-30 12:53:51 PDT
But this command line is a standard gtk+/gdk  feature that is common to all gtk+/gdk programs.
Here is a reference:

http://developer.gnome.org/gtk/stable/gtk-running.html

Quote:

All GTK+ applications support a number of standard commandline
options. These are removed from argv by gtk_init(). Modules may parse
and remove further options. The X11 and Windows GDK backends parse some
additional commandline options.
...

The following options are really used by GDK, not by GTK+, but we list them here for completeness nevertheless.

--class class.  Sets the program class; see gdk_set_program_class().
...
Comment 19 Benjamin Smedberg [:bsmedberg] 2011-08-30 13:00:30 PDT
If it is implemented by gtk_init, then why doesn't it work already? We use gtk_init.
Comment 20 reikred 2011-08-30 19:41:25 PDT
Benjamin: I think this is the key quote:

"The X11 and Windows GDK backends parse some additional commandline options"

So what is missing is gdk_init() NOT gtk_init() and POSSIBLY also the specific call gdk_set_program_class() unless that gets taken care of inside gdk_init.

If I were better at google code search I'd find exactly where this is done in google-chrome or chromium browser (where --class option still works). That could serve  as a blueprint for firefox to use. Can someone help? I can only find other examples such as from this search:

http://www.google.com/codesearch#search/&q=%20gdk_init&type=cs
Comment 21 Karl Tomlinson (ni?:karlt) 2012-03-21 18:34:23 PDT
The widget backend is (probably inappropriately) overriding the class with gtk_window_set_wmclass and XSetClassHint.
Comment 22 reikred 2012-03-21 22:09:12 PDT
Karl,

Yes, that is exactly what is happening. In fact I have been working on a patch to forefox 10.0.2 that fixes the problem. I have tested it and it works well. Give me a few minutes and I will try and make a real patch file and post it.
Comment 23 reikred 2012-03-22 00:38:50 PDT
Created attachment 608252 [details] [diff] [review]
Patch for Bug 496653, relative to firefox release 10.0.2 source code

This bug has pestered me for years, literally, and I finally found the time (6 days of work) to dig sufficiently into the source code to fix it, hopefully once and for all, rather than the hacks and workarounds I have been compiling for my own use. I'm by no means an expert on Mozilla, but I have been very motivated, out of necessity, to get this done, so I hope the effort is appreciated.

Details: The fix affects only one file, namely (sourcedir)/xpfe/appshell/src/nsXULWindow.cpp

To get the file to compile, I had to hack the compilation recipe and add the following flags:

> -I/usr/include/gtk-2.0\
> -I/usr/lib/gtk-2.0/include\
> -I/usr/include/cairo\
> -I/usr/include/gdk-pixbuf-2.0\
> -I/usr/include/pango-1.0\
> -I/usr/include/glib-2.0\
> -I/usr/lib/glib-2.0/include\

I am also no expert on GNU autotools, so I ask if someone please can update Makefile.in or .deps/nsXULWindow.pp or whatever is the file that needs to be fixed to get the necessary includes.
Comment 24 Henrik Skupin (:whimboo) 2012-03-22 00:46:56 PDT
Comment on attachment 608252 [details] [diff] [review]
Patch for Bug 496653, relative to firefox release 10.0.2 source code

Thanks reikred for working on this. It's really appreciated. So to get it finally into the code base of Firefox you should create a patch against the mozilla-central repository. See 'Get the source' on the following MDN page:

https://developer.mozilla.org/En/Simple_Firefox_build

Once it has been done review has to be requested from a peer of the appropriate sub module. You can check other bugs in the same component on Bugzilla to find the right person. You can't set r+ on your own patches.

Feel free to ask if something is unclear. Thanks again for your help!
Comment 25 Henrik Skupin (:whimboo) 2012-03-22 00:50:59 PDT
Do we know which change for Firefox 3.0 has been caused this? Would you mind testing the 3.0 beta builds which can be found here: ftp://ftp.mozilla.org/pub/firefox/releases/ It would be a starting point to figure out the causing checkin.
Comment 26 reikred 2012-03-22 09:29:51 PDT
(In reply to Henrik Skupin (:whimboo) from comment #24)
> Comment on attachment 608252 [details] [diff] [review]
> Patch for Bug 496653, relative to firefox release 10.0.2 source code
> 
> Thanks reikred for working on this. It's really appreciated. So to get it
> finally into the code base of Firefox you should create a patch against the
> mozilla-central repository. See 'Get the source' on the following MDN page:
> 
> https://developer.mozilla.org/En/Simple_Firefox_build
> 
> Once it has been done review has to be requested from a peer of the
> appropriate sub module. You can check other bugs in the same component on
> Bugzilla to find the right person. You can't set r+ on your own patches.
> 
> Feel free to ask if something is unclear. Thanks again for your help!

Henrik, I have already done a complete build and tested the patch within a local copy of the Firefox 10.0.2 source tree. Are you saying you want me to download a NEWER version of the src (10.0.X or whatever mozilla-central is)? I suppose I could, but couldn't someone who has that source tree already try the patch and see if the diffs look sensible? 

Then there is the autotools|autoconf|./configure|Makefile.in|whatever that needs to be updated. I spent several hours trying to figure this out but I had no success. What needs to be done to get the additional -I flags in? Rerunning ./configure certainly did not help, and recreating ./configure bombed. I don't know what to do nor how to do it.

If someone can lend a hand here that would be great.
Comment 27 Karl Tomlinson (ni?:karlt) 2012-03-22 16:48:14 PDT
This will be easier to fix in nsWindow.cpp.  That file already finds the necessary include files.

I think it was a bug for the class to be given the localized brand name in bug 335068.  See bug 737791 about that.

I suggest using gdk_get_program_class() instead of the brandName when calling gtk_window_set_wmclass and XSetClassHint.

(If it ends up that the default from g_get_prgname() is not suitable, we can gdk_set_program_class() in nsAppRunner before gtk_parse_args().)
Comment 28 Martin Stránský 2012-03-22 23:37:13 PDT
I have a patch for that, will attach it soon.
Comment 29 reikred 2012-03-23 00:51:47 PDT
Karl and Martin,

I recall that I have tried before to fix the problem in nsWindow.cpp rather than nsXULWindow.cpp, but when I did that, nsXULWIndow.cpp still "won" and WM_CLASS did not get the value I was trying to set. Not sure if that is because nsWindow.cpp does not get used for most windows or for some other reason.

You both seem to know quite a bit about this bug, but I would just like to report the observation that I made that hacking nsWindow.cpp did not have the expected effect.

This was a while back, but the code I tested was something like the following:

//nsWIndow.cpp line 3952 from firefox-10.0.2 source tree
        nsXPIDLString brandName;
        GetBrandName(brandName);
        NS_ConvertUTF16toUTF8 cBrand(brandName);

//test hack start
// not a complete fix, just trying to see if an override via env 
// MOZ_WM_CLASS will stick when applied from this location. Answer: It did not 
// stick, nsXULWIndow.cpp still "won", for unknown reasons.
        char* env_moz_wm_class= getenv("MOZ_WM_CLASS");
        if (env_moz_wm_class == NULL) {
            printf("MOZ_WM_CLASS not in evironment\n");
        } else {
            printf("MOZ_WM_CLASS=|%s|\n", env_moz_wm_class);
            brandName.AssignASCII(env_moz_wm_class); //this appears to work
        }
        NS_ConvertUTF16toUTF8 cBrand(brandName);
// test hack end
// the following is for context
        if (mWindowType == eWindowType_dialog) {
            mShell = gtk_window_new(GTK_WINDOW_TOPLEVEL);
            SetDefaultIcon();
            gtk_window_set_wmclass(GTK_WINDOW(mShell), "Dialog", cBrand.get());
            gtk_window_set_type_hint(GTK_WINDOW(mShell),
                                     GDK_WINDOW_TYPE_HINT_DIALOG);
            gtk_window_set_transient_for(GTK_WINDOW(mShell),
                                         topLevelParent);
            mTransientParent = topLevelParent;
//.......etc etc etc
Comment 30 Martin Stránský 2012-03-25 23:30:22 PDT
Created attachment 609230 [details] [diff] [review]
patch with gdk_get_program_class()

A patch with gdk_get_program_class() + unification of the MOZ_X11 code.
Comment 31 Karl Tomlinson (ni?:karlt) 2012-03-26 00:39:59 PDT
Comment on attachment 609230 [details] [diff] [review]
patch with gdk_get_program_class()

Looks good, thanks.

I don't know whether or not it is important to also fix up the gtk_window_set_wmclass calls in nsWindow::Create.
Comment 32 Martin Stránský 2012-03-26 07:05:08 PDT
Created attachment 609309 [details] [diff] [review]
wmclass v2

It's not needed but let's be consistent.
Comment 33 Karl Tomlinson (ni?:karlt) 2012-03-26 15:21:24 PDT
Comment on attachment 609309 [details] [diff] [review]
wmclass v2

Please wrap lines where necessary to stay within 80 columns

>+  class_hint->res_class = (char *)res_class;

and use const_cast<char*>(res_class) here.
Comment 34 Martin Stránský 2012-03-27 01:16:41 PDT
Created attachment 609650 [details] [diff] [review]
patch for check-in

Fixed.
Comment 35 Ryan VanderMeulen [:RyanVM] 2012-03-27 16:18:41 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a6f316cea6a
Comment 36 Ed Morley [:emorley] 2012-03-28 14:02:25 PDT
https://hg.mozilla.org/mozilla-central/rev/7a6f316cea6a
Comment 37 reikred 2012-08-16 11:58:47 PDT
Gentlemen, I have downloaded firefox 14.0.1 and tested the bugfix, and I can report that the bug IS STILL THERE. If I specify --class Firefox2, then I end up with (from xprop in X11)

WM_CLASS(STRING) = "Navigator", "Firefox2"

It should be

WM_CLASS(STRING) = "Firefox2", "Firefox2"

as it was in Firefox 3.x long ago before this bug was introduced. 

One interesting thing did happen: The very first time I fired  up the new browser (14.0.1 installation), there was initially one window that had the correct WM_CLASS setting.
However, that window disappeared after firefox had checked for old plugins (etc),
and all the new windows popped by firefox (old session starting up) has one wrong value again.

Please note carefully the following: As I have mentioned before in this thread, it is likely nsXULWIndow.cpp that is the cause of new windows getting  the "Navigator" moniker rather than the Firefox2 class name that I asked for.
Comment 38 Mike Hommey [:glandium] 2012-08-16 12:13:45 PDT
--class is not supposed to change both the resource name and resource class.

Try, for example, gimp --class foo...
Comment 39 reikred 2012-08-16 14:38:54 PDT
(In reply to Mike Hommey [:glandium] from comment #38)
> --class is not supposed to change both the resource name and resource class.
> 
> Try, for example, gimp --class foo...

Okay, but then the problem is something else, namely that Firefox does not properly implement the GTK standard option called --name=<name>. I should be able to say

firefox --name Firefox2

and get the desired result. To use your example,

gimp --name foo --class bar

does the right thing, as can be seen from running xprop on it:

WM_CLASS(STRING) = "foo", "bar"
Comment 40 Mike Hommey [:glandium] 2012-08-16 22:26:25 PDT
(In reply to reikred from comment #39)
> (In reply to Mike Hommey [:glandium] from comment #38)
> > --class is not supposed to change both the resource name and resource class.
> > 
> > Try, for example, gimp --class foo...
> 
> Okay, but then the problem is something else, namely that Firefox does not
> properly implement the GTK standard option called --name=<name>. I should be
> able to say
> 
> firefox --name Firefox2
> 
> and get the desired result. To use your example,
> 
> gimp --name foo --class bar
> 
> does the right thing, as can be seen from running xprop on it:
> 
> WM_CLASS(STRING) = "foo", "bar"

Then that's another bug (which I don't think is worth fixing).

Note You need to log in before you can comment on or make changes to this bug.