65521 - [linux] modal dialogs should only freeze parent window (not all windows)

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Deven Corzine]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.17 i686; en-US; 0.7) Gecko/20010105
BuildID:    2001010517

Currently, modal dialog windows freeze all Mozilla windows; they should only
freeze the parent window which is affected by the dialog.

Reproducible: Always
Steps to Reproduce:
1. Open two browser windows.
2. Use "Open Web Location" or enter a nonexistent server into the URL.
3. When the modal dialog appears, attempt to use the other browser window while
the dialog is active.  (You can't.)

Actual Results:  All Mozilla browser windows freeze completely during a modal
dialog, whether or not the results of that dialog have any bearing on the
windows in question.

Expected Results:  Only the parent window that owns the modal dialog should be
frozen.  That window is dependent on the user's interaction with that dialog.
All other windows should operate normally, without regard for whether or not the
modal dialog has been satisfied.  Multiple simultaneous modal dialogs (bound to
different parent windows, of course) should be possible to have.

[It is MUCH too easy to fail to notice a modal dialog (especially delayed ones
such as connection timeout or slow DNS lookup failures) and switch windows or
screens, obscuring the dialog.  Other browser windows seem to be "hung" and it
requires a search for the misplaced modal dialog, and may easily fool users into
thinking that Mozilla is "broken" and killing the application forcefully...]

Comment 1

[Fabian Guisset]

worksforme mozilla0.7 win32. Although I have been able to reproduce this only
once, and it happened only when I opened the modal dialog in the first window
(i.e. the one that called the second window). I'm no longer able to reproduce
however much I try.

Comment 2

[Deven Corzine]

Fabian, just to be clear -- you're saying that when you have a modal dialog
(such as Open Web Location) active in your Win32 version of Mozilla, you can
browse normally in other Mozilla windows without satisfying the dialog first?

Comment 3

[Peter Trudelle]

->danm, should that ideally be window-modal, or app-modal?

Comment 4

[John Morrison]

To be clear, that dialog (open web location) [and other dialogs] are window
modal on win32, but application modal on linux and mac (although window modal
is not common for mac applications in my experience).

Comment 5

[Dan M]

  The reason for this, copied (with some updates) from bug 19221: (I'm not sure 
what bug 19221 is these days; it's been morphed a few times. But it's partially 
this bug.)

  The situation you're describing is a condition of the gtk toolkit. If memory 
serves, gtk_window_set_modal() sets up an event filter that weeds out events not 
destined for the window made modal. The unfortunate effect is that modal windows 
are effectively application modal, not just window modal. As far as I know, the 
Mozilla crowd has no plans to ever try to change this gtk behaviour, outside a 
softly spoken desire to wean ourselves of gtk. If/when we do that, we'll have to 
spin up our own widget modality.
  So unless it's our fault for misusing gtk, we're stuck with this behaviour. 
Lacking an epiphany along those lines, or a weaning from gtk, we're stuck with 
it.

Comment 6

[Deven Corzine]

If that's a necessary consequence of using gtk_window_set_modal(), maybe we
shouldn't be using it.  Surely we can intercept the events at some other point
in the input handling that will only apply to the parent window?  (Either by
setting up a similar input filter in GTK, or maybe at a higher level?)

Regardless, this doesn't seem like a bug that should be lightly tossed aside for
future consideration.  It can cause major usability problems and user confusion
on Linux, and it sounds like it isn't consistent with the behavior on Windows.

What would be the impact of not having ANY modal dialogs?  Maybe the cure is
worse than the disease?  (What if dialogs weren't modal, but always brought the
dialog to the top when the parent window gets an event?)

Comment 7

[Deven Corzine]

Come to think of it, this seems like a serious problem in GTK that could bite
other applications -- GTK really ought to make it optional whether to freeze
just the parent window(s) or all application windows for modal dialogs.  Has
this issue been discussed with the GTK developers?

Comment 8

[Dan M]

*** Bug 65522 has been marked as a duplicate of this bug. ***

Comment 9

[Dan M]

  Deven: I haven't brought it up with the gtk folk, no. As you say, we should 
consider a home-spun modality UI instead. It's something we'll probably need to 
do someday anyway. And it's actually a somewhat interesting prospect and I 
wouldn't mind working on it. I'd rather work on it over some of the things I'm 
currently working on. But I have priorities and honesty suggests I admit that 
there's no place for this project on *my* short-term schedule, thus the "future" 
milestone. I'll add the helpwanted keyword, but not expect too much impact.
  Also, I'm not a big fan of modal dialogs and I think we at least overuse them. 
But there are at least a few places in the code where they're completely 
required. So we can't dump them outright.

Comment 10

[Deven Corzine]

It might be worth contacting the GTK developers for their input.

I agree that the modal dialogs are overused.  Where are they indispensible?

Comment 11

[Dan M]

There are a few dialogs which must be guaranteed to return an answer from the 
user, and the thread of execution must suspend while it awaits. There's the 
authentication dialog posed by networking code, and the JavaScript prompt and 
confirm dialogs. A few others I think, too. My poor choice of words, "thread" and 
"suspend" do kind of get you thinking that a programming model could be devised 
where the dialogs themselves musn't necessarily be modal. But that's not where we 
are.

Comment 12

[Deven Corzine]

I don't know; are modal dialogs REALLY necessary even there?  If someone hits
"Back" on the browser that's waiting for an authentication dialog, it seems more
reasonable to discard the dialog and execute the "Back" function than to ignore
the user's input...

Comment 13

[Dan M]

From a UI standpoint, maybe not. But from a programming standpoint, modal dialogs 
allow vastly simplifying assumptions to be made, and we're making 'em. The best 
UI might be to fill the affected browser window with an authentication form and 
let it loose. I am so not volunteering to even try to code that up.

Comment 14

[Dan M]

*** Bug 41758 has been marked as a duplicate of this bug. ***

Comment 15

[Jesse Ruderman]

See also bug 59314 for the idea of allowing the user to go 'back', close the 
window, etc. while an auth dialog or javascript alert is up.

Comment 16

[peter]

Attachment: patch nsGtkEventHandler.cpp patchfile (it will do other windows too)

Comment 17

[peter]

That last patch is for .7. 

It works by disabling windows that spawn modal dialogs under gtk/linux. 

It allows all of the events to flow through(instead of using gtk's(broken) modal
event model) it then filters them out in nsGtkEventFilter.

I will make another patch for .8 when it is released or I can do it off of a
nightly. Just ask.

Please send any comments to [e-mail]

Comment 18

[peter]

Attachment: This patch is for the nighty(or .8)

Comment 19

[Christopher Blizzard (:blizzard)]

This is a good start however there are some problems that I see.

o You're using the STL.  Bad.  Can't use that since it isn't portable enough. 
If you want a simple array class check out nsVoidArray.

o We don't want to throw out all events if we see that an event is destined for
the parent window of a dialog that is marked as modal.  We should rewrite them
for the dialog instead.  I think that's what gtk does.  This allows size changes
and visibility changes to be properly registered iirc.

I haven't looked at this patch at length yet.  There are a lot of really subtle
things that you need to watch out for with modal windows so we need to be very
careful here.

Comment 20

[Dan M]

  I haven't looked at length yet either, and I'm rather hoping I can pawn that 
job off on Christopher, when it's ready to go. But this is me sending 
encouragement to Peter: I think you have the right idea.
  I recommend taking a look at the gtk+ source. Since its idea of modality has 
been working for us -- except for the part this bug complains about -- we'd 
probably be safe emulating its event handling. Take a look at gtk_main_do_event() 
in gtkmain.c. My (incomplete) understanding is that gtk modality is implemented 
by a stack of modal widgets (the most recently pushed one being the only actively 
modal one at any one time). The stack is in a global called "grabs".
  There's a whole raft of special treatment depending on the event type. Events 
like GDK_MAP, GDK_PROPERTY_NOTIFY, GDK_EXPOSE (and a bunch of others) ignore the 
modality/grabbing, while others like GDK_KEY_PRESS and GDK_MOTION_NOTIFY pay 
strict attention. I think this is code you'll have to adapt into Mozilla if you 
want to spin your own modality, which does seem to be the solution to this bug.

Comment 21

[peter]

The problem with the implementation of modality under gtk+ is that the parent is
not a concept that the toolkit knows about.

The side effect of this of course is that if you do a gtk_window_set_modal(or
whatever) on a dialog...it will not pass through any events for anything other
than that window. In order for this to work. We would have to rewrite gtk+ so
that there was a concept of parentage in windows so that if we did an open
dialog it could be modal just to the parent.

This is kind of why I needed to comment out all of the gtkmodality stuff under
gtkwidget. I was using the 1.2 version of gtk. So this info maybe a bit outdated.

Yeah it seems as though it is a structural problem with gtk.

Since its model seems to only allow application wide modality instead of window
modality which is what my patch is chasing after.

Of course I will look into those two functions in Gtk that you mentioned. 

I will also look into rewriting it to use the nsVoidArray that you mentioned.

I also have used this to find some rather evil parts of the code(ie the print
dialog is launced from the hidden window and not from the window that asked for
the print(which breaks this modality stuff) but I have a patch for that too))

