User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1 If i shut down Windows while FF is running the next time i start FF the session saver of TabMixPlus says that the last session crashed. Nothing unusual can be observed if windows shuts down with FF running. It just exits. The extension developer says the fault is FF not the extension. It seems that FF exits differently if closed manually then if it was closed by windows on shutdown. The extension doesn't have the ability to write to the config that FF was shut down properly like it does if FF is closed manually. My Windows installation is set up properly so that programs can shut down and take some time to perform closing functions. But as already said, FF exits immediatly so it isn't killed hardly by Windows because it takes too long to exit. Reproducible: Always Steps to Reproduce: 1. Start FF 2. Restart Windows 3. Restart FF Actual Results: Session saver of TMP says last session crashed, like FF didn't exited properly last time. Expected Results: Nothing, since FF was shutdown properly.

there's an old suite bug about this. DUPEME.

Bug 212316 is a Mozilla Suite bug. I'd rather have one for Firefox itself, since this will affect the SessionStore service to be introduced for Firefox 2. What should be done: * WM_QUERYENDSESSION should cause the equivalent of canQuitApplication to be called (issuing quit-application-request and quit-application-granted) * This can be done either through having the -killAll command line switch actually do something (which it apparently doesn't for Toolkit apps) * Or do this right in place: http://lxr.mozilla.org/mozilla/source/toolkit/xre/nsNativeAppSupportWin.cpp#581 Feel free to file additional bugs or dupe this against a Toolkit specific bug.

This patch removes the useless "killall" call and replaces it with what we already do from JS (cf. goQuitApplication in globalOverlay.js). For simplicity and for what I'd like to do in bug 338040, I've moved the canQuitApplication method from globalOverlay to nsCloseAllWindows (which probably should be renamed to nsQuitAppHelper or something).

Comment on attachment 223910 [details] [diff] [review] nicely shut down the app at WM_QUERYENDSESSION (cf. goQuitApplication) Rob or mconnor have looked at this code more recently than I.

I tried the patch on Firefox 1.5.0.2 and unfortunately it didn't work out. I'm doing heavy debugging on this now and I found out the following: If you do closer->CloseAll() within the message handler for WM_QUERYENDSESSION something strange is happening: at some point in time every top level window gets WM_SESSION although I didn't return from the WM_QUERYENDSESSION handler and the the app gets closed by Windows. I tried *very* hard to find some routine within firefox which calls exit(), abort(), throws an exception or raises a signal but to no avail (I used API hooking provided by MS detours - nothing got logged). The last thing I see in my log is a WM_DESTROY entering nsWindow::ProcessMessage - but no more exit

I have come to the conclusion that WM_ENDSESSION must be handled in order to shut down cleanly. WM_QUERYENDSESSION may be used to call canQuitApplication but after returning from the handler the app may be shut down any time after having received a WM_ENDSESSION message on every top level window. I have tried to PostQuitMessage() in WM_QUERYENDSESSION but even that need not lead to WM_QUIT received by the main message loop in nsAppShell. This means that we have no way to ensure a clean shutdown when we do not handle WM_ENDSESSION. Unfortunately this means that we have to reconsider the current policy of using ScopedXPCOMStartup in XRE_main - at least on windows this scope may be never left!

Comment on attachment 223910 [details] [diff] [review] nicely shut down the app at WM_QUERYENDSESSION (cf. goQuitApplication) The problem is that Windows sends WM_QUERYENDSESSION and WM_ENDSESSION to /each/ window and this patch only handled it for the global message window. I'm not sure how to proceed in this case.

WM_QUERYSESSION may be vetoed by _any_ window so I would say that it is sufficient to have it handled only by the global message window. WM_ENDSESSION is the real problem, as it is sent to every top level window and after returning from the handler the app may be terminated any moment. I think it would be best to handle the cleanup within a WH_GETMSG hook

