When loading the attached file the following happens: 1) Slow script warning after x seconds 2) Meta refresh causes page to reload 3) Browser hangs - no slow script warning This works as expected in 3.6.13

Btw you have to click "Stop script" after step 1, of course. Should this block? It's a regression, an easy way to (accidentally) hang the browser, and it's just annoying.

FF4 beta 10: OK FF4 beta 11: broken Bisecting...

Works: Feb 01 - http://hg.mozilla.org/mozilla-central/rev/8b5cb26bbb10 Fails: Feb 02 - http://hg.mozilla.org/mozilla-central/rev/1c2d53a2dcfb Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8b5cb26bbb10&tochange=1c2d53a2dcfb

Bug 621764 and bug 620615 look suspicious. CC'ing people.

Yeah bug 620615 looks like a good candidate.

(In reply to comment #5) > Yeah bug 620615 looks like a good candidate. OK, blocking on that bug and moving to Toolkit bugs.

It's almost certainly Bug 621764.

I can't verify this at present as my build is still in progress, but a quick look at attachment 508997 [details] [diff] [review] makes me think that the new EnterModalState function of the window could call EnterModalState on the current context twice: http://mxr.mozilla.org/comm-central/source/mozilla/dom/base/nsGlobalWindow.cpp#6356 This is probably fine, but the problem might be that the LeaveModalState function of the window will only call LeaveModalState on the current context once if the return value of EnterModalState is not provided as an argument: http://mxr.mozilla.org/comm-central/source/mozilla/dom/base/nsGlobalWindow.cpp#6469

Jan de Mooij, how did you find this?

Won't hold the release, but dolske, can we get this investigated, and re-nom'd if we think it's likely to be widespread?

(In reply to comment #9) > Jan de Mooij, how did you find this? I use meta refreshes to load random scripts in the browser. I also use window.location = ... but that fails if the script throws. Refreshing is cheaper than starting a new browser process for each test. I'm afraid this might hurt other test frameworks.

NOTE: At present, this patch is provided "as is" without testing of any kind! It seems that comment 8 was in the right direction, and this patch fixes the problem by changing all callers to pass the return value of EnterModalState to LeaveModalState. I'm not sure if there is a convenient way to cover the patch with regression tests. I included the fix for bug 633891 (remove an unused line) because it is a follow-up to bug 621764 too. Thanks to Jan for finding the regression range!

Comment on attachment 515773 [details] [diff] [review] Fix known callers to use the return value of EnterModalState Yay, was just getting to this today and I see you've got a patch. Laziness wins again! ;) Anyway, looks fine to me but over to jst since I can't review this. The other approach here would be to change EnterModalState to return the window COM-style -- EMS(nsDOMWindow **aWindow) -- and not have it twiddle the one thing when aWindow == nsnull. I don't think there's much advantage of one over the other.

I've now verified that the patch passes the existing regression tests (the few failures I observed on the tryserver build, on some platforms, seem unrelated). At this point in the cycle, I'm not sure if this patch qualifies for inclusion in the release candidate and if so what is the procedure / which flags to use.

Comment on attachment 515773 [details] [diff] [review] Fix known callers to use the return value of EnterModalState I think this is perfectly safe to take, so a+ing.

Comment on attachment 515773 [details] [diff] [review] Fix known callers to use the return value of EnterModalState Sadly, didn't land in time for RC1.

Still reproducible for me on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0. The dialog appear and I get a refresh even if I press continue or stop script. The same behavior is present if I try to close the dialog from the x button. In all this time, Firefox is not responding.