1266353 - alert() and family can block setTimeout() but not the event handler callbacks

Status: NEW

(Core :: DOM: Core & HTML, defect, P3)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: setTimeout-alert.html

STR:

1. Run the test case.
2. Without dismiss the 'blocked' alert, switch to another tab.
3. Switch back
4. Repeat step 2/3
5. Try to dismiss all the alert boxes

Expected:

1. On step 2 and 3, the 'visibilitychange' alert boxes should not overlap (since the 'blocked' alert should block the event handler
2. 'setTimeout' should not show before alert boxes are all dismissed

Actual:
1. Alert boxes overlaps
2. as expected.

Note:

This is discovered during investigation of bug 1263760.

The DOM spec states the event loop should "optionally" paused by alert(), but it does not say "optional" for prompt() or confirm().

Chrome shows the same behavior on the event loop (the "before" console.log() shows), but the alert boxes comes one at a time.

Safari blocks all the events and setTimeout callback as expected.

Comment 1

[:Gijs (he/him)]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from bug 1263760 comment #8)
> (In reply to :Gijs Kruitbosch from bug 1263760 comment #7)
> > (In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from bug 1263760 comment #6)
> > > My current theory is that the tabs rely on `onUnloadTimeout` invoked by
> > > setTimeout to switch to that state (`this.setTabState(tab,
> > > this.STATE_UNLOADING)`), however setTimeout is blocked by the select dialog.
> > 
> > This is somewhat plausible - I vaguely recall there being an issue with
> > setTimeout and nested event loops. IIRC nsITimer is not affected, so you
> > could potentially test it by using that instead of setTimeout and seeing if
> > the problem goes away?
> 
> Yeah that works locally. I can prepare a patch for it. However, it is very
> troubling that the prompter can blocks the setTimeout of the entire world.
> 
> Should we be fixing the deeper issue instead, or I should just submit a
> tabbrowser.xml patch for review?

I don't have an opinion on this because I don't know how hard the setTimeout issue is to fix. I suspect that fixing it would likely change behaviour on the web. Imagine websites that do this:

function foo() {
  setTimeout(function() { document.body.textContent = "Wahey, you passed the alert"; }, 0);
  alert('hello!');
}

Of course, this is a trivial example, but I suspect that to a certain degree the bug about setTimeout's behaviour masks bugs with how this stuff works for in-content pages - though then again, there are already issues with some of this via e.g. bug 727397. I don't know how exactly we queue events for tabs blocked on from-content tab-modal prompts and if it's possible to keep doing that but unbreak setTimeout for chrome consumers. Maybe Olli can help clarify what we should/shouldn't care about here.

I do suspect it would be useful, and potentially upliftable/shippable for 48 or whatever we need it for, to switch to an nsITimer (maybe using Timers.jsm) for this code, whereas an in-depth fix of the setTimeout issue seems likely to be more involved.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

There are two separate issues here, if you compare our behavior with Chrome. 

1. When there is a modal dialog, Chrome behaves the same way (blocks setTimeout and allow event callbacks)
2. In Chrome, if any of the event callbacks invoke another modal dialog, the modal dialog is queued behind the currently displayed dialog.

I don't really know if (1) is a bug or a feature, or, if it's a bug, maybe it's a bug shouldn't be fixed (so we don't change the behavior of the Web).

The behavior of (2) is definitely a bug. In Firefox, I am seeing modal tab-content dialogs being shown on top of another (and the semi-transparent modal mask gets darker and darker).

Depend on a decision here we could limit the scope of this bug to fixing (2) only, and leave (1) alone.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I'm now lost what this bug is about. comment 2 talks about issues in Chrome. and then fixing (2) of them. 
And Chrome seems to have window-modal dialogs, not tab modal.

But yes, we don't block visibilitychange events, or many other events either when showing modal dialogs. per-connected-domain-page-event-queues would help here (and that could be implemented using per-connected-domain-child-process or as a separate layer on top of the main event loop)

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

If that makes it easier to understand, (2) is now bug 1266611.

We would still need to decide if the behavior of the event loop is a bug or not in this bug.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

In general I think the behavior described (that some events still fire when an alert() is up) is a bug, unless fixing it is not web compatible.  We would need the infrastructure in bug 715376 to do this though.

Comment 6

[Andrew Overholt [:overholt]]

I'm guessing we're not planning on getting to this in the next few months (or sooner) so I'm tagging it as backlog but I'm happy to be incorrect here.