Last Comment Bug 660785 - Tabs are always restored at startup after closing in Private Browsing mode
: Tabs are always restored at startup after closing in Private Browsing mode
Status: VERIFIED FIXED
[testday-20110930]
:
Product: Firefox
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 8
Assigned To: Jez Ng [:int3]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-31 04:18 PDT by severinbreuer
Modified: 2011-11-21 14:13 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (1.24 KB, patch)
2011-06-14 14:21 PDT, Jez Ng [:int3]
paul: review-
Details | Diff | Review
Patch v2 (1.58 KB, patch)
2011-07-30 23:35 PDT, Jez Ng [:int3]
paul: review+
Details | Diff | Review

Description severinbreuer 2011-05-31 04:18:48 PDT
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

If you exit FF in Private Browsing mode, restarting has the same effect as exiting Private Browsing from the menu: pre-Private Browsing tabs are restored. This occurs even if a user has selected "Show a blank page" or "Show my home page" (as opposed to "Show my windows and tabs from last time") as the option for "When Firefox starts". 

Restoring tabs when exiting Private Browsing is sensible, but start-up behavior should be the same whether Firefox was closed in regular or Private Browsing mode. If a user has "Show my windows and tabs from last time" enabled, Firefox should (and does) restore the pre-Private session when Firefox is restarted. But if the user has enabled "Show my home page", the home page should be shown, not the tabs from the previous non-private session. Users can always restore tabs with "Restore Previous Session" in the History menu if they need to. But if a user closes the browser (rather than exiting Private Browsing via the menu), then Firefox should close both the private session and the suspended non-private session unless a user has selected to have sessions restored at each startup.

Reproducible: Always

Steps to Reproduce:
1) Start Firefox. Set start-up option to "Show my home page" and set a home page.
2) Switch to Private Browsing.
3) Close Firefox.
4) Start Firefox.

Actual Results:  
Firefox restores the tabs that were open before switching to Private Browsing.

Expected Results:  
Firefox should display the home page.
Comment 1 :Ehsan Akhgari (out sick) 2011-05-31 13:50:24 PDT
This behavior is by design, because otherwise those tabs would be lost forever.
Comment 2 severinbreuer 2011-05-31 14:27:01 PDT
Where do you get the idea that they'd be lost forever? They're listed plain as day in the History menu. And if the suspended non-private session was simply closed as expected when Firefox was closed, the Restore Previous Session menu option in that same menu should restore them just fine.
Comment 3 :Ehsan Akhgari (out sick) 2011-06-13 10:01:11 PDT
Alex, comment 2 doesn't seem like a bad idea to me.  How would you feel about us treating these types of sessions as any other session, and show a Restore Previous Session button in about:home in the cases that we do now for regular sessions?