I made a new patch based on Simon Bünzli's work against Firefox-1.5.0.4. On receipt of WM_QUERYENDSESSION canQuitApplication is called and the message is replied accordingly. On receipt of WM_ENDSESSION all windows are closed and an instance of a new component nsIXRENativeShutdown is used to cleanly shut down the application. This got necessary because there is no guarantee that WM_QUIT ever gets to the main application loop. Additionally the calling of ::DestroyWindow is suppressed as this seems to terminate the application immediately - the WM_DESTROY-Handler is called instead (this detail took a LOT of time to find :-()

(In reply to comment #9) > I made a new patch based on Simon Bünzli's work against Firefox-1.5.0.4. Thanks for the patch. However I'd appreciate it if you honored my work and wouldn't put your name above it (this concerns nsICloseAllWindows.idl, nsICloseAllWindows.js, updates.js and nsCommandLineServiceMac.cpp).

You are right of course, and I definitely respect your work (at least in my comment to the patch ...) As this patch is also part of our product, I _had_ to put our name on it. Because you had omitted your own name I wasn't sure if I should add it for you.

Benjamin, does this patch look good? Do we need another rev?

This patch looks very scary. In addition to changing existing interfaces, it forcefully shuts down XPCOM in the middle of the eventloop, never letting the stack unwind. I don't really understand what the semantics of WM_ENDSESSION are, but it seems to me that unless the app is really forcefully terminated after we return from WM_ENDSESSION we should go with calling nsIAppStartup.quit(eForceQuit) from this message and unwinding the stack properly.

What can I do to convince you?

Explain to me why letting the stack unwind is impossible? I'm not sure there's anything you can do to convince me that exiting in the middle of processing events is a good idea.

Let's see ... The WM_ENDSESSION-Message is definitely the last Message send to an Application after it has agreed to terminate by returning TRUE from WM_QUERY_ENDSESSION. Exceptions are Messages sent by the application itself in response to closing windows. But even those aren't handled normally - if you call ::DestroyWindow after having received WM_ENDSESSION the application is terminated immediately (this took me almost three weeks to find out because no debugger I found works after receiving WM_QUERY_ENDSESSION) From the documentation of WM_ENDSESSION: "If the wParam parameter is TRUE, the session can end any time after all applications have returned from processing this message. Therefore, an application should perform all tasks required for termination before returning from this message. The application need not call the DestroyWindow or PostQuitMessage function when the session is ending." PostQuitMessage() is the function which is called by nsIAppStartup.quit(eForceQuit). It is supposed to post a WM_QUIT-message to the application's message queue which normally would terminate the event loop. Unfortunately the WM_QUIT never gets dispatched to the event loop because after return from the WM_ENDSESSION handler the application normally terminates immediately.

The problem with this approach is that we're shutting down XPCOM without ever walking back up the stack: the stack is very likely to contain owning references to various XPCOM components (through comptrs), which can cause unusual crashes during XPCOM shutdown. Possible alternatives: 1) refrain from ever returning from the WM_ENDSESSION call by doing a longjmp from the event handler to a previous safe point to walk back up the stack and exit. Still need special handling to avoid ::DestroyWindow and whatnot. 2) Call nsIAppStartup::Quit(eForceQuit) from the WM_ENDSESSION handler and then call exit(). This will obviously leak lots of XPCOM stuff but at least won't crash. 2a) in addition, send some of the profile-shutdown and xpcom-shutdown notifications before calling exit()... this will perform additional synchronous cleanup, without actually releasing services and doing scary event/thread shutdown.

*** Bug 355939 has been marked as a duplicate of this bug. ***

*** Bug 351663 has been marked as a duplicate of this bug. ***

Can we change the summary of the bug to be a more generic "firefox doesn't exit properly... etc" cuz I've got a whole bunch of rdf failure in the same situation bugs that depend on this fix. Easier for searching and all.

Is there a similar function that might cause the same issue in osx? I can't recall for sure but I'm fairly certain I've seen the corrupt rdf issue on osx as well.

*** Bug 358667 has been marked as a duplicate of this bug. ***

*** Bug 360652 has been marked as a duplicate of this bug. ***