The problem with only accepting events for the dialog is that is application
modality(I think) and I wanted window modality. If I kept a list of windows that
accepted events and threw ones out for the parent of the dialog I think I would
pretty much get the same thing right?(or not...I may be confused)

Thank you for your encouragement. I haven't contributed to the tree before.

Comment 22

[peter]

http://bugzilla.mozilla.org/show_bug.cgi?id=19221

Check out plaster's comments at the end of this bug to see what I am talking
about. He actually looked at the gtk+ stuff.

Comment 23

[Dan M]

  Looking at your patch more closely...
  Ah. I see you're setting up modality as a dialog is created, by disabling its 
parent. And gutting nsXULWindow::SetModal. Partly right; there are some problems. 
Not all dialogs are necessarily modal, and some non-dialogs can be modal. Take a 
look at nsXULWindow::ShowModal(); that's where the decision is made to go modal, 
and where you'd most happily plug in.
  There's a kind of ugly bit there that goes

  window->SetModal(PR_TRUE)
  ...
  EnableParent(PR_FALSE)

This is a somewhat badly structured attempt at cross-platform-ness. Platforms 
that need to do something to the window itself to be modal implement the first 
one; platforms that need to do something to its parent implement the second. It's 
somewhat unclean, but my point is, these two methods together are where to hook 
in modality code.

  But anyway, the Windows implementation, for instance, ignores SetModal, relying 
