890504 - Proper force-quitting with canceling beforeunload event listeners

Status: UNCONFIRMED

(Toolkit :: Startup and Profile System, defect, P3)

Description

[László Károlyi]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/29.0.1502.0 Safari/537.36

Steps to reproduce:

Hi,

I wanted to implement a force-quit at an extension level, using appStartup.quit(appStartup.eForceQuit), which ignores the beforeunload event handlers and shuts down firefox immediately.


Actual results:

As it seems, firefox does execute all the beforeunload event handlers before shutdown.


Expected results:

Beforeunload event handlers should have been ignored, and firefox should shut down immediately.

Comment 1

[László Károlyi]

This bug is also related to https://bugzilla.mozilla.org/show_bug.cgi?id=890505 .

Comment 2

[Gabor Krizsanits (INACTIVE)]

Dave, do you know where does this bug belong / who might know more about this part in general?

Comment 3

[Benjamin Smedberg]

Why do you think that eForceQuit should not execute beforeunload handlers? Note that it should force-quit in either case.

Comment 4

[Gabor Krizsanits (INACTIVE)]

If I recall correctly the problem is that beforeunload event might trigger a popup window, and that prevents the shutdown.

Comment 5

[László Károlyi]

It's more like a confirm window. And the 'force-quit in either case' does not apply: if I click the 'Stay on the Page' button, Firefox does not close.

Comment 6

[Benjamin Smedberg]

We should fix eForceQuit; nothing should be able to cancel shutdown once you've hit that point. I'm not sure about whether it should be firing the beforeunload handlers, but I'm sure it ought to quit anyway.

Also, whether or not bug 890505 gets fixed, hacking around this bug by removing unload listeners sounds like an engineering nightmare and I don't recommend it.

Comment 7

[Gabor Krizsanits (INACTIVE)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> We should fix eForceQuit; nothing should be able to cancel shutdown once
> you've hit that point. I'm not sure about whether it should be firing the
> beforeunload handlers, but I'm sure it ought to quit anyway.

If regardless of the answer in the confirm window we shut down, we should not show the confirmation window I guess. So I would just not call domWindow->CanClose in nsAppStartup::Quit in case of eForceQuit for each window (which would fire the beforeunload event). I'll try a patch like that soon on try to see if it breaks something. That would also mean that printing cannot block shutting down either any longer.

> 
> Also, whether or not bug 890505 gets fixed, hacking around this bug by
> removing unload listeners sounds like an engineering nightmare and I don't
> recommend it.

This sounds like a very valid argument.

Comment 8

[Gabor Krizsanits (INACTIVE)]

Hmm. That didn't work out.