*** Bug 360680 has been marked as a duplicate of this bug. ***

For non-technical users who are totally oblivious to session saving, the resulting error message on load comes over as very strange and a bit obnoxious -- ask my father. He had no clue why Firefox 2.0 kept giving him stupid errors but fortunately in this case I realised what he meant because I started getting the same problem. (He never closes apps before quitting Windows -- there really is no need to shut them all down manually.) I think it would be helpful if you were able to fix this one ;) Oddly, I am not familiar with this problem affecting SessionSaver with Firefox 1.5 and earlier... (Win 2k/XP)

Why dont you just start closing down on the wm_queryendsession?? many apps do this. The user has requested closing windows so im sure they will understand, its much better than having it pop up on restart complaining and doubling up my tabs.

if it's correct that this causes bug 232252, then this bug causes dataloss. I'll leave that for others to judge.

dietrich/bsmedberg: What is the correct behavior for session restore? This bug sounds like a feature to me. Please give us some input on whether we should take this patch on the 1.8 branch. Thanks!

What patch? there is not a workable solution here (yet). There is a bug.

Over in bug 342885, I've posted a branch-safe bandaid patch for the Session Restore issue caused by this bug (cf. attachment 249012 [details] [diff] [review]). Jay might be talking about that one...

Bug 365749 is a similar bug for Linux.

(In reply to comment #28) > dietrich/bsmedberg: What is the correct behavior for session restore? This bug > sounds like a feature to me. Please give us some input on whether we should > take this patch on the 1.8 branch. Thanks! > When you shut down the system properly and systematically receive prompts to save whatsoever...Servelet.htm files this is more a nagging annoyance than a feature, or am I in error? OK, it's not a catastrophe, but I would describe it more like a pain in...

At least one person is experiencing a similar problem on Mac: bug 367892.

This behavior overrides the "Clear Private Data on Exit" option, too. I have Firefox configured to: 1) "Show my home page" at startup (*not* my "windows and tabs from last time") 2) "Clear private data" when I close Firefox. If I shutdown Windows without closing Firefox first, the next time I start FF I'm asked if I want to restore the last session. If I answer yes, I get my websites and tabs from last time -- contradicting my desire to have my private data cleared. Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/2006120418 Firefox/2.0.0.1

Right, so, the suggestion you're making is that crash-proof session saving should itself be disabled when privacy protection is in place? There is no setting in the Preferences to enable/disable crash-proof session saving, so your session is always recorded at all times in case of crash or power failure, in a rather readable JavaScript file. Either this facility needs to be optional, or, it must be disabled (at cost to the user in terms of crashes) when private data is set to be cleared on exit so that nothing is recorded. You are satisified with this? As an aside, re-opening and closing Firefox after the crash/power failure is NOT enough, because your profile also contains a sessionstore.bak file with potentially traceable information in.

