511456 - Cancelling a quit still closes some windows

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

Steps to reproduce:
1. Have an unsaved Gmail draft in one window
2. Have some more windows open
3. Press Ctrl+Q
4. Get asked by the Gmail window if you really want to quit
5. Press Cancel

Expected results:
All windows stay open

Actual results:
After pressing cancel, the non-Gmail windows close. History|Recently closed windows opens empty windows instead of restoring what was in them.

(this is with a mozilla.org Firefox 3.5.2 build on Ubuntu)

Comment 1

[Benjamin Smedberg]

Mossop, do you know the workflow of Ctrl+Q?

If it's using quit-application-requested, will content-area onbeforeunload handlers get a notification then, or only when quit-application-granted has been fired?

Comment 2

[Benjamin Smedberg]

<gavin> bsmedberg: permitUnload is what you want to search for
<gavin> bsmedberg: biesi's bug would kind of be annoying to fix
<gavin> we'd need to start checking permitUnload for all windows/tabs before trying to close any of them
<biesi_> yeah
<gavin> what happens if you permit one tab to close, but not the next?
<biesi_> well then you don't close any
<bsmedberg> gavin: yeah, don't close any
<bsmedberg> gavin: it sounds like something we should be doing at quit-application-requested, way before we get to actual window-closing
<gavin> I guess so

Comment 3

[Dave Townsend [:mossop]]

(In reply to comment #1)
> Mossop, do you know the workflow of Ctrl+Q?

I don't no, and probably wouldn't have time to dig into it until after next week. I have a suspicion that this bug is a dupe too.

Comment 4

[Benjamin Smedberg]

Attachment: Checkpoint rev. 1, trades one bug for another

This patch trades one bug for another: quitting now works well, but beforeunload handlers are not called at all when the user just closes one window.

I'm also going to have to learn browser-chrome tests setup, and I'm not sure whether it can deal with the modal prompts that onbeforeload handlers put up...

Comment 5

[Dave Townsend [:mossop]]

Fixed by bug 537449