on EnableParent to kill interaction with the parent window. This is not the 
(current) gtk model, but it's the model you're shooting for. You should be able 
to disconnect SetModal on the Mozilla gtk build and hook up your window disabling 
code to EnableParent, as Windows does, and be happy. We do keep track of the 
parent chain in Mozilla; this is how Windows works, after all, and it does work.
  In fact, it might be simpler than that. EnableParent is implemented in the gtk 
build. I just watched in gdb though, and EnableParent did nothing because the 
parent nsWidget had a null mWidget member. It seems like the parent widget is 
malformed, or maybe nsXULWindows are holding on to the wrong object for the 
parent under gtk. [e-mail] or [e-mail] should be able to 
clarify. It could be that fixing this problem (and then gutting SetModal()) is 
all you'd need to do. Or maybe you'd also need to put in the event filters we've 
been discussing, depending on exactly how gtk_widget_set_sensitive(false) widgets 
behave. Be careful messing with the parent chain in nsXULWindow, though, since 
that's cross-platform code and working on the others.

Comment 24

[phsu]

I agree with your idea of where the setmodal code should go...however I had a
tough time finding(3 weeks...but I am a beginner) a unique pointer for the
entire windows widgets...the only one I could find(and I think I tried them all)
was the nsWindow->GetTopLevelWindow(orWidget). The
nsWidget->GetTopLevelWindow(or nsParent, or this->GetParent(), etc) may be null
or yielded the parent moz_area or some other gtk draw widget or something. As I
tried printing out the pointers that I could find(The only common ones I could
find were associated with nsWindow). It could be that I could find the nsWindow
that is associated with the nsWidget->SetModal() but I could find a way(but I am
not a real pointer caster or anything like that).

If you know of a way off the top of your head. That would be great. The method
that you described was my original intention...but I ran into trouble and this
was the backup plan. 

I tried messing about with it nsXULWindow as well...but I also have nsWidgets
there as well. So that was the big dilemma that I had run into. Couldn't get
from my nsWidget to its nsWindow so I ended up doing it with the creation of
nsWindow. 

I will look into EnableParent()

Comment 25

[peter]

Here is a patch (following) that has the vector removed and I was able to move
the enable_window from nsWindow to nsWidget(yeah! my original intention)

So now it does the stuff using the enable part. I had tried to do this from the
SetModal stuff. But I wasn't able to find the parent window from there(of course
after danm's suggestions I realized that I already had that in nsXULWindow)

Many more comments are welcome. It seems to work very well now. Because now I
see some of these non-modal dialogs and they appear to work properly. 

I have another patch for dialogs for print being launched from the
hiddenWindow(and I think we want them to be launched from the
currentWindow(browser etc)) but I am not sure if I should include this in this
bug/patch.

Comment 26

[phsu]

Attachment: Patch for nightly that has suggestions/fixes

Comment 27

[Dan M]

  About the latest patch: I think we don't actually use Enable() to en- or 
disable widgets that aren't top-level windows, but the interface implies it could 
be used for that, so I wouldn't want to prevent it. The patch changes the whole 
top-level window when a contained widget is Enabled. I think you only want to 
enable_window if the widget itself is the top levl window.
  Yes, that's different usage from SetModal, which does look for the top-level 
window. But I argue that SetModal can be sloppier because it doesn't have any 
possible secondary meaning, like Enable.
  But besides that argument, I'm curious whether what Enable already does -- 
gtk_widget_set_sensitive -- would be sufficient by itself without the new 
enable_window functionality, if only it actually worked. I'm arguing that there's 
a fundamental problem here (mWidget is null, preventing set_sensitive from 
working) that wants to be understood before anything else happens. Superwin 
Christopher? Any ideas?

Comment 28

[Christopher Blizzard (:blizzard)]

gtk_widget_set_sensitive isn't going to change the way that we do things since
it's only valid on real gtk widgets.  Remeber, we aren't using full fledged gtk
widgets here so it's not going to make a bit of difference when it comes to
focus handling.

Comment 29

[phsu]

I agree with danm.

But the Enable is called in many cases(probably on object creation for many
things) however the only time it is called to disable a widget is when the modal
dialog box pops open with nsXULWindow. However my model requires a GtkWindow * 
as a common thing for all of the events. If I block on a certain widget I run
into a problem that although I block on the big parent widget I still will not
block on all of the little widgets inside. Of course in talking with blizzard
yesterday, I am not super sure that I cannot use something else to determine
which events to block.

But I wasn't able to find any common element other than the GtkWindow that
contained all of my stuff. If I block on that it seems to work fine. If I block
on the parentWidget(or its equivalent) I cannot raise the parent above the
modal...but I still can activate all of its little children. 

