Closed Bug 8673 Opened 25 years ago Closed 25 years ago

ShutDown() should try to close all open windows.

Categories

(SeaMonkey :: UI Design, defect, P2)

All
Mac System 8.5
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sfraser_bugs, Assigned: davidm)

Details

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.
Status: NEW → ASSIGNED
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.
Priority: P3 → P2
Target Milestone: M9
Move to M9 ...
Target Milestone: M9 → M10
Now to M10...
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.
Assignee: law → davidm
Status: ASSIGNED → NEW
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.
Status: NEW → ASSIGNED
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.
QA Contact: beppe → paulmac
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
fix checked in
Blocks: 17907
Status: RESOLVED → VERIFIED
marking verified for now. I wonder if this will break with new webshell stuff.
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.