84047 - [ibench] modal dialog comes up under the main window (not above)

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Gagan]

On running the ibench tests on Mac the last page generates a modal dialog box 
and also a refresh. The result is that the dialog box comes up hidden behind the 
main window and the app appears "hung" 

The best way to reproduce it is to run the ibench page-loading tests (on a new 
profile) or at least make sure that you have this pref so that the dialog will 
come up...

user_pref("security.warn_submit_insecure", true);

Comment 1

[John Morrison]

There is a shortened version of the test that will run in a continuous loop
doing the final form submission at http://cowtools/bugs/84047/index.html.
It will save time in hitting this hung state (requires the pref to be set
user_pref("security.warn_submit_insecure", true);)

It's actually not a refresh and an alert, though. More specifically the 
sequence is a redirect to a page with a hidden FORM and some Javascript that 
runs onload. 

In that JS, when the number of loops has reached the maxloops number, the 
JS will do a document.resultsForm.submit() to POST values back to a server.

That causes (with the pref set true) necko to spin up the 'form submission'
warning dialog. That dialog initially comes to the front, but a moment later 
the parent window raises itself over the dialog, and the app is hung.

Comment 2

[John Morrison]

note: this bug arose out of investigation of bug 81480. 
And, wow, what a coincidence: my simplified test is actually 
http://cowtools/bugs/81480/index.html (not ../bugs/84047/..).

Comment 3

[Simon Fraser [no longer active]]

Maybe the iBench JS is focussing the browswer window after the modal dialog 
starts to come up?

Comment 4

[John Morrison]

Nope. The ibench test is not doing an explict js focus() of the window, 
and the stripped down test at http://cowtools/bugs/81480/index.html isn't 
either. 

Comment 5

[lchiang]

John - does the Windows iBench test also produce this modal dlg, but the modal
dlg comes to front for the user to dismiss?

Comment 6

[John Morrison]

This dialog appears on any POST form submission and the user has not yet 
unchecked the 'warn me' checkbox. (Which sets security.warn_submit_insecure to 
false in prefs.js).

So, yes, this dialog appears in the win32 ibench test, provided that the user
has not previously POSTed a form, and unchecked the checkbox.

But, for my shortened testcase above, trying this with windows (win2k) I do not
get the dialog falling behind the parent window (so no lockup). On mac, though,
it's easy to hit this condition with that testcase.

Comment 7

[John Morrison]

Note: this is not limited to ibench, since any similar form submission app 
could hit this as well (although it's not terribly common to do this sort 
of bang-bang logic). 

I shortened the testcase down to a single link that you can click on to 
get the hang. It's also at http://cowtools/bugs/81480/index.html.

If you watch closely, you can see the following things happen after you 
click on "express route":
  1) the urlbar updates to 'redirect.html', and then to 'death.html'
  2) the security warning dialog comes up (for a POST being done by 
     'death.html')
  3) the page 'death.html' paints and the main window raises above the modal.

In fact, I think that this is related to paint supression. It is when the 
document of the main window paints that it raises itself above the modal 
dialog.

Comment 8

[basic]

Changing platform

Comment 9

[Dan M]

  Beaucoup thanks, John, for coming up with the express test. And you're right, 
the bug happens in the paint suppression code. When it unsuppresses, it activates 
the browser window -- inappropriately, I'd say -- thus hiding the modal window.
  That window activation wants looking at. But in the meantime we can prevent its 
doing this kind of harm with the following patch. It fixes a bug in fail-safe 
code that should have (and now will) prevented a modal window's frontmost 
position from being usurped by a non-modal window.

Index: mozilla/xpfe/appshell/src/nsAppShellService.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsAppShellService.cpp,v
retrieving revision 1.148
diff -u -2 -r1.148 nsAppShellService.cpp
--- nsAppShellService.cpp	2001/06/25 11:27:37	1.148
+++ nsAppShellService.cpp	2001/07/01 21:21:48
@@ -642,7 +642,9 @@
 #ifdef XP_MAC
     /* Platforms on which modal windows are always application-modal, not
        window-modal (that's just the Mac, right) want modal windows to
        be stacked on top of everyone else. */
-    zlevel = nsIXULWindow::highestZ;
+    if ((aChromeMask & (nsIWebBrowserChrome::CHROME_DEPENDENT |
+                        nsIWebBrowserChrome::CHROME_MODAL))     && aParent)
+      zlevel = nsIXULWindow::highestZ;
 #else
     /* Platforms with native support for dependent windows (that's everyone

Comment 10

[jag (Peter Annema)]

> The bug happens in the paint suppression code. When it unsuppresses, it
> activates the browser window -- inappropriately, I'd say -- thus hiding the
> modal window.

Has a bug been filed on this yet?

r=jag

Comment 11

[Alec Flett]

seems reasonable.. is there ever the possibility of 2 modal windows at once
(i.e. two javascript window.alert's at the same time? Doesn't seem like this
would hurt that (i.e. just the last one to be brought to the front would get the
attention) but thought I'd ask.

sr=alecf

Comment 12

[Dan M]

Jag: it turns out bug 77675 is the more general case of this bug that you were 
wondering about. Alec: the nature of modal windows makes the possibility of two 
simultaneous unrelated modal windows -- I won't say impossible -- but I don't see 
how. Of course we have modal windows that themselves will open a new modal 
window, and this patch won't affect that case; neither helping or hurting. But I 
don't know of any problems with inappropriate activation events happening in that 
case, so I think this patch will fix all known modal window hangs of this type.

Patch is in on the trunk.

Comment 13

[Dan M]

...and on the 0.9.2 branch. Closing.

Comment 14

[Brian Ryner (not reading)]

The window coming to the front when it shouldn't sounds a lot like an instance
of bug 77675.

Comment 15

[Brian Ryner (not reading)]

oops, danm already said that... this is what i get for looking at bugs while i'm
on vacation

Comment 16

[John Morrison]

verified fixed on testcase 07/05 builds.