But to deal with his suggestion...What cases would we would only want to disable
part of a window? I guess I just don't know any cases that would be true. But
that is what this is for:P

If we can determine if the nsWidget->Enable() is a topLevelWidget then I am all
down for it. Just don't know how to do that. Maybe it is a tag...I will look
into that.

Comment 30

[Dan M]

  Alrighty then. It sounds like your explicit window disabling will be necessary; 
set_sensitive isn't going to help. I am still concerned that Enable should be 
usable on sub-level widgets without making the parent window modal (though as you 
say, we probably never to do this in the current codebase). But it's an 
interface; it gets to make fewer assumptions.
  About determining whether you're a top level widget, I'm not ultra familiar 
with the gtk widget/window code, but isn't nsWidget::mIsToplevel just that?

Comment 31

[peter]

yes it is. I have added that one line to it...Seems to act the same...but I know
from the print statements that there is a change in the flow of the program.

if(mIsTopLevel)
enable_window(GetTopLevelWindow, bState);

That is the only change. I didn't see a difference on the outside..but the code
still works and you are right it may have caused problems if that hadn't been done.

Comment 32

[Jesse Ruderman]

See also bug 62740, no window should end up in front of a modal dialog on Mac 
(since on Mac, all window-modal dialogs are also app-modal, according to hirata 
and jrgm).

Comment 33

[peter]

Attachment: Just a one line change for disabling only if mIsTopLevel

Comment 34

[Dan M]

  So you've run with this latest patch and it, well, works? It looks mostly good 
to me, except I think you still have one thing to worry about: some events should 
get through even to the disabled window. There's some very similar logic in 
widget/src/mac/nsWindow::ModalEventFilter, where we spin our own modality as 
you're doing. Not that you could use that as a guideline.
  It might be safest to let all events through by default, only suppressing 
certain explicit ones. I assume right now you're losing GDK_EXPOSE messages, for 
instance. Does that not cause any drawing problems? You might think about 
dropping only mouse button, mouse movement, key and drag events.

Comment 35

[phsu]

I would be all down for doing that. It seems to do alright for drawing the
window behind and does the redraws when you resize and move the window. But yes
you are right I will look into that.

Comment 36

[peter]

I think the nsWindow GdkEventMasks work for the expose events etc...

The only problem that I can see is that I am not sure what mask to use for the
GDK_DELETE event. As I think I don't get all of the events(like expose,
property_change) because of the masks set up in nsWindow. 

That is the only problem I have found thus far. The expose and resize events
seem to be passed down ok. Just the GDK_DELETE event is getting through(we need
to set up a mask..because if I get it, it should be thrown out) so I am not sure
what mask to use in nsWindow.

Comment 37

[peter]

Ok I think I know what I want to do now. inside of handle_delete_event(in 
nsWindow) i just want to "return" if the window is disabled. Of course I guess 
that means more connection between nsGtkEventHandler and nsWindow so that I can 
return a true if my nsWindow->GetTopLevelWindow() matches one in the dWindow 
list in nsGtkEventHandler. If there is a better way to do this(ie find the 
nsWindow that nsWidget->Enable() is associated with so I can set a tag inside 
of nsWindow...perhaps by casting my nsWidget as an nsWindow(as if mIsTopLevel 
is true that should be true)) Perhaps it is better to do it with an interaction 
with nsGtkEventHandler.

Sorry this is kind of thinking out loud. If you have any suggestions please 
write me at [e-mail]. I will post a patch with this tommorrow.

The problem that I am trying to fix here( the redraw resize expose seem to work 
ok) is the window manager delete (close button) and closing the parent window 
when the modal is still open.

Comment 38

[peter]

Alright done on the handle_delete_event.

But I am now working on getting the focus in/out event for the parent of a modal
dialog box to pass the event to the child.(ie browser spawns dialog, user clicks
on browser, but dialog gets the focus)

Comment 39

[peter]

Sorry I have been kinda busy with work. I am not sure if we want parity between
windows and linux on the focus on the parent window yeilds a focus on the
child(dialog window). This may be difficult as my solution thus far is a bit messy.

Anyhow my patch is coming up after this and it deals with the parent(disabled)
window being destroyed from behind a modal dialog. This should fix this problem. 

Comment 40

[peter]

Attachment: This is a patch for the deletion of a window that is disabled

Comment 41

[Deven Corzine]

Just noticed something strange about the modal dialogs.  While they disable any
mouse events for all windows, KEYBOARD events still work.  Why is this?

Comment 42

[phsu]

Cuz I need to change it...thanks.

Comment 43

[phsu]

Actually the bug that you describe is for the normal mozilla...not the one with
my patch. I wasn't able to recreate your bug with my patched mozilla...But I was
able to with the normal mozilla(nightly bin pulled) so you may want to write a
bug up about it. Don't know how the wider community will feel about my patch. I
will post on the unix newsgroup again and hopefully they won't ignore it.

Comment 44

[John Morrison]

bug 53476 is about the fact that you can get keyboard access to the parent
while a modal dialog is up on Linux.

Comment 45

[peter]

Well as far as I can tell this fixes that bug as well.

Comment 46

[phsu]

