194323 - preference dialog blocks other windows of mozilla to be in front of it

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Robin Lu]

On gnome, use the latest metacity ( at least later than Oct.2002 ) as window
manager:
1. launch mozilla
2. edit->preference to open the preference dialog.
3. click the help button

expect:
the help window goes above
result:
the help window is always behind the preference dialog.

This also causes the bug 194150 .

According to http://bugzilla.gnome.org/show_bug.cgi?id=88926 , the dialog
without any denpendence is above the entire application and the preference
dialog does not have the "dependent" feature. Although it is the entire
application's pref dialog, it would be very inconvenient as long as we could
open windows other than dialogs from the pref dialog. 

We could add the "dependent" feature to the pref dialog. It works well and won't
affect invoke the dialog from another window.

Comment 1

[Robin Lu]

Attachment: patch

Comment 2

[timeless]

Comment on attachment 115096 [details] [diff] [review]
patch

this doesn't sound right. please make sure it doesn't break windows or some
other platform.

Comment 3

[Robin Lu]

I have tried my patch on MS Windows. The preference dialog becomes transient for
the window which invoked it. That means, the dialog is always in front of its
transient parent. That does not means modal. I can still manipulate the window.
When minimize the transient parent window, the dialog is minimized as well. If I
chose "preference" from another window of mozilla, the existing preference
dialog will appear but behind the current active window. I think it is
reasonalble for a dialog. 

I do not have other platforms to test with.

Comment 4

[timeless]

Comment on attachment 115096 [details] [diff] [review]
patch

that's not acceptable.

Comment 5

[Dan M]

Note this is bug 112199. There appears to be a genuine problem here but most
people can't see it. I don't see it using gnome & metacity 2.4.0.92-5 (no idea
what date that is). Robin, do you have unusual window manager focus settings?

I'm curious how making the Prefs window dependent/transient helps this
situation. Independent as it is currently, it's just another window. As is the
Help window. Neither should have difficulty being placed above the other.
Binding the prefs window to a browser window doesn't seem like a solution. Again
I'm wondering whether people who see this problem are seeing some fight between
Mozilla and non-standard WM settings.

Should it turn out that making the prefs window dependent is somehow the right
solution from a technical standpoint, I still have to agree with timeless: the
prefs window doesn't logically belong to any one instance of a browser window;
not even the one from which it was opened. I think it's a UI mistake to bind the
prefs window to a browser.

Comment 6

[Kyle Yuan]

ccing hp.

danm, your metacity (2.4.0.92) comes with the RedHat 8.0 that released at the
end of Aug, but hp's fix got checked-in on Sept. If people upgrade their
metacity, they will definitely see this problem.

timeless, please make sure you really know what we are talking about before your
reviewing.

Comment 7

[Havoc Pennington]

The way many apps work is that if you select Preferences again from a 
different toplevel, they reset the dependence/transient parent to the 
new toplevel. That will work fine with metacity for example, 
gnome-terminal does it. Setting the transient parent for dialogs is pretty 
important for this and other reasons (metacity is ultimately intended
to have similar behaviors to what Robin describes on Windows; iconify 
dialog with the main window, etc.)

The reason dialogs are kept on top is that people tend to lose them 
otherwise.

metacity is one of the few window managers to use the transiency relationship
pretty heavily as Windows does, but in principle it has been Correct(tm) to set 
the transient for relationship here ever since the ICCCM was released in around
1994, so it's not really a newfangled thing to do, it's been in the
specification for a long time.

Comment 8

[Dan M]

I'm using metacity 2.4.3 now and I see the bug. Something else is going awry
here. The Preferences window isn't supposed to be transient at all (before the
proposed patch, it's opened without the "dependent" feature flag). Any window
opened with the "dialog" feature flag (like the Preferences window) somehow ends
up being transient. This despite the fact that at the Prefs window's creation
its main widget's parent is null.

I think that's the real problem. An independent dialog needs to be not
transient. I'm confused how the patch even helps. Adding the "dependent" flag
should only make the window transient. Which it already (mistakenly) is. Perhaps
it ends up in the buggy state where it wants to always be topmost because of
some bad interaction between its having no parent and insisting on being
transient to something.

This patch by the way only affects the prefs dialog as it's created. I'm pretty
sure the prefs dialog remains attached to the browser window that opened it,
regardless of whether prefs are opened from another browser. If we want to
change that behaviour, we have the other platforms to worry about, too.

Comment 9

[Havoc Pennington]

That's the whole thing here, one of the meanings of the "dialog" semantic type
is that the dialog is kept on top - other meanings include that the dialog is
centered on its parent or on the screen, that it doesn't have a maximize button, 
that if the dialog has a parent it's not in the window list, etc.

One possible fix here, that may be right, is to put the help window in a 
different window group. The window group defines a single "application" - 
windows in the same group as a dialog are the windows the dialog 
will be kept on top of. Perhaps "help browser" is a different application 
from the web browser - or perhaps even each mozilla toplevel browser plus 
associated dialogs is its own application. (That last is probably right; 
an application is basically a main window plus dialogs, toolbars, palettes, etc.)

But playing with window groups is going to be more complicated to implement 
than other options, since GTK doesn't really support it yet you would have 
to do some weird Xlib hacks.
See http://bugzilla.gnome.org/show_bug.cgi?id=59724

Comment 10

[Robin Lu]

danm,