zpao: I actually don't know how the sessionstore code triggers this new way of restoring sessions.  How hard would it be for us to get this to work for private browsing mode too?
Comment 4 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-06-13 10:16:45 PDT
(In reply to comment #3)
> zpao: I actually don't know how the sessionstore code triggers this new way
> of restoring sessions.  How hard would it be for us to get this to work for
> private browsing mode too?

Should be easy (famous last words). I think we just need to stop setting browser.sessionstore.resume_session_once here: https://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/nsSessionStore.js#635
Comment 5 :Ehsan Akhgari (out sick) 2011-06-13 12:14:58 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > zpao: I actually don't know how the sessionstore code triggers this new way
> > of restoring sessions.  How hard would it be for us to get this to work for
> > private browsing mode too?
> 
> Should be easy (famous last words). I think we just need to stop setting
> browser.sessionstore.resume_session_once here:
> https://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/nsSessionStore.js#635

FWIW, I had already tried that, but it doesn't seem to work.
Comment 6 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-13 13:31:15 PDT
We didn't want starting private browsing mode to be as destructive as closing the entire application.  Ideally we should have private windows running simultaneously, but until we get there I want us to keep this behavior in place so users can enter and leave private browsing with the least amount of extra overhead actions.

While your session is available in history, the assumption is that you want it back (just as when we crash the assumption is that you want it back, and we in that case also restore it automatically).
Comment 7 :Ehsan Akhgari (out sick) 2011-06-13 13:35:16 PDT
(In reply to comment #6)
> We didn't want starting private browsing mode to be as destructive as closing
> the entire application.  Ideally we should have private windows running
> simultaneously, but until we get there I want us to keep this behavior in place
> so users can enter and leave private browsing with the least amount of extra
> overhead actions.

This is not about whether or not to automatically get the original session back when you leave the private browsing mode.  It's about whether that should happen after a restart if you had closed Firefox without leaving the private browsing mode.
Comment 8 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-13 13:45:55 PDT
Oh, yeah then we should just follow normal startup behavior (whatever the user set that as).
Comment 9 :Ehsan Akhgari (out sick) 2011-06-13 14:42:47 PDT
OK, reopening then!  zpao, I'd assign this to you, except that I don't want to overload you if you don't have time!  But you may be the best person to fix this :-)
Comment 10 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-06-14 11:52:46 PDT
I'm going to have Jezreel start looking at this.
Comment 11 Jez Ng [:int3] 2011-06-14 14:21:50 PDT
Created attachment 539320 [details] [diff] [review]
Patch v1

I tried the fix in comment #4 and it seems to work fine.
Comment 12 :Ehsan Akhgari (out sick) 2011-06-14 14:41:47 PDT
(In reply to comment #11)
> Created attachment 539320 [details] [diff] [review] [review]
> Patch v1
> 
> I tried the fix in comment #4 and it seems to work fine.

Hmm, can you please describe your STRs?
Comment 13 Jez Ng [:int3] 2011-06-14 14:57:14 PDT
Steps to reproduce:

1. Apply patch in comment #11
2. Set pref to 'Show my home page'
3. Open up google.com, facebook.com
4. Enter private browsing mode
5. Quit Nightly
6. Reopen Nightly

Results:
Only the homepage is shown upon reopening.

I've tried the other two pref options in step #2 as well, and they both give the desired result upon reopening.
Comment 14 :Ehsan Akhgari (out sick) 2011-06-14 15:09:07 PDT
Hmm, then I don't know what I was doing wrong.  If zpao thinks that this patch is good, I think we should take it.
Comment 15 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-26 13:32:04 PDT
Comment on attachment 539320 [details] [diff] [review]
Patch v1

Review of attachment 539320 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/sessionstore/src/nsSessionStore.js
@@ +632,5 @@
>  
>              this._saveStateObject(oState);
>            }
> +          // don't restore the non-private session automatically upon resuming (bug 660785)
> +          this._prefBranch.setBoolPref("sessionstore.resume_session_once", false);

Let's just not set the pref at all. If for some reason the user sets it (or an addon or whatever), we should respect that.

You can keep the comment (no need to have the bug number though) or say that we'll respect normal startup-related prefs and restore the session if applicable, otherwise it will be available under restore previous session.
Comment 16 Jez Ng [:int3] 2011-07-30 23:35:34 PDT
Created attachment 549629 [details] [diff] [review]
Patch v2

Updated patch as per the review.
Comment 17 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-01 16:23:19 PDT
Landed on fx-team: http://hg.mozilla.org/integration/fx-team/rev/e9c579848ab6
Comment 18 Tim Taubert [:ttaubert] 2011-08-02 05:27:32 PDT
http://hg.mozilla.org/mozilla-central/rev/e9c579848ab6
Comment 19 Virgil Dicu [:virgil] [QA] 2011-09-30 07:05:42 PDT
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20110929 Firefox/10.0a1
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20110929 Firefox/9.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0

Verified on Ubuntu 11.04, Windows XP, Windows 7 and Mac OS 10.6. 

The "When Firefox starts" preferences are remembered after the user quits Firefox while in Private browsing mode.
Comment 20 idealas 2011-11-20 04:22:57 PST
is there any way to get the older "buggy" behaviour? I would like it back since surprisingly I was using it as a feature :)
Comment 21 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-11-21 10:18:55 PST
(In reply to idealas from comment #20)
> is there any way to get the older "buggy" behaviour? I would like it back
> since surprisingly I was using it as a feature :)

You can set Firefox to restore your session automatically and your session should be restored following Firefox quitting from PB mode.
Comment 22 idealas 2011-11-21 14:13:39 PST
sure thing i can, unfortunately i want it to restore tabs only when i quite ff in private browsing, just like it was before fix with "dont restore tabs" selected.

Note You need to log in before you can comment on or make changes to this bug.