I guess I need some input on what people feel is necessary in the patch, as I
think that implementing focus on parent->dialog will be really messy. I have
most of the code to accomplish this, but I need to make some window manager
calls so that it correctly reflects the state of the X/mozilla/gtk focus.

right now I am using gtk_window_set_focus() or gtk_widget_grab_focus() but
neither seem to reflect changes in the window manager. I am going to try
window->SetFocus()...well that didn't work.

I guess I am wondering if people think that the "focus from parent to dialog" is
very important.

Comment 47

[Deven Corzine]

If you're talking about automatically giving the focus to the dialog when the
parent window gets focus, I'd say that's pretty important, as is always keeping
the dialog stacked above the parent.  (I'm only talking about modal dialogs.)

If you pass events (keyboard, particularly) to the modal dialog window when the
parent is frozen and has focus, that should work fine.  If the events get to the
dialog window, it probably doesn't matter much whether or not the dialog "has
focus" officially.

What about situations where a modal dialog is unmapped by a window manager that
is keeping the modal dialog (for some reason) on a different virtual screen than
the parent window?  Should the modal dialog be forced to be mapped on the same
screen (if that's possible) whenever the parent gets focus or mouse/keyboard events?

Oh, and I hope freezing the parent does NOT block the delete message when you
try to close a window.  Even if the parent has a modal dialog, you should be
able to delete the window.  (And the modal dialog, of course, should be deleted
along with it.)

Comment 48

[phsu]

Well. 

Stacking works.

Not sure about the multiple desktops and the control over that(I kinda doubt it
as it may be window manager dependent)

I tried it out on a windows box and closing the parent window is not allowed
while a modal is up.

The focus on the parent yields a focus on the dialog(ie the parent cannot be
focused) is what I was asking about. As in the prev build you could focus on the
parent, but you couldn't interact with it. But it wouldn't pass through
focus/keyboard/mouse/delete events.

I am trying to bring the modality under linux in parity with windows. So I have
been using it as a model for operation.

Comment 49

[Deven Corzine]

Can something be done to keep the delete functionality working despite the modal
dialog, on all platforms?  (Does this need its own bug number?)

Comment 50

[Dan M]

  My take on Windows parity with focusing the modal child of a window instead of 
the window itself: does your patch keep the modal child floating above its 
parent? I'm thinking not. That would be a bad thing, since the modal dialog can 
then be accidentally lost behind the parent, and it'll be a mystery why the 
parent is unresponsive. That would be a killer problem.
  As for the child window taking actual focus, like Windows, that would be great. 
But the current modality code doesn't do this either, so it's no regression 
checking in a patch that improves other aspects but doesn't affect this one.

Comment 51

[peter]

It does keep the modal above the parent. And that is what I was thinking on the
focus thing(let it lie for now(as it has been))

Yeah the modal will not be lost below the parent.

Well if anyone has any other suggestions...I guess it is done?

Peter Hsu

Comment 52

[Dan M]

  I've run with the patch and it seems to work. Hmmph. I really thought you'd 
have to filter for specific event types, and I'm surprised it keeps the modal 
window on top of the parent, lacking any specific code to that effect. Perhaps 
because it's dropping the activate event? Eh?
  But it seems to work. I'm ready to get it checked in if someone more familiar 
with gtk specifics gives it the thumb. Blizzard?

Comment 53

[Christopher Blizzard (:blizzard)]

I need to digest this for a bit.  Modality and grabs are fun stuff in gtk.

The modal is kept above the parent as a result of the transient call in
nsWindow::CreateNative and as long as your window manager is dumb as a post it
should keep it on top.

Comment 54

[Dan M]

  I just noticed a curious case where keystrokes seem to get through to the 
parent of a modal window. I noticed it while looking at bug 70775. Try this: open 
a browser window, open the "Find in This Page" dialog, search for something it 
won't find. Ignore the helpful "not found" alert. Click on the Find window.
  Running with this patch, ESC won't dismiss the dialog unless you click in its 
titlebar. Then apparently the window begins accepting keystrokes, because ESC 
will close it (resulting in a crash).
  Again, it's better behaved with this patch -- at least you have to click in the 
titlebar -- so I wouldn't ask this problem be fixed before checking in. I guess 
I'm just FYIing.

Comment 55

[Deven Corzine]

Even if the modal dialog is always on top of the parent, do we know that it's
always on the same virtual desktop screen as the parent?  (Obviously this is an
interaction with the window manager, just wondering if we can ensure it or not.)

Of course, the serious problem occurred when other windows were frozen in
addition to the parent, where the parent window and modal dialog were on another
virtual screen from the unresponsive ones on the current screen.  (That's why
this bug was originally filed, after all.)  Once the modality only affects the
parent window, this window-manager stuff won't be as much of a concern.

What about delete events to the parent that's frozen?  Will it beep, then focus
and raise the modal dialog window?

P.S. [meta] Are bugs commonly left as "new" until fixed, instead of being
assigned to someone?

Comment 56

[phsu]

It will not beep. 

But it will raise the Modal Dialog above all other windows. 

Window manager interactions are things that I cannot control as they are all
different. It is many times an option in the window manager(ie I have no control
over it)

I agree that would be nice. But I don't think that window manager interactions
are standardized. And the modal problem would be the worst when the whole
MOZILLA would freeze up which this patch fixes. 

The bug wasn't really filed for this reason, it was just one way for it to show
up.

Comment 57

[Deven Corzine]

I filed this bug because of all Mozilla windows getting frozen because of one
window with a modal dialog, not because of window-manager interactions.  I'm
just saying that once this bug is fixed, there might be window-manager problems
but I doubt they'll be nearly as severe...

Comment 58

[peter]

Yes you are right.

Comment 59

[Deven Corzine]

If there's a fix in hand, should this bug be assigned to the person with the
fix?  Also, when can we expect the fix to land in the trunk?

Comment 60

[Christopher Blizzard (:blizzard)]

After it has gone through the review and super-review process.  I want to go
over this but it won't happen until after 0.8.1.

Comment 61

[peter]

I am posting a renewed patch for this bug...however it also fixes the bug#70775.
It just tosses out the keyboard events for the parent window as well. Now the
keyset keyboard events will be chucked along with the rest of them.(before you
could still scroll up and down in the browser when the save dialog was
up...which under windows is impossible)

Peter Hsu

Comment 62

[peter]

Attachment: This is a patch that fixes both 65521 and 70775

Comment 63

[Dan M]

Smokin! Thanks for figuring out 70775 too, Peter!

I do have a concern ... your fix has to trap events in two places: 
handle_gdk_event and again, downstream, in handle_key_press_event. An event is 
caught upstream if it obviously belongs to a Mozilla window and the window is 
modally disabled. Otherwise it's passed on (through the gtk_main_do_event in 
handle_gdk_event), and eventually downstream we figure out it really does belong 
to a window, and there you catch the event in a handler for the specific kind of 
event that's slipping through in this case and causing problems.

This makes me worry that there's an entire class of event targeting that isn't 
being caught. It's something else for Blizzard to consider when he gives this 
patch the eye.

But it does rather handily fix both problems. Thanks again!

Comment 64

[peter]

I think the "other" case takes into account when an event is fired at the
titlebar and not at the contents of the window. That is why I need to block
events for delete/keypresses. But I am not sure.

This would also explain why without my patch the dialog will crash on the whole
window...but with my patch just on the title bar. I guess if I can catch all of
the events that can be fired at a title bar that would be good. Alternatively I
can have it so that every event that is called checks to see if its parent is
disabled...I tried to keep the numbers of calls to the IsDisabled(or whatever)
down to a minimum to keep the speed up.

What it meets to be that if object is false(a window) or GTK_IS_SUPERWIN(a
window)...we are in some strange case... The only time I ever see it is when
windows are coming up and when they are being deleted. 

I will look into it some more.

Peter Hsu

Comment 65

[peter]

Attachment: Patch that is the same prev. But blocks key release events

Comment 66

[Brett Granger]

Just "listening in"...  This bug affects us in a pretty big way.

Comment 67

[peter]

Attachment: Patch for the new CVS stuff that blizzard put in(mar/20/01)

Comment 68

[peter]

The previous patch removes some warnings, and uses the changes that blizzard put
into the code. In order for the last patch to work(the one before that one is
the same functionally) you need to do a CVS update in widget/src/gtk(the 3/16/01
patch will also work for .8)

The new patch is functionally the same...it just works with the changes that
blizzard put into the tree.(the old one also works...but this new one follows
his structure better)

Comment 69

[selmer (gone)]

Are the patches on this bug sufficiently good to bring this in from the Future
milestone?

Comment 70

[peter]

Well ask the guy that wrote it...

But it seems to work for me. I was using it for about a month now. But I 
haven't tried it with any nightlies since last friday.

I also have fixes for some of the other bugs contained in the patch.

This was my first contribution so I don't know how you determine it is 
ready for "prime time".

Peter Hsu

Comment 71

[Dan M]

eh. Milestones. I plan to get the patch checked in as soon as a gtk cognoscente 
besides the author says he likes it as much as I do.

Comment 72

[selmer (gone)]

dmose, you marked this dogfood and did not explain how this is preventing you
from doing work.  Did you mean catfood?  I'm moving this to dogfood- since the
workaround is to dismiss the offending dialog.  Please convert to catfood if
that was your intent.

That said, it would still be good to get r=/sr= on the existing patch if possible.

Comment 73

[Dan Mosedale (:dmosedale, :dmose)]

I marked this as dogfood during the time when the intent was to repurpose the
nsdogfood keyword and the nsCatFood keyword did not yet exist.  Nominating for
catfood.

Comment 74

[Dan M]

r=danm

Comment 75

[[e-mail]]

(Using linux build 2001042508, usually running remotely from a Sun CDE)
Maybe one of the most important dialogs that should be made non-modal
until this gets maybe sorted out differently is 
"The operation timed out while contacting ..."?
The annoying thing here is that (at least for me) when this happens
in a minimized browser window, the dialog is not visible (minimized too),
and all other windows are locked, causing confusion.
Sometimes too, the dialog is *behind* the browser window that caused
it to appear, thus never getting visible by minimizing other windows.
The only response to give to this is "OK", so modality doesnt seem
important here....

Comment 76

[Boris Zbarsky [:bzbarsky]]

*** Bug 87588 has been marked as a duplicate of this bug. ***

Comment 77

[John Morrison]

Hmmm. This baby seems to have gone cold. This had r=danm in April, and I 
suppose it was just needing sr=. I guess now it needs a little update and 
checking out again, but perhaps it would be good to bring this up to date
again.

Comment 78

[Brett Granger]

Yes, is there a good reason that this was let go cold?  This bug has started to
move to the top of the annoyance pile for our product now.  If I can help, let
me know.

Comment 79

[Boris Zbarsky [:bzbarsky]]

*** Bug 93247 has been marked as a duplicate of this bug. ***

Comment 80

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 97377 has been marked as a duplicate of this bug. ***

Comment 81

[Rui M. Silva Seabra]

It shouldn't block EVEN the parent window. Example:
You are making something with cookies, and need to test it. If you are like me
and have a huge list of forbidden cookies and images, it takes a lot of time to
load that window. There is no reason for it to freeze any browser window. Cookie
and Image permissions windows are one such example of unneded freezing of windows.

Comment 82

[Pierre Chanial]

Yes!
Excerpt from the N6.1 Release Notes:
"If Netscape is locked up but doesn't seem to have crashed, make sure there are
no dialog boxes still open. Close each window on your desktop one at a time and
if you uncover a dialog window, dismiss it"

Comment 83

[Boris Zbarsky [:bzbarsky]]

*** Bug 91632 has been marked as a duplicate of this bug. ***

Comment 84

[Boris Zbarsky [:bzbarsky]]

so... what's up with this patch?  is it still applicable?

Comment 85

[Dan M]

Attachment: updated/current version of peter's last patch

Comment 86

[Dan M]

I had trouble applying Peter's latest patch; you never know what it is patch is
complaining about. So I've redone it and attached it just above. This version
also deletes some now useless code. I've been playing with it for a while now
and had no problems. This patch (and peter's original patch) affects the code
used to fix bug 14131 and bug 16310. I've been running with it for a while now
and had no problems, but I'd appreciate some extra eyes on it, especially
regarding those other two bugs.

Comment 87

[Colin Phillips]

if there's a useable (complete) patch for this one, please let me know which one
it is!!

Comment 88

[Christopher Blizzard (:blizzard)]

I spent a couple of hours looking and working with that patch and unfortunately,
it has some pretty serious problems.  It drops events when it shouldn't and it's 
going to wreak havoc with embedding.  Basically, it needs a new approach.  It's
on my short list to work on, though.  I know lots of people want this.

Comment 89

[Christian Reis]

Hoping we will see this sometime soon; it's a major usability problem with Mozilla.

Comment 90

[Christopher Blizzard (:blizzard)]

It's hard to get right.

Comment 91

[Colin Phillips]

But it is soooo right.  :)

Comment 92

[[e-mail]]

Well, maybe it would be worth to at least reconsider why all these dialogs
have to be modal? One annoying example is the dialog that appears when
a hostname couldnt be resolved. When I have iconified that window in the 
meantime, as soon as the error dialog appears, or rather doesnt appear,
because its iconified, all other windows stop to respond. Now I have got
to dig up all the iconified windows to find out which is responsable.
Even more annoying is the fact that now the dialog can actually happen
to be *behind* the parent window! Unless I move each window around I will
never be able to get rid of it. And all this just because the host wasnt found?
There are dozens of other messages that could be dealt non-modally.
IMHO modal dialogs should only be issued, if something terrible would happen
unless activity is stopped.
The bottom lines is, rather than figuring out how to only freeze the
parent window, why not refrain from freezing alltogether as much as possible?

Comment 93

[Colin Phillips]

That doesn't help people like us (OEone) build an applicaiton suite on top of
mozilla.  Any dialog in any application throughout the entire system that is
modal freezes the entire system.  Ideally it would simply freeze that
application, and you could task switch to another application without an issue
(which is exactly where the solution to this bug comes into play). Therefore a
work around to solve this bug for the mozilla suite of applications isn't going
to do anything more than solve this problem for mozilla.  Others wanting to use
mozilla as a framework are still stuck with this problem.

Comment 94

[[e-mail]]

Colin, but that is just a special case of the general problem that 
mozilla seemingly doesnt do much multitasking: often things that stop, hang or
slow one mozilla window 
will stop, hang or slow the rest. This is a problem I often have: I am getting
windows that arent repainted while mozilla is doing something like 
retrieving big emails, or something I even dont know what. It would be nice to
get that changed, but its not the issue of this bug I think, or is it?
Lets get this fixed quickly in a way that annoys that least number of people
when 1.0 is out.

Comment 95

[Colin Phillips]

Agreed.  Multitasking, good.  Being annoying, bad.

Comment 96

[Akkana Peck]

Agree with johann.  Most dialogs that are modal don't have any need to be.  But
that's not this bug; you should file bugs on specific dialogs that are modal
that shouldn't be.  Behavior like dialogs popping up without letting you know
which parent window had the error, or dialog windows popping up behind their
parent, are also bugs (but not this bug).

I have long been mightily tempted to put in a secret pref to disable all
modality across the app, so no dialog could ever be modal.  (There are cases
where modality is actually good, but they're so rare that I could live with it
to solve the other 95% of cases where windows shouldn't be modal but are.)  But
of course, I would never admit to this because I know I'd get lynched by the
win/mac ui people for even suggesting it.

Comment 97

[Dan M]

no time this milestone :(

Comment 98

[Brice Lecomte]

*** Bug 100492 has been marked as a duplicate of this bug. ***

Comment 99

[Dan M]

Attachment: current (but still not right) patch

Just freshening the last (broken, but still very useful) patch, which could no
longer be easily applied. I was inspired to do this by the recent discovery
that this patch also fixes bug 88827. Also I believe it contains all previous
patches, so I'm using the new (since this bug was begun) Bugzilla feature to
mark the others obsolote.

Comment 100

[Dan M]

dumping on Chris, since it's he who has in mind writing a different patch.

Comment 101

[John Malmberg]

OpenVMS is badly affected by this bug.

Comment 102

[Navin Gupta]

*** Bug 115846 has been marked as a duplicate of this bug. ***

Comment 103

[Dan M]

Hey Chris -- I've played with this patch and rather liked it. It seemed like a
real improvement over current behaviour. Do you think you'll get to rewriting it
before 1.0? If not, is it better to check it in or leave this broken?
Personally, and I admit my ignorance about messed up event delivery, it seems to
me that less things would be broken if we took the patch.

Comment 104

[Christopher Blizzard (:blizzard)]

It's been a while since I've messed with the patch but I remember spending
several hours with it and fixing one thing after another with event delivery and
realizing that the patch that I was going to end up with was going to look very
different than what is here.  The problems that this will cause will happen
mostly in an embedding context.  The browser might be fine but when it's used in
a Gtk application things start to go haywire.

I just haven't had time to spend on this bug, unfortunately but I can tell you
that the problems that it will cause outweigh the benifit of it being in the
tree.  At least, that's my opinion.

(As a side note, the gtk2 code that's already in the tree already doesn't have
this problem.  Modal windows only block their immediate parents.)

Comment 105

[Tuukka Tolvanen (sp3000)]

*** Bug 120505 has been marked as a duplicate of this bug. ***

Comment 106

[Dan Mosedale (:dmosedale, :dmose)]

blizzard: the current behavior is pretty painful.  Could this patch be checked
in with ifdefs such that for the embedding case it gets built differently than
for the browser case?

Comment 107

[Christopher Blizzard (:blizzard)]

No, they share too much code.

Comment 108

[m_mozilla]

this bug says modal dialogs should only freeze parent window...

but what about tabbed browser? See bug 123913

    [RFE] tab-specific alerts are app-modal
          and should be tab-modal
          (need "sheets" widgets on non-MacOSX)

does the pending patch give any relief to tabbed browser users?

-matt

Comment 109

[Boris Zbarsky [:bzbarsky]]

That would be bug 59314

Comment 110

[m_mozilla]

Boris, I don't think bug 123913 is a straight dup of bug 59314.

Each bug can be fixed while leaving the other broken (see comments in either
bug), they seem independent to me and so I'm de-dupping.

Please correct me if I am mistaken.

-matt

Comment 111

[Asa Dotzler [:asa]]

*** Bug 121812 has been marked as a duplicate of this bug. ***

Comment 112

[R.K.Aa.]

*** Bug 130833 has been marked as a duplicate of this bug. ***

Comment 113

[Dan Mosedale (:dmosedale, :dmose)]

*** Bug 130362 has been marked as a duplicate of this bug. ***

Comment 114

[Boris Zbarsky [:bzbarsky]]

*** Bug 131549 has been marked as a duplicate of this bug. ***

Comment 115

[Boris Zbarsky [:bzbarsky]]

*** Bug 132416 has been marked as a duplicate of this bug. ***

Comment 116

[Vadim Berezniker]

*** Bug 114772 has been marked as a duplicate of this bug. ***

Comment 117

[Vadim Berezniker]

*** Bug 137035 has been marked as a duplicate of this bug. ***

Comment 118

[Christopher Blizzard (:blizzard)]

The patch in bug #129591 fixes this.

Comment 119

[Christopher Blizzard (:blizzard)]

The fix for bug 129591 has landed on the trunk so this should be fixed there.

Comment 120

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 128688 has been marked as a duplicate of this bug. ***

Comment 121

[Christian :Biesinger (don't email me, ping me on IRC)]

shouldn't this be marked FIXED now?

it is at least fixed for me :)

Comment 122

[Christopher Blizzard (:blizzard)]

I'm going to try to get this into the 1.0 branch.  So, no, not yet.

Comment 123

[Georg Wild]

Well, the mean thing works, but you can't open a window with right click in
another window, I think!

Comment 124

[Christopher Blizzard (:blizzard)]

Fix checked in on the 1.0 branch and the trunk.

Comment 125

[Asa Dotzler [:asa]]

adding branch resolution keyword.