What you see is not caused by transient, it is caused by transient to none. In
current metacity, a dialog which is transient to none is supposed to be on the
top of the entire application, which means it is on the top of every windows of
this application. It does not mean this dialog is transient because it doesn't
transient to a special window but to all. I think it is somehow reasonable
because dialog does not show an icon in taskbar in gnome. Otherwise, you have to
minimize all the windows of this application to dig the dialog out if you
occasionally send it back.

My patch set the "dependent" of the pref dialog which means the dialog will be
only on the top of the window that created it instead of on the top of the other
windows of the application. 

You are right that the dialog remains attached to the window that opened it and
we don't have any good ideas to change the transient parent dynamicly in xul level. 

Comment 11

[Dan M]

Attachment: enforce Mozilla rules of transience

I figured this out while in the gym by the way, but you've been here
explaining. I was thinking about crying foul on Metacity but I see the gnome
docs even suggest this behaviour, "GTK_WINDOW_DIALOG [describes] a window for
transient messages and dialogs."

Problem is, and we all get this I know, Mozilla is explicit about whether it
wants windows to be transient. It could happen to dialogs or toplevel windows.
And could not happen to either, too. The attached patch fixes this problem (and
another related one; non-dialogs cannot be made transient). It looks fine in
Metacity because the window decorations are identical.

What do you think? Is this a gnome UI outrage? It's what Mozilla wants.
Arguably a big old window like Mozilla prefs, and -- ahem -- any other window
that Moz will similarly set loose to float around on its own, is less a
(transient) dialog, more a control panel kind of thing, anyway.

Comment 12

[Robin Lu]

I think it's a good patch. I will attach one for gtk2 soon.

Comment 13

[Robin Lu]

Attachment: patch for gtk2

Comment 14

[Havoc Pennington]

This approach isn't really ideal because it makes up a new hybrid
toplevel/dialog window type
for the prefs dialog - it's a dialog inside the window due to dialog buttons, 
but a toplevel from a window manager perspective. Definitely unkosher 
according to most UI guidelines.

If it isn't feasible to rework the code to associate the prefs dialog with the
specific toplevel browser window where Preferences was last selected 
from the menu, the way to make it like a GNOME control panel (a standalone
dialog) would be to set the DIALOG semantic type, but to change the dialog's
group leader to be unique to that dialog (so it's a "separate application" from
the browser windows). That way you won't be able to maximize the prefs dialog 
for example.

Comment 15

[Dan M]

Browsers are funky things, though. Even the main document window may be full of
dialogesque buttons, depending on the document you've loaded. And on UI
guidelines; that's just it: our whole problem here is that the prefs dialog
doesn't behave like a gtk dialog, so we're already in new territory.

However, as you say the best thing probably is to put different Mozilla window
types (browser, mail, prefs, others...) in different window groups. (This is the
same thing that isn't quite working yet, mentioned in comment 9, right?) This
was attempted some time ago. The plan was to group windows by their windowtype
XUL attribute. I think it failed because a window couldn't be re-grouped after
it had been mapped, and the windowtype attribute couldn't be fetched until after
its window had been mapped.

I imagine there's a solution. Guess we haven't found it yet. Personally it's not
me who has time to hunt for it at the moment.

Comment 16

[Tom Mraz]

I think that the danm's patch would be good enough for the 1.4 timeline if it
doesn't regress behaviour in other WMs.

This bug is very visible with the new metacity (as in Redhat 9 and so on).

Comment 17

[Asa Dotzler [:asa]]

Not going to block 1.4 for this.

Comment 18

[timeless]

.

Comment 19

[Christopher Blizzard (:blizzard)]

Comment on attachment 115457 [details] [diff] [review]
patch for gtk2

Nope, havoc is right.  I don't want this done like this.

Comment 20

[Tom Mraz]

So what's the correct solution? If the pref window in mozilla isn't really a
modal window, why it should be always on top? I'm inclined to say that it's a
problem of metacity trying to impose some new conventions which have never been
there before. But... Havoc would surely say that it's a mozilla's problem. So
maybe the solution is not to use metacity or not to use mozilla or not to use ....
Maybe it's mostly Redhat's problem because it has mozilla as a primary browser
and metacity as a primary WM.

:-(

Comment 21

[Robin Lu]

Attachment: patch for gtk2

I have modified the patch according to hp's comment.
Blizzard, could you have a look at it? If you have reviewed it, I will create
one for gtk.

Comment 22

[Robin Lu]

Attachment: patch for both of gtk and gtk2

This patch works for both of gtk and gtk2.

Maybe you guys never launch Help while the preference panel is on top, but some
one does.

Comment 23

[Kai Engert [:KaiE:]]

Scott, FYI only, this is the bug I mentioned recently when you changed the
certificate manager to a dialog.

Comment 24

[Brian Ryner (not reading)]

Comment on attachment 128214 [details] [diff] [review]
patch for both of gtk and gtk2

I think this should be ok.  Please put a space after 'if' as is done elsewhere
in these files.

Comment 25

[Robin Lu]

Attachment: patch

Thanks for bryner's review!

Comment 26

[Robin Lu]

Comment on attachment 151630 [details] [diff] [review]
patch

roc, could you help me to sr the patch? Thanks!

Comment 27

[Robin Lu]

Great thanks!
checked in trunk.

Comment 28

[R.K.Aa.]

Been starting to see this now and then - not sure what triggers it:

(Gecko:1010): GLib-GObject-WARNING **: invalid cast from `GdkWindow' to `GtkWindow'

(Gecko:1010): Gtk-CRITICAL **: file gtkwindow.c: line 1883
(gtk_window_set_transient_for): assertion `parent == NULL || GTK_

Comment 29

[Boris Zbarsky [:bzbarsky]]

This caused bug 280518, looks like.