(In reply to comment #35) > This behavior overrides the "Clear Private Data on Exit" option, too. This issue doesn't really override it - it simply prevents the code responsible for that feature from running. Other possible side-effects: toolbar settings get corrupted/lost, the downloads list gets corrupted/cleared, setting changes get forgotten. IOW: everything which could need clean-up or might be being processed at the point of abrupt shutdown or which should be run when Firefox closes can and does break. (In reply to comment #36) > Right, so, the suggestion you're making is that crash-proof session saving > should itself be disabled when privacy protection is in place? That's a request like bug 366572 (which seems likely to be WONTFIXED). > As an aside, re-opening and closing Firefox after the crash/power failure is > NOT enough, because your profile also contains a sessionstore.bak file It usually IS enough, because that file is deleted when Firefox is properly closed and the current session won't be resumed (and of course also whenever you clear your private data).

If FF wants to restore a session after a crash (regardless of my privacy settings), I guess I'm willing to live with that. That's a crash -- all bets are off. But during a normal shutdown, I expect my privacy preferences to be respected. It concerns me that FF is circumventing privacy by claiming a crash occurred -- when it in fact did not. My only point was to emphasize the this bug has consequences related to user privacy -- which is a hot-topic these days. I suppose an option that would allow me to completely turn off session saving -- crash related or otherwise -- does make sense. How does a user know conclusively that FF completed a normal shutdown? (rhetorical question)

I agree that FF needs to properly shut down when running during a windows shut down. The session restore feature alerted me to a potential problem and I turned that function off in my about:config file. But then I realized that other things weren't getting cleaned up on a windows shut down. Like some cookies stayed around. Nothing major, but as noted by others, this violated my privacy settings. Those same cookies get cleaned up if I close FF then close windows. But not when I shut down windows (which in turn shuts down FF automatically)

This is the problem: > when I shut down windows (which in turn shuts down FF automatically) > but when waking up again FF thinks it crashed and pops up that nagging window in spite it was orderly shut down by windows (remember windows sends a message to running apps warning of imminent shutdown, WM_QUERYENDSESSION)

no, we can't remember this, because it's Comment 16 and you people keep adding comments.

Windows sends WM_QUERYENDSESSION (as noted before) to allow certain apps to popup a dialog along the lines of, "You have unsaved work, save | discard | cancel", where cancel prevents the machine shutting down. This is OK for apps where you might lose data, but in Firefox you won't (unless you've been dumb enough to type something significant into an open form field and leave it there, not a significant case IMHO). I think Firefox should just close gracefully when Windows shuts down, with no confirmation dialog. It would be annoying; imagine every application doing this. You'd have to click 'confirm' once for every app open to shut the machine down. It should be reserved for very special cases. The current behaviour of FF is obviously wrong, but at least it shuts down without requiring a confirmation dialog to be clicked. Clearly it needs to shut down gracefully as to avoid the 'session restore' prompt when next started up (session restore should be an extension-installed feature, except on a genuine crash).

Presumably still with the exception of messages like "There are still downloads in progress"? (Something you may have forgotten about.) What's annoying is when you have several windows open and every one needs to query you "There are x tabs open..." -- if you're going to have a quit confirmation, please just do it once :-)

I'm not sure I understand how the last two comments relate to this bug. This bug is not about confirmation dialogs at all, and nor is WM_QUERYENDSESSION in itself. We are discussing how to use WM_QUERYENDSESSION to save the necessary data so that the next Firefox session won't think Firefox crashed. This has nothing to do with any popup windows or confirmation dialogs.

Fixing this will probably solve bug 345345.

Taking this back.

This patch will shut down the browser forcefully but cleanly when we receive WM_ENDSESSION. It avoids the problems with WM_ENDSESSION by never dispatching that message, so Windows waits until the process can shut down cleanly. Unfortunately, this doesn't trigger session-save, so when the user starts up next time we don't restore their session. I consider this a bug in the patch (need to discuss with dietrich).

