It would be useful to have a 'Recently Closed Windows' menu that works as for 'Recently Closed Tabs' (and maybe an Undo Close Window keyboard shortcut too). While you're less likely to accidentally close a window than a tab, it can still be useful to go back to an earlier window, and it would be cheap to implement. Implementation -------------- Bug 344642 removed the Undo Close Window API from the SessionStore API because it wasn't being used in Fx2 (and there wasn't time to test it). Reverting that would provide the necessary backend, and it would just need a menu and shortcut hooking up. Alternative ----------- If this isn't implemented, then it'd be good to change the SessionStore API so extensions can at least do this themselves without horrific hacks. At the moment one would expect the following code to dump data on closed windows, but getWindowState throws an exception because in the nsSessionStore service's own "domwindowclosed" observer it voids the window data (dietrich confirmed the problem on IRC). Components .classes["@mozilla.org/observer-service;1"] .getService(Components.interfaces.nsIObserverService) .addObserver( { observe: function(aSubject, aTopic, aData) { var state = Components .classes["@mozilla.org/browser/sessionstore;1"] .getService(Components.interfaces.nsISessionStore) .getWindowState(aSubject); dump(state); } }, "domwindowclosed", false ); Note: the window state actually gets invalidated at http://lxr.mozilla.org/mozilla1.8/source/browser/components/sessionstore/src/nsSessionStore.js#481

With the current implementation, you're actually supposed to get the closed window's state on unload and not on domwindowclosed.

(In reply to comment #1) > With the current implementation, you're actually supposed to get the closed > window's state on unload and not on domwindowclosed. > Yes but, somewhat counter-intuitively, unload gets called after domwindowclosed does. For example: window.addEventListener("unload", function() { alert(window.__SSi); }, false); will alert "undefined", because the __SSi will have already been deleted from the window. Hence calling getWindowState from an unload handler... window.addEventListener("unload", function() { try { alert(Components.classes["@mozilla.org/browser/sessionstore;1"] .getService(Components.interfaces.nsISessionStore) .getWindowState(window)); } catch (ex) { alert(ex); } }, false); ... will always give you an error like this: [Exception... "'[JavaScript Error: "aWindows[i] has no properties" {file: "file:///C:/Program%20Files/Mozilla%20Firefox%202/components/nsSessionStore.js" line: 1013}]' when calling method: [nsISessionStore::getWindowState]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "JS frame :: chrome://quickprompt/content/quickprompt.js :: anonymous :: line 224" data: yes]

I can confirm that by the time the unload event fires SessionStores window state has already been wiped and calling getWindowState at that point results in an error.

Work-around for reference: In an overlay to browser.xul call eval("closeWindow = " + closeWindow.toString().replace("if (aClose)", "mySaveWindowState(); $&")); where |mySaveWindowState| is a function calling nsISesionStore::getWindowState for |window| in order to further process that state.

One quite cheap solution would be to simply cache the closing window's state inside the window itself and fall back to that one where it makes sense (IMO mainly getWindowState and getWindowValue - and getTabValue should still work anyway). Such a patch should allow extensions to use the two patterns suggested in comment #0 and comment #2.

The patch looks good and the approach sensible, though I haven't tested it in action. For consistency it might make sense to do the same for getClosedTabCount and getClosedTabData, just in case people try and read those.

Dietrich: any chance you might get to review this before it's too late?

Comment on attachment 265000 [details] [diff] [review] enable all read-only APIs for closing windows i'd prefer either a new api, or if you re-use the existing apis add an explicit parameter or use a parameter value that indicates that the caller wants the last-closed-window. either way, please update the idl docs as well.

Comment on attachment 265000 [details] [diff] [review] enable all read-only APIs for closing windows (In reply to comment #9) > add an explicit parameter or use a parameter value that indicates > that the caller wants the last-closed-window. Uhm, the caller doesn't want the last-closed-window, she wants the window she passes anyway (aWindow) - and that's what she gets. All this patch does is unbreak one edge-case we so far haven't handled well at all - there are no API changes or anything else. As for the (somewhat unfortunately out-of-context) change referring to _lastWindowClosed - we've just set that value 10 lines above to contain the data for the current window (AKA this._windows[aWindow.__SSi] ), you might thus want to consider the complete function as changed and not just what the patch shows you. Care to have a second look? Thanks.

Comment on attachment 265000 [details] [diff] [review] enable all read-only APIs for closing windows ugh, my misunderstanding. yes, this looks fine for fixing the closing windows problem.

Thanks.

Checking in browser/components/sessionstore/src/nsSessionStore.js; /cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v <-- nsSessionStore.js new revision: 1.70; previous revision: 1.69 done

Bug closed as fixed, but where is "undo window close" button?

This bug didn't add a "undo window close" button. It just made it so the window state can still be read while the window is closing. Basically it changed it so the "Alternate" code from comment #0 works. You'll need to use an extension like Tab Mix Plus or Session Manager if you want to be able to restore closed windows. Or you can file another bug requesting the feature be added to Firefox itself.

Verified with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090617 Minefield/3.6a1pre ID:20090617031528

in-litmus+ https://litmus.mozilla.org/show_test.cgi?id=7794