The default bug view has changed. See this FAQ.

Command line option --class <WM_CLASS> does not work, proposed MOZ_WM_CLASS workaround

RESOLVED FIXED in mozilla14

Status

()

Core
Widget: Gtk
--
major
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: reikred, Assigned: Martin Stránský)

Tracking

({regression})

Trunk
mozilla14
All
Linux
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

8 years ago
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
(Reporter)

Comment 1

8 years ago
The diff was generated from Firefox version 3.0.10 source tree.
(Reporter)

Updated

8 years ago
Hardware: x86 → All
Version: unspecified → 3.0 Branch

Comment 2

7 years ago
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
(Reporter)

Comment 3

7 years ago
The problem still exists in Firefox 4b6.
(Reporter)

Comment 4

7 years ago
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.
(Reporter)

Comment 5

7 years ago
Another WM_CLASS bug, related to the StartupWMClass=Firefox-bin (or whataver value) setting in the firefox.desktop file being ignored.
(Reporter)

Comment 6

7 years ago
(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
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
Whiteboard: [CLOSEME 2011-05-30]

Comment 8

6 years ago
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.
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.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → INCOMPLETE
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?
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INCOMPLETE → ---
Unless --class is a feature of GTK/X11 itself, I don't think it's worth supporting.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → WONTFIX

Comment 12

6 years ago
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
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

6 years ago
(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.
No. Currently there is no bug to fix, AIUI.

Comment 16

6 years ago
(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?
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.
(Reporter)

Comment 18

6 years ago
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().
...
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
If it is implemented by gtk_init, then why doesn't it work already? We use gtk_init.
(Reporter)

Comment 20

6 years ago
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
The widget backend is (probably inappropriately) overriding the class with gtk_window_set_wmclass and XSetClassHint.
Component: General → Widget: Gtk
Product: Firefox → Core
QA Contact: general → gtk
Whiteboard: [CLOSEME 2011-05-30]
Version: 3.0 Branch → Trunk
Blocks: 29856
Keywords: regression
(Reporter)

Comment 22

5 years ago
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.
(Reporter)

Comment 23

5 years ago
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.
Attachment #608252 - Flags: review+
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!
Attachment #608252 - Flags: review+
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.
Assignee: nobody → reikred
Status: REOPENED → ASSIGNED
Keywords: regressionwindow-wanted
(Reporter)

Comment 26

5 years ago
(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.
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().)
(Assignee)

Comment 28

5 years ago
I have a patch for that, will attach it soon.
(Reporter)

Comment 29

5 years ago
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
(Assignee)

Comment 30

5 years ago
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.
Attachment #609230 - Flags: review?(karlt)
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.
Attachment #609230 - Flags: review?(karlt) → review+
(Assignee)

Comment 32

5 years ago
Created attachment 609309 [details] [diff] [review]
wmclass v2

It's not needed but let's be consistent.
Attachment #609230 - Attachment is obsolete: true
Attachment #609309 - Flags: review?(karlt)
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.
Attachment #609309 - Flags: review?(karlt) → review+
(Assignee)

Comment 34

5 years ago
Created attachment 609650 [details] [diff] [review]
patch for check-in

Fixed.
Attachment #609309 - Attachment is obsolete: true
Attachment #609650 - Flags: review+
Attachment #609650 - Flags: checkin?
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Attachment #608252 - Attachment is obsolete: true
Attachment #609650 - Flags: checkin?
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a6f316cea6a
Assignee: reikred → stransky
Flags: in-testsuite?
Keywords: checkin-needed, regressionwindow-wanted
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/7a6f316cea6a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED
Blocks: 737791
Depends on: 776325
(Reporter)

Comment 37

5 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
--class is not supposed to change both the resource name and resource class.

Try, for example, gimp --class foo...
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 39

5 years ago
(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"
(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).
You need to log in before you can comment on or make changes to this bug.