Last Comment Bug 52209 - JS timers can fire while a modal dialog is open
: JS timers can fire while a modal dialog is open
: dom0
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
P3 major (vote)
: Future
Assigned To: Johnny Stenback (:jst,
: Prashant Desale
: Andrew Overholt [:overholt]
: 60150 (view as bug list)
Depends on: 418493
Blocks: 279518 337716
  Show dependency treegraph
Reported: 2000-09-11 17:49 PDT by Johnny Stenback (:jst,
Modified: 2008-10-31 15:32 PDT (History)
22 users (show)
csthomas: blocking1.9?
dveditz: blocking1.8.1.1-
dveditz: blocking1.8.0.9-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Testcase (130 bytes, text/html)
2000-09-11 17:50 PDT, Johnny Stenback (:jst,
no flags Details
Prevent timeouts from running while modal dialogs are open. (4.26 KB, patch)
2006-10-03 09:30 PDT, Johnny Stenback (:jst,
jonas: review+
brendan: superreview+
dveditz: approval1.8.1.1-
Details | Diff | Splinter Review

Description User image Johnny Stenback (:jst, 2000-09-11 17:49:43 PDT
The following HTML causes mozilla to bring up more than one modal dialog:

setTimeout("alert('timer alert');", 1000);

This is bad, and even seems to cause a crash in todays build. Dan, we talked
about this, one possible solution could be to give timers a context and make the
timers not fire (postpone them in stead) if the context (window) has a modal
dialog open.
Comment 1 User image Johnny Stenback (:jst, 2000-09-11 17:50:18 PDT
Created attachment 14442 [details]
Comment 2 User image Peter Trudelle 2000-09-14 15:50:08 PDT
Comment 3 User image Thomas 2005-02-20 03:37:26 PST
If I run the attachement I get an "alert" screen, followed by a "timer alert".
Are these those modal dialogs you're talking about?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20050111
Comment 4 User image Jesse Ruderman 2006-05-30 08:57:41 PDT
See bug 337227 and bug 339683 for some related problems.
Comment 5 User image Brendan Eich [:brendan] 2006-10-02 13:21:18 PDT
Is bug 279518 a dup of this bug?

Comment 6 User image Chris Thomas (CTho) [formerly] 2006-10-02 16:03:05 PDT
(In reply to comment #5)
> Is bug 279518 a dup of this bug?
> /be

The testcases seem to be almost identical.
Comment 7 User image Johnny Stenback (:jst, 2006-10-03 09:30:43 PDT
Created attachment 241072 [details] [diff] [review]
Prevent timeouts from running while modal dialogs are open.

This prevents timeouts from running in a window (and any of its its top-level window's children) while there's a modal dialog open for that window (or any window inside the same top-level window). When the modal dialog is closed we dispatch an event off of which we run any pending timeouts in all windows that were affected by the modal dialog.
Comment 8 User image Brendan Eich [:brendan] 2006-10-03 12:36:13 PDT
Hoping Darin can take a look too.

Comment 9 User image Jesse Ruderman 2006-10-03 15:45:30 PDT
Is this also a problem for other asynchronous-callback things JavaScript has access to, such as XMLHttpRequest?
Comment 10 User image Boris Zbarsky [:bz] (still a bit busy) 2006-10-04 13:37:56 PDT
On trunk with threadmanager, it probably is.
Comment 11 User image Boris Zbarsky [:bz] (still a bit busy) 2006-10-04 13:38:45 PDT
I'll try to get to this ASAP, but given the state of time I have and my review queue that's going to be probably at least a week.  :(
Comment 12 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2006-10-04 16:02:50 PDT
Comment on attachment 241072 [details] [diff] [review]
Prevent timeouts from running while modal dialogs are open.

I take it LeaveModalState is only called on the toplevel window? If not you need to grab the toplevel window in there.

>+  for (i = 0; i < length && aTopWindow->mModalStateDepth != 0; i++) {

Shouldn't this be |aTopWindow->mModalStateDepth == 0|?

r=me with that.
Comment 13 User image Brendan Eich [:brendan] 2006-10-04 16:33:52 PDT
Comment on attachment 241072 [details] [diff] [review]
Prevent timeouts from running while modal dialogs are open.

r=me with that sicking good catch fixed.

Will this back-port to 1.8 branch easily?

Comment 14 User image Johnny Stenback (:jst, 2006-10-05 10:30:59 PDT
Fix (including sicking's fix) landed on the trunk.
Comment 15 User image Brendan Eich [:brendan] 2006-10-05 10:39:15 PDT
Not sure we can back-port for 1.8.1.x -- some extension or Mozilla-only content might be counting on the ability to run timeouts "in the background" of a modal.  Comments?

Comment 16 User image Boris Zbarsky [:bz] (still a bit busy) 2006-10-05 11:17:32 PDT
I don't think we want to mess with this on the 1.8 branch...

I do think we need a followup for trunk for XMLHttpRequest and anything else that does async notifications.  Like event handlers?  At least on GTK key events get dispatched to the main window while a modal dialog is up.  :(
Comment 17 User image Jesse Ruderman 2006-10-05 11:28:22 PDT
And mousemoves, somehow: bug 324149.
Comment 18 User image Igor Bukanov 2006-10-07 18:08:29 PDT
Self-reminder: check if generator close hooks still allow to execute scripts when modal dialog is showing even with this bug fixed. For example, when GC is triggered in another window, the script can still show more alerts AFAICS. 
Comment 19 User image Daniel Veditz [:dveditz] 2006-10-07 18:21:06 PDT
If this can be landed safely in I'd like to get it into 1.8.0.x as well.
Comment 20 User image Jesse Ruderman 2006-10-14 23:51:42 PDT
*** Bug 60150 has been marked as a duplicate of this bug. ***
Comment 21 User image Mike Schroepfer 2006-11-01 10:14:35 PST
Brendan/Bz do we want this on the - you state in comment 15 that you are not sure then asked for approval on the patch - so we are a little confused..
Comment 22 User image Brendan Eich [:brendan] 2006-11-01 10:54:51 PST
I had to review the chronology here to recall my thinking, but clearly I first shared dveditz's wish to fix this in 1.8.x, then thought better of it due to the unfrozen but significant API incompatibility.  I'm still thinking we should hang tough on 1.8.x and fix this only for 1.9.  Boris?

Comment 23 User image Boris Zbarsky [:bz] (still a bit busy) 2006-11-01 18:44:35 PST
I stand by what I said in comment 16 -- I think this is fragile code and would rather not touch it on the branch.

Did those followups ever happen, by the way?
Comment 24 User image Daniel Veditz [:dveditz] 2006-11-13 12:05:27 PST
I think we're going to want this in FF2 at some point, but I'll defer to the fragility judgment for the next ones.
Comment 25 User image Boris Zbarsky [:bz] (still a bit busy) 2006-11-15 21:42:48 PST
Filed bug 360871 and bug 360872 on comment 16.
Comment 26 User image Daniel Veditz [:dveditz] 2006-11-27 17:20:10 PST
Comment on attachment 241072 [details] [diff] [review]
Prevent timeouts from running while modal dialogs are open.

Not this time, possibly not ever on the 1.8 branch
Comment 27 User image Daniel Veditz [:dveditz] 2006-12-14 15:25:40 PST
Should this be attempted on the 1.8 branches and can it be done safely? We've had more trunk baking time now.
Comment 28 User image Jay Patel [:jay] 2007-01-05 15:12:36 PST
Based on comment #22 and comment #23, we don't plan on taking this on the branches.  
Comment 29 User image Mark Smith (:mcs) 2008-03-05 10:26:09 PST
Because of this change, I had to switch to using nsITimer in one of the extensions I work on.  To help other extension developers, I added this issue to

Note You need to log in before you can comment on or make changes to this bug.