Firefox and Thunderbird windows grouped under "Mozilla" rather than in separate groups

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
XUL
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Tuukka Tolvanen (sp3000), Assigned: Tuukka Tolvanen (sp3000))

Tracking

({regression})

Trunk
mozilla1.9alpha1
x86
Linux
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

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

Comment 1

12 years ago
yeah, backing out bug 29856 seems to fix this for me.
Created attachment 219827 [details] [diff] [review]
fix

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

12 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

12 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

12 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

12 years ago
Created attachment 220848 [details] [diff] [review]
patch1: brandShortName -> res_class

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

12 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

12 years ago
Created attachment 220858 [details] [diff] [review]
patch2: with gtk2 ifdefs, allow empty windowType
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

12 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

12 years ago
Created attachment 228652 [details] [diff] [review]
patch3

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+
(Assignee)

Comment 19

12 years ago
roc, checkin?
Whiteboard: [checkin needed]
I can't check anything in for a while, my laptop died and I'm travelling.
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
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
Depends on: 737791
You need to log in before you can comment on or make changes to this bug.