Closed
Bug 335068
Opened 18 years ago
Closed 18 years ago
Firefox and Thunderbird windows grouped under "Mozilla" rather than in separate groups
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: tuukka.tolvanen, Assigned: tuukka.tolvanen)
References
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
7.65 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
In gnome window list 2.12.3, firefox and thunderbird windows are now all grouped under "Mozilla" rather than separately. STR: 1. start Firefox and Thunderbird 2. open new firefox windows until window grouping occurs Expected: Firefox windows grouped as e.g. "Firefox", not including Thunderbird windows Actual: fx & tbird windows grouped into one "Mozilla" group regression 2006-04-08-04-trunk firefox & 05-trunk thunderbird -- 2006-04-10-04-trunk firefox & thunderbird blaming bug 29856 from the sound of it and the regression window
This should fix it.
Assignee: jag → roc
Status: NEW → ASSIGNED
Tuukka, can you verify that this patch fixes the regression for you?
Assignee | ||
Comment 4•18 years ago
|
||
It doesn't... If I change "Mozilla" in http://lxr.mozilla.org/mozilla/source/xpfe/appshell/src/nsXULWindow.cpp#1345 that's the window group name I get.
do you feel like whipping up a patch to replace that with the name taken from nsXULAppInfo? (see http://lxr.mozilla.org/mozilla/source/chrome/src/nsChromeRegistry.cpp#2080 )
Assignee | ||
Comment 6•18 years ago
|
||
with appshell in tier 9 and toolkit incl. xulapp in tier 50 the code from nsChromeRegistry won't quite work out as such...
Hmm. There's got to be a way to do this. We might have to move nsIXULAppInfo somewhere else.
Assignee | ||
Comment 8•18 years ago
|
||
would getting brandShortName from brand.properties w/ nsIStringBundle be ok? that wfm for fx and tb anyhow... bsmedberg mentioned he'd been thinking about moving nsIXULAppInfo up into tier 9 somewhere
Yes, brandShortName sounds like a great solution. If you can't get the brandShortName then just use "Mozilla".
Assignee | ||
Comment 10•18 years ago
|
||
here we go ...mind you, as this causes the apps to group separately, the label shown in the window list is no longer picked up from WM_CLASS.res_class, but from WM_CLIENT_LEADER's WM_NAME, which makes it "Firefox" even when brandShortName is something else like "Minefield". That's consistent with the pre-regression behaviour though. One question is what to do when windowType turns out blank -- this patch leaves it as is such that no action is taken, but that may leave res_class inconsistent with other windows of the app.
Assignee: roc → tuukka.tolvanen
Attachment #219827 -
Attachment is obsolete: true
Attachment #220848 -
Flags: review?(roc)
Assignee | ||
Comment 11•18 years ago
|
||
> from WM_CLIENT_LEADER's WM_NAME, which makes it "Firefox" even when
...as far as I can tell by poking with xprop, anyway; didn't see that string elsewhere.
I think you should set the appName even if windowType is empty. Also I'd like the whole body (apart from "return NS_OK;") to be inside #ifdef MOZ_WIDGET_GTK2, so non-GTK2 platforms don't eat the cost of this (admittedly small). Thanks!
Assignee | ||
Comment 13•18 years ago
|
||
Attachment #220848 -
Attachment is obsolete: true
Attachment #220858 -
Flags: review?(roc)
Attachment #220848 -
Flags: review?(roc)
Is an empty string for res_name and role OK?
Assignee | ||
Comment 15•18 years ago
|
||
what would you like them to fall back on, besides "Popup" and "Toplevel"?
Blocks: 29856
How about setting res_name to "Mozilla" and just not calling gdk_window_set_role if role is empty? I really don't know this X WM stuff.
Assignee | ||
Comment 17•18 years ago
|
||
Sets res_class to brandShortName rather than the "Mozilla" placeholder already with the gtk_window_set_wmclass calls before realize, so if the xul windowtype turns out missing, nothing needs to be done later. Role is only set if there is a xul windowtype. Adds the early gtk_window_set_wmclass for eWindowType_dialog which was omitted from the patch from bug 29856 (why?) -- otherwise dialogs without xul windowtype are left with the good old "Gecko", "Appname-bin". Narrows SetWindowClass back to one arg since the other one needs to be figured out before SetWindowClass anyway.
Attachment #220858 -
Attachment is obsolete: true
Attachment #228652 -
Flags: review?(roc)
Attachment #220858 -
Flags: review?(roc)
Comment on attachment 228652 [details] [diff] [review] patch3 Thanks!
Attachment #228652 -
Flags: superreview+
Attachment #228652 -
Flags: review?(roc)
Attachment #228652 -
Flags: review+
I can't check anything in for a while, my laptop died and I'm travelling.
Comment 21•18 years ago
|
||
mozilla/widget/src/xpwidgets/nsBaseWidget.h 1.82 mozilla/widget/src/xpwidgets/nsBaseWidget.cpp 1.146 mozilla/xpfe/appshell/src/nsXULWindow.cpp 1.170 mozilla/widget/src/gtk2/nsWindow.h 1.63 mozilla/widget/src/gtk2/nsWindow.cpp 1.176 mozilla/widget/public/nsIWidget.h 3.150
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
You need to log in
before you can comment on or make changes to this bug.
Description
•