ShutDown() should try to close all open windows.

VERIFIED FIXED in M11

Status

P2
normal
VERIFIED FIXED
20 years ago
14 years ago

People

(Reporter: sfraser_bugs, Assigned: davidm)

Tracking

Trunk
All
Mac System 8.5

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

20 years ago
Copied from <news:sfraser-61DF59.12352021061999@news.mozilla.org>

> Due to some great work by Simon Fraser, Simon was able to fix some
> memory problems throughout the app that were causing our messenger and
> composer objects to not get destroyed when you exited the application.
> Obviously this had a trickle down effect on a lot of things in our code.
>
> After running with his changes, if I bring up mailnews and then Close
> the app through the menu item, messenger is properly released. However,
> if I choose Exit instead, we leak.
>
> It appears that Close and Exit have slightly different implementations.
> Close gets a root webshell, gets a web window and then closes the web
> window.
>
> Exit gets the appshell and shuts it down.
>
> I wonder if shutting down the appshell is supposed to have a trickle
> down effect where all the windows are then closed. If that's true, then
> the close code is never getting called. Or should our implementation of
> Exit call Close first and then tell the appshell to shut down?

You should post this on xpfe so the xptoolkit folks see it (makes
sense, no?).

Behaviour has always been different for Exit() and Close(), and
I don't think Exit() is doing the right thing, currently. It should
try to close each open window in turn, and, if a window cannot
be closed (say it's a modified composer document, and the user
cancelled the Save prompt) then the exit sequence should be
terminated, and the application stay running.

---------------

Looking in nsAppShellService::Shutdown(), I see some #ifdef'ed out code
that tries to close all open windows. It does not handle the case of windows
which refuse to be closed (e.g. user cancels the Save dialog), and it should.

We also need to fix this so that windows get destroyed in the same way,
whether you hit the close box in the UI, or choose Exit from the menu.

Updated

20 years ago
Status: NEW → ASSIGNED

Comment 1

20 years ago
Accepting this, I'll look to do something for M8.  Ultimately we will need some
mechanism to block shutdown if a window so desires.  Once
nsIAppShellService::Shutdown is called, I fear it is too late to go back.

Updated

20 years ago
Priority: P3 → P2
Target Milestone: M9

Comment 2

20 years ago
Move to M9 ...

Updated

20 years ago
Target Milestone: M9 → M10

Comment 3

20 years ago
Now to M10...
(Assignee)

Comment 4

20 years ago
I may have writen this code for globalOverlay.js quit function. It is commented
out right now since there are some RDF problems in the task menu. If you want
feel free to reassign this bug to me.

Updated

20 years ago
Assignee: law → davidm
Status: ASSIGNED → NEW

Comment 5

20 years ago
I'll gladly hand this one off to you, David.

My thinking is that we will have to invent a more robust "window close"
mechanism.  My thinking is that we'd turn nsIBrowserWindow::Close into an "ok to
close?" method.  It would have 2 possible results: OK or Cancel.
Implementations of nsIBrowserWindow ought then implement that method by
delegating to some application-specific logic.  In our case, nsIBrowserWindow
ought to call some per-Window JS code (perhaps specified by an onclose handler
on the XUL <window> element).  Implementors of that code would either do
window.close() and return OK, or put up a Save/Don't Save/Cancel dialog and
return OK (after saving or not) or Cancel (if the user says to do that).

nsIAppShellService::Shutdown would then go to each top-level window, get an
nsIBrowserWindow*, and call Close().  This would stop when/if some window said
cancel.  BTW, this code needs to be wired into the platform-specific system
shutdown event(s) (I know Windows has one).

An issue might be that this semantic for nsIBrowserWindow::Close doesn't fit
with it's current semantic (which might be simply a close notification).  If
that's the case, then we probably want to invent a new OkToClose() method and
let Close() continue to work the way it does.

This solution requires some input/cooperation with the toolkit team (to get OK
for onclose handler) and the webshell/browser-window guru(s) since this impacts
any potential webshell embedders.

Anyway, that was my thinking.
(Assignee)

Updated

20 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 6

20 years ago
Here is the plan of record. Each window can have a JS routine called
tryToClose() which will return true if the window was closed and false if the
window couldn't be. The quit routine ( in JS) will walk the window list from
most recently used to last used and either call tryToClose or close if
tryToClose isn't implemented. It will stop when tryToClose returns false. If it
walks all the way through the window list it will then call AppShellService
shutdown.

Updated

19 years ago
QA Contact: beppe → paulmac
(Assignee)

Updated

19 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

19 years ago
fix checked in

Updated

19 years ago
Blocks: 17907

Updated

19 years ago
Status: RESOLVED → VERIFIED

Comment 8

19 years ago
marking verified for now. I wonder if this will break with new webshell stuff.

Updated

19 years ago
No longer blocks: 17907
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.