(In reply to comment #47) > Unfortunately, this doesn't trigger session-save Why should it? Just because the user currently doesn't have a chance of canceling the shutdown at WM_QUERYENDSESSION? If we'd handle that one as well, there would be no reason to do something different in the OS shutdown case than in the app quit manually case. Dispatching the "quit-application-requested" and "quit-application-granted" notifications at WM_QUERYENDSESSION would have the further benefit of SessionStore updating its data one last time in case the session should be resumed. Otherwise you might lose a tiny amount of data (at worst the last 10 seconds of work).

I am of two minds about WM_QUERYENDSESSION, but let's solve the WM_ENDSESSION bug first. In any case, nsIAppShutdown.quit(eAttempQuit) ought to be all that's necessary for it... if there are notifications such as quit-application-requested that should be fired, that method ought to fire them, instead of having that logic spread throughout the tree. nsCloseAllWindows.js is something of a disgrace.

> If we'd handle that one as well, there would be no reason to do something > different in the OS shutdown case than in the app quit manually case. I thought the whole point in this bug is that OS shutdown must behave equivalently to quitting manually. Can you think of any usecase where the behaviour should be different?

(In reply to comment #50) > I thought the whole point in this bug is that OS shutdown must behave > equivalently to quitting manually. Can you think of any usecase where the > behaviour should be different? Definitely, as not all system shutdowns are equal. In fact, AIUI, from a Windows perspective, there's two types of shutdowns: - user initiated - system initiated User initiated shutdowns should be cancel-able by the user if it turns out they forgot to complete an action on a webpage, or didn't realize Firefox was still open, or whatever. Same for other applications that feel like they should verify shutdown actions (word processors, etc) and I *think* that Windows provides a mechanism for signalling back to the OS the fact that "the user said he didn't actually want to quit, so don't shut down". System initiated shutdowns are ones where the OS is forcing a shutdown to apply a security update (famously, Windows does this overnight for people who don't shut down their systems) or something like that. In cases like that, Windows ignores the callback to cancel the shutdown, and in cases like that, Session Saver will make a lot of our users happy. My feeling is that user-initated shutdowns should show the "Do you want to close N tabs?" warning, and a cancel action there should cancel the shutdown. When it's a system-forced shutdown, session-saver should be invoked as the process is killed. Note: it might turn out that system-initiated shutdowns just ignore the callback to cancel the shutdown and kill the process at the OS level; in those cases, I think, session saver automatically saves the day. Regardless, that's the user experience that I think we want to put forward: observe the user's request to cancel a shutdown if we can, save their session if we can't, and if they accept the shutdown, then we shutdown normally.

(In reply to comment #51) > My feeling is that user-initated shutdowns should show the "Do you want to > close N tabs?" warning, and a cancel action there should cancel the shutdown. > When it's a system-forced shutdown, session-saver should be invoked as the > process is killed. > I have a patch for this dialog, as beltzner may have seen already. I will attach it to a bug RSN and make this bug depend on that one. I also have a pretty good idea of how to hook windows and X11 up to this dialog. Mind if I take this bug?

Please be aware that on XP and up both the WM_QUERYENDSESSION and WM_ENDSESSION cannot be blocked for more than 5 seconds before the OS intervenes. This may mean it is better to just save-session-and-quit instead of promting. Link: http://msdn2.microsoft.com/en-gb/library/bb394721.aspx

Michael, I'm happy for you take this bug, but I do want to make sure that we put the important smarts into nsAppStartup, instead of spreading it around even further ;-) (and be careful about how you treat WM_ENDSESSION, as noted above, because dispatching that event will force-kill the browser).

Re. Mike Beltzner's comment on the user experience I would like to comment that the described behaviour of asking the user wether he "really" wants to shut down is something I find very annoying. If the user says that he wants to shut down he expects the PC to shut down and don't bother him with questions. Word processors and the like only ask the user to save the current file if he has unsaved changes in that file. They never ask him wether he wants to shut down. The user's expectation is that FF behaves as if the "close window" button has been clicked and you wouldn't show a confirmation dialog then. I think it is important that FF's behaviour is consistent. "Close window" and shutdown should have the same behaviour. So either one puts a confirmation dialog in both or in neither.

I think ideally we'd detect if there is unsubmitted form information and prompt then. Not sure what other cases there would be for not just shutting down. Download in progress is another. Being prompted *Every* time, especially now that doing a system shutdown won't corrupt the profile is just going to replace one annoyance with another (Although I assume we can disable this one if it gets implemented).

I agree 100% with Frank, and I find it amazing that someone had to spell it out; I thought it was obvious that inconsistent behaviour is bad UI. majken@gmail.com's comments are irrelevant to this bug. Maybe we should warn about unsaved form information when Firefox is closed in general, but certainly not in the special case where Windows is shut down -- that would be inconsistent. Shutting down Windows must behave the same as closing Firefox, and once it does, we can address majken@gmail.com's ideas in a separate bug.

Timwi - any real discussion of shutdown UI is irrelevant to this bug. The point of this is to prevent profile corruption and stop Firefox from thinking it crashed (or stop it from crashing whatever it was actually doing). I was replying to beltzner and never said anything about only doing it in one situation.

Frank, the problem with your assertion is that you do have unsaved data, your current session and tab set. This is why, by default, closing a window with multiple tabs prompts for confirmation. On trunk right now, File->Exit will prompt the user to save their session and quit, cancel, or just quit. I think we should show that prompt (if the user hasn't disabled it) and cancel the OS shutdown if the user choose Cancel. Its not special-casing shutdown, its following the same behaviour as the app normally exhibits. It already works that way on Mac in Firefox 2, and its saved me a few dozen times in the last couple of years, since I usually have something I need to submit or bookmark for later.

Unless I am mistaken, the notification that Windows sends to a program upon Windows shutdown, is exactly the same notification that is send if a user manually ends the process from the task manager. If the above is in fact true, than this bug would have to be a WONTFIX due to Windows deficiencies. I am 100% sure we don't want to make a manual end process be treated as a normal shutdown.

Another point I just thought of. IF the browser treats windows shutdown as a normal shutdown, then if I lose power so my UPS system causes a windows shutdown I will not be able to recover my browser session even though I have it set to save sessions on browser crash. I really think this needs to be WONFIXed and people who complain about this should be told that they should close fFrefox before windows shutdown. It seems that this bug is trying to break the way things work for people who do things correctly for the benefit of people who insist on doing things incorrectly. Just my $.02.

Bill, I'm not sure what your point is: we have known dataloss caused by the abrupt shutdown of the eventloop and the ability to prevent it: the points in question are only the behavior of WM_QUERYENDSESSION and not WM_ENDSESSION.

(In reply to comment #62) > Bill, I'm not sure what your point is: we have known dataloss caused by the > abrupt shutdown of the eventloop and the ability to prevent it: the points in > question are only the behavior of WM_QUERYENDSESSION and not WM_ENDSESSION. > I guess I got between what what the intent of the bg is and what commenters have been suggesting as a solution. If the intent is to prevent data-loss if Firfox is running when windows is shutdown, then I am all for it. It sounded as if the intent was to treat a Windows shutdown while Firefox is running identically to the case where the user requested a browser exit. I have serious issues with that approach largely because I would expect my session to be saved if I have is set to save session on crash in either of the following 2 conditions. 1. If the UPS system initiated the Windows shutdown. 2. If this is a multi-user Windows Server system and i ma one of fifty users logged in and browsing the web when system admin does a server shutdown. It sounded as if this bug was heading in a direction to break expected behavior in both of those cases. As long as both of those cases are treated as a browser crash so that the session in saved and restored I have no issue with this. Avoiding the data-loss is fine. Treating it as a normal shutdown is not.

(In reply to comment #62) > Unless I am mistaken, the notification that Windows sends to a program > upon Windows shutdown, is exactly the same notification that is send > if a user manually ends the process from the task manager. End Task in Task Manager is the same as kill -9 -- the process is instantly terminated. To terminate every program when you go to Start > Shutdown would be utterly ridiculous. Think about it. (That *is*, however, the behaviour you get if you press the power button with that button set to shut down the computer -- all processes are terminated without prejudice before logout). No, when you ask to log out or shut down Windows, every program is politely asked to quit the same as on any other platform. The program can ask a question like "Do you wish to save?" or "There are still downloads in progress, quit?" If you click No or Cancel, shutdown is aborted. Firefox trips over itself at this point on any platform and terminates itself and loses data. There *may* be a Windows deficiency, but I've never had any problems with any other Windows software responding to this message, nor have I had any bother on Mac OS 9 or Mac OS X with other software. (Normally; occasionally it may a bit spoggly.) It's just Firefox.

(In reply to comment #64) > (That *is*, however, the behaviour you get > if you press the power button with that button set to shut down the computer -- > all processes are terminated without prejudice before logout). No, it's not, at least on my PC. I just tried it with Notepad on winxp sp2. I get the Yes/No/Cancel dialog, and pressing cancel aborts the shutdown process. Just like a regular shutdown, and just like pressing Alt+F4 with the app in focus.

(In reply to comment #64) > No, it's not, at least on my PC. Ah, sorry, that might be a 2000/XP difference. I have to remember that XP changed a few things :)

This is the last patch modified to work on 1.8.1 branch.

Comment on attachment 271939 [details] [diff] [review] 333907v1-branch181.patch If I apply this patch after building and rebuild the widget/toolkit area's, it works. On a clean build it breaks the build. I think it is a dependency thing with appcomps.

This is a combination of patches that finally actually builds from a clean source tree and works on 1.8.1 branch. Note that it is based very much on Benjamin's patch, but has one very important addition to actually enable getting the nsIAppShutdown service.

Comment on attachment 271728 [details] [diff] [review] Handle WM_QUERYENDSESSION and WM_ENDSESSION, v1 widget cannot depend on appcomps, because widget is tier_gecko while appcomps is tier_toolkit. This is the whole point of the nsIAppShutdown stuff in the previous patch.

Hm, I thought this wouldn't work (which is why I went for sticking stuff in nsAppShell.cpp), but it seems to work just fine on my Vista VM.

Comment on attachment 272574 [details] [diff] [review] Handle WM_QUERYENDSESSION and WM_ENDSESSION, v2 hrm, if multiple app windows are open does the app shut down all the way? And does this work even if no-remote is used? if so, this is great, and we should use an equivalent branch patch

I had looked at doing the same thing (1.8.1 branch), but just put a fprintf to stderr in there to see if I was getting the messages, I did not, so I didn't pursue it further. (In reply to comment #74)

(In reply to comment #74) > (From update of attachment 272574 [details] [diff] [review]) > hrm, if multiple app windows are open does the app shut down all the way? > It should. > And does this work even if no-remote is used? > Hm, unfortunately not. However, it should be doable. > if so, this is great, and we should use an equivalent branch patch > A branch patch would be different since there's no nsTryToClose.js component and manually calling tryToClose on every window is required for proper firefox shutdown on branch.

Checking in toolkit/xre/nsNativeAppSupportWin.cpp; /cvsroot/mozilla/toolkit/xre/nsNativeAppSupportWin.cpp,v <-- nsNativeAppSupportWin.cpp new revision: 1.30; previous revision: 1.29 done

is this a wontfix for 1.8.1?

We now correctly get quit-application-requested and quit-application-granted at shutdown, quit-application seems still to be missing though, which has the consequence that SessionStore still considers this a crash. This could be remedied by forcing sessionstore.js to be written to disk at quit-application-granted as well -- which however won't help the new Quit dialog, since that one seems to set the .resume_session_once pref after SessionStore gets quit-application-granted and has already deleted sessionstore.js (and won't rewrite it precisely due to the missing quit-application). So, any idea why "quit-application" doesn't get sent?

(In reply to comment #79) > So, any idea why "quit-application" doesn't get sent? > quit-application is sent in the app-startup quit function after closing all windows. Not sure how that can happen unless appStartup is null or quit fails to close all windows.

Missing the freeze, moving out.

Hiding windows instead of destroying them prevents Windows from prematurely terminating us. (Once we receive WM_ENDSESSION with wParam == TRUE, the application will be terminated, so leaking mWnd is no big deal at this point)

Comment on attachment 274538 [details] [diff] [review] Hide windows during WM_ENDSESSION Need more testing..

Comment on attachment 274538 [details] [diff] [review] Hide windows during WM_ENDSESSION This has issues with multiple windows for some reason..

This seems to work pretty well.

(In reply to comment #86) > Created an attachment (id=275201) [details] > Avoid hiding windows during ENDSESSION > > This seems to work pretty well. > Err, ignore the comments about hiding windows.. I don't bother doing that anymore. I'll fix the comments at checkin.

Comment on attachment 275201 [details] [diff] [review] Avoid hiding windows during ENDSESSION Boy... do you really prefer this to the original patch I put in this bug? This makes me feel nervous because we have control over our own windows but not necessarily external windows such as plugins.

(In reply to comment #88) > (From update of attachment 275201 [details] [diff] [review]) > Boy... do you really prefer this to the original patch I put in this bug? This > makes me feel nervous because we have control over our own windows but not > necessarily external windows such as plugins. > The original patch cannot cancel shutdown as far as I can tell. The OSX and Linux code can cancel shutdown, so I think Windows should also be able to do that.

FYI, you should take vista into consideration, it is behaving differently than xp for the patch I've tried to use. http://msdn2.microsoft.com/en-us/library/ms700677.aspx WM_QES can receive a ENDSESSION_FORCEFULSHUTDOWN flag which foreshadows the fact that you cannot cancel an endsession request.

This version fixes the patch for 1.8.1 branch, was not correctly setting return values before.

Checking in widget/src/windows/nsWindow.cpp; /cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v <-- nsWindow.cpp new revision: 3.703; previous revision: 3.702 done Checking in widget/src/windows/nsWindow.h; /cvsroot/mozilla/widget/src/windows/nsWindow.h,v <-- nsWindow.h new revision: 3.239; previous revision: 3.238 done

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007081013 Minefield/3.0a8pre Works as expected. Thanks to Michael and Benjamin for fixing this issue.

Michael, can you prepare a combined patch for the 1.8 branch? I suspect this will solve a lot of the "my bookmarks/prefs have disappeared" issues we've been seeing.

Created testcase in litmus -> setting in-Litmus to +

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102809 Minefield/3.0a9pre Still seeing the same dialog after a system shutdown.

Shall I reopen this bug or file a new one?

(In reply to comment #100) > Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102809 > Minefield/3.0a9pre > > Still seeing the same dialog after a system shutdown. I can't reproduce with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007110103 Minefield/3.0a9pre but bug 212251, surprisingly, isn't fixed. And what about bug 212316, which looks like it has been fixed by attachment 272574 [details] [diff] [review] ?

Fixed? Well I have been using Linux FC6, and the bug is there, on 2.0.0.9. Will try windows again, one of this days. Probably a more general issue than originally thought.

(In reply to comment #103) > Fixed? Well I have been using Linux FC6, and the bug is there, on 2.0.0.9. The fixed (or otherwise) status refers to the trunk (that is, the builds leading towards Firefox 3), so whether it appears on 2.x.x.x on any platform is not relevant.

I tried this bug also on Vista, latest trunk build and with the same result: this bug is not yet fixed. Are there people who see this fixed? Steps to reproduce: - Run Minefield with a new profile - While it is open click Start -> Shut Down - Start your computer and start the same browser Result: a dialog pops up with the choice "Restore Previous Session" or "Start New Session".

ok meanwhile i tried 2.0.0.9 on xp, and it's there as well, now i understand, but as this is so stupidly nagging if firefox does not do a front jump to 3.0 I do think we will be losig some users on the way. Take care

hmm branch patch has been sitting for a few months. What's the status, should it get review? Lots of users hit this issue, definitely a retention problem wrt bookmarks and 3 is still several months off. Requesting branch blocking to get it on the right radars.

I've been using the 1.8 branch patch in Komodo since I posted it to this bug, and it did fix our shutdown issues.

IE7 doesn't bug users on startup after a simple reboot - it just starts. Now that IE7 has tabs and a popup-blocker, I'm migrating my wife and children's laptops away from Firefox to IE7, because I'm fed up with them asking me "what should I answer to this question? - Firefox didn't crash I just rebooted my laptop - and what is a session anyway?". Fix it quick in the current release branch guys, or I suspect you're going to have a mass exodus on your hands.

While this would be nice to have on the branch, it is not a "blocker". A branch patch can be considered for approval if - you get reviews on the branch patch - fix the fact that ifaces are changed on the 1.8 branch (need _MOZILLA_1_8_BRANCH uglification or explicit approval from bsmedberg or brendan that this iface is an exception) - get an owner that will drive this in - request approval on the appropriate branch patch