Closed
Bug 496653
Opened 15 years ago
Closed 12 years ago
Command line option --class <WM_CLASS> does not work, proposed MOZ_WM_CLASS workaround
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: reikred, Assigned: stransky)
References
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
7.88 KB,
patch
|
stransky
:
review+
|
Details | Diff | Splinter Review |
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
The diff was generated from Firefox version 3.0.10 source tree.
Comment 2•14 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
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.
Another WM_CLASS bug, related to the StartupWMClass=Firefox-bin (or whataver value) setting in the firefox.desktop file being ignored.
(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•13 years ago
|
||
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•13 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.
Comment 9•13 years ago
|
||
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
Closed: 13 years ago
Resolution: --- → INCOMPLETE
Comment 10•13 years ago
|
||
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 → ---
Comment 11•13 years ago
|
||
Unless --class is a feature of GTK/X11 itself, I don't think it's worth supporting.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → WONTFIX
Comment 12•13 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
Comment 13•13 years ago
|
||
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•13 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.
Comment 15•13 years ago
|
||
No. Currently there is no bug to fix, AIUI.
Comment 16•13 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?
Comment 17•13 years ago
|
||
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•13 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 → ---
Comment 19•13 years ago
|
||
If it is implemented by gtk_init, then why doesn't it work already? We use gtk_init.
Reporter | ||
Comment 20•13 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
Comment 21•12 years ago
|
||
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
Updated•12 years ago
|
Blocks: 29856
Keywords: regression
Reporter | ||
Comment 22•12 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•12 years ago
|
||
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 24•12 years ago
|
||
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+
Comment 25•12 years ago
|
||
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.
Reporter | ||
Comment 26•12 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.
Comment 27•12 years ago
|
||
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•12 years ago
|
||
I have a patch for that, will attach it soon.
Reporter | ||
Comment 29•12 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•12 years ago
|
||
A patch with gdk_get_program_class() + unification of the MOZ_X11 code.
Attachment #609230 -
Flags: review?(karlt)
Comment 31•12 years ago
|
||
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•12 years ago
|
||
It's not needed but let's be consistent.
Attachment #609230 -
Attachment is obsolete: true
Attachment #609309 -
Flags: review?(karlt)
Comment 33•12 years ago
|
||
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•12 years ago
|
||
Fixed.
Attachment #609309 -
Attachment is obsolete: true
Attachment #609650 -
Flags: review+
Attachment #609650 -
Flags: checkin?
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Updated•12 years ago
|
Attachment #608252 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #609650 -
Flags: checkin?
Comment 35•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a6f316cea6a
Assignee: reikred → stransky
Flags: in-testsuite?
Keywords: checkin-needed,
regressionwindow-wanted
Target Milestone: --- → mozilla14
Comment 36•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7a6f316cea6a
Status: ASSIGNED → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 37•12 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 → ---
Comment 38•12 years ago
|
||
--class is not supposed to change both the resource name and resource class. Try, for example, gimp --class foo...
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 39•12 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"
Comment 40•12 years ago
|
||
(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.
Description
•