Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-birch/ Since Per-Window Private Browsing, Restore previous session option is available in Private Browsing window and session can be restored into this window. Steps: 1. Open some tabs in normal browsing (and make sure "Show my home page" is selected as startup option) 2. Restart Firefox 3. Open a private browsing window 4. While in private browsing window, restore previous session (from about:home page or from History menu) Expected result: Previous session is restored in normal browsing window Actual result: Previous session is restored in Private Browsing window. After that, you cannot restore that session in normal browsing mode.
Hmm, should we just disable the Restore Previous Session menu item in private windows altogether?
That seems like a logical choice to me.
Summary: Previous session is restored in Private Browsing window → Disable the Restore Previous Session command in per-window PB builds
Comment on attachment 690272 [details] [diff] [review] Patch (v1) You really should enable/disable the Browser:RestoreLastSession command as the window opens rather than while opening a menu containing such a menu item.
Attachment #690272 - Flags: review?(dao) → review-
Summary: Disable the Restore Previous Session command in per-window PB builds → Disable the Restore Previous Session command in private windows
I do this in delayedStartup because canRestoreLastSession doesn't have the correct value before the onload handler for the first window has been run, apparently.
Comment on attachment 690361 [details] [diff] [review] Patch (v1) (In reply to Ehsan Akhgari [:ehsan] from comment #5) > I do this in delayedStartup because canRestoreLastSession doesn't have the > correct value before the onload handler for the first window has been run, > apparently. This is because the sessionstore service is initialized in delayedStartup. Please move your block up there such that we don't get the service twice. >+ let ss = Cc["@mozilla.org/browser/sessionstore;1"]. >+ getService(Ci.nsISessionStore); Indentation seems random here. I'll also lobby against the trailing dot and suggest this: let ss = Cc["@mozilla.org/browser/sessionstore;1"] .getService(Ci.nsISessionStore); However, this also looks like it fits well on one line. Although we do avoid overly long lines, my feeling is that we're not pedantic about enforcing an 80 characters limit in most browser code. >+ let restoreItem = document.getElementById("Browser:RestoreLastSession"); >+ if (ss.canRestoreLastSession >+#ifdef MOZ_PER_WINDOW_PRIVATE_BROWSING >+ && !PrivateBrowsingUtils.isWindowPrivate(window) >+#endif >+ ) >+ restoreItem.removeAttribute("disabled"); goSetCommandEnabled("Browser:RestoreLastSession", false);
Attachment #690361 - Flags: review?(dao) → review-
(In reply to Dão Gottwald [:dao] from comment #6) > >+ restoreItem.removeAttribute("disabled"); > > goSetCommandEnabled("Browser:RestoreLastSession", false); Err, s/false/true/
I don't see any valid reason why the getService() call would fail unless things are otherwise seriously broken. I would leave it out of the try/catch and remove the null check for "ss".
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
Is this already covered by an automated test?
It does not appear to be.
Flags: in-testsuite? → in-testsuite-
(In reply to Josh Matthews [:jdm] from comment #13) > It does not appear to be. Thanks! The creation of such a test is now tracked in bug 856043.
You need to log in before you can comment on or make changes to this bug.