463188 - Restore user's session at startup if they quit Firefox during Private Browsing mode

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[(no longer active)]

Currently, if browser.startup.page is set to 3, session store will restore the previous session after exiting the Private Browsing mode.  I think we should make this the default behavior, even for users who don't have browser.startup.page set to 3, the user may simply close the browser in hopes of exiting the PB mode, and when they discover that Firefox has been shut down, they might try to start it again in the hopes of seeing their old session.

I think we should add a new pref for this, to make sure we don't mess with browser.startup.page's already set in users' profiles.

Comment 1

[Simon Bünzli]

Why not just set browser.sessionstore.resume_session_once to true when you get the quit-application-granted notification while still in PB mode?

Comment 2

[Justin Wood (:Callek)]

For what its worth, I'd suggest some way to ensure that if [x] time elapses we *dont* restore the session, as that might be counter to what the user expects, and could even be privacy problems (even if not Private Browsing Mode heavy problems)

The question of what value of x is probably another discussion

Comment 3

[Ria Klaassen (not reading all bugmail)]

If someone else starts the browser he will be confronted with all the secret browsing pages of the previous user, who may not be happy with this. Or do I misunderstand this bug?

Comment 4

[Justin Wood (:Callek)]

(In reply to comment #3)
> If someone else starts the browser he will be confronted with all the secret
> browsing pages of the previous user, who may not be happy with this.

Its about restoring with the session that was open _before_ Private Browsing Mode was entered. Where currently we "close" the previous session (restored when Tools->Private Browsing is unchecked). This is about actually restoring that session when the window is closed (and reopened).

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

Here was the initial design proposed (which may or may not actually be a good idea):

http://people.mozilla.com/~faaborg/files/shiretoko/privacy_i2.png
(scroll all the way down to the bottom to see the private browsing flow)

When the user closes the last private browsing window we automatically relaunch Firefox with their last session. The idea is that we are interpreting their action as "I want to stop private browsing" and not "I want to close Firefox."

Something that I am worried about is that the user opens several embarrassing tabs, and then opens up private browsing mode.  As the start page suggestions, they clear their recent history using the Clear Recent History dialog.  Of course, history isn't the same thing as active tabs (in the implementation model, but not the user's mental model), so these embarrassing tabs are persisted in the previous session.  The user eventually closes down Firefox, and leaves.  The next person to load Firefox will see the sensitive tabs, even though the user thought they were taking every action they needed to in order to cover their tracks.

Automatically restoring the previous session helps in some respect in that the user gets a chance to see everything from before they started private browsing.  However, if your intention is to close Firefox, the behavior may be kind of annoying.

Comment 6

[(no longer active)]

Attachment: Patch (v1)

(In reply to comment #1)
> Why not just set browser.sessionstore.resume_session_once to true when you get
> the quit-application-granted notification while still in PB mode?

Yes, thanks for the pointer.  Although I did that inside the private browsing exit notification in case the user is quitting Firefox...

(In reply to comment #5)
> Something that I am worried about is that the user opens several embarrassing
> tabs, and then opens up private browsing mode.  As the start page suggestions,
> they clear their recent history using the Clear Recent History dialog.  Of
> course, history isn't the same thing as active tabs (in the implementation
> model, but not the user's mental model), so these embarrassing tabs are
> persisted in the previous session.  The user eventually closes down Firefox,
> and leaves.  The next person to load Firefox will see the sensitive tabs, even
> though the user thought they were taking every action they needed to in order
> to cover their tracks.

Hmm, I see your concern.  I think one solution could be to take this patch, and warn the user if she invokes CPD in PB mode and selects "Visited Pages", perhaps even via a modal dialog.  Anyway, setting ui-r? on this in order to make a decision, but anyway what comment 0 explains is very odd at best, and will lead to data loss (when session store overwrites sessionstore.js).

Comment 7

[Simon Bünzli]

Comment on attachment 346700 [details] [diff] [review]
Patch (v1)

(In reply to comment #6)
> > Something that I am worried about is that the user opens several embarrassing
> > tabs, and then opens up private browsing mode.  As the start page suggestions,
> > they clear their recent history using the Clear Recent History dialog.

Easy (and correct) solution: Delete _stateBackup in SessionStore's "browser:purge-session-history" handler. r+=me with that change.

Comment 8

[(no longer active)]

(In reply to comment #7)
> Easy (and correct) solution: Delete _stateBackup in SessionStore's
> "browser:purge-session-history" handler. r+=me with that change.

Thanks.  But still I think doing that without prompting might surprise those users who don't have an embarrassing tab open in their non-private session.  Let's see what Alex thinks here first.

Comment 9

[Simon Bünzli]

Whatever you do, please don't forget to delete _stateBackup in the purge-session-history handler.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 346700 [details] [diff] [review]
Patch (v1)

I didn't actually test the patch, but am giving a ui-r+ based on described functionality.

Comment 11

[Alex Faaborg [:faaborg] (Firefox UX)]

>> Easy (and correct) solution: Delete _stateBackup in SessionStore's
>> "browser:purge-session-history" handler. r+=me with that change.
>
>Thanks.  But still I think doing that without prompting might surprise those
>users who don't have an embarrassing tab open in their non-private session. 
>Let's see what Alex thinks here first.

Really we should be adding tabs to the list of history items in the Clear Recent History dialog.  I talked to Johnath a bit about this today.

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

Filed follow up bug 463553 to cover the problem of keeping tabs open when the user is clearing their recent history.

Comment 13

[(no longer active)]

Attachment: For checkin

OK, this patch addresses Simon's comment in comment 7.  Asking approval to land for beta 2.

Comment 14

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/41b90187bda2

Comment 15

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=7428 added to Litmus.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ehsan, I tried to reproduce the former behavior by using a build from 11/06, but after a restart from within the PB mode the old session is always restored. Do I miss something?

Comment 17

[(no longer active)]

(In reply to comment #16)
> Ehsan, I tried to reproduce the former behavior by using a build from 11/06,
> but after a restart from within the PB mode the old session is always restored.
> Do I miss something?

How did you restart from within the PB mode?  If you restart from within the app (like for example after installing an add-on), then the session store components sets a pref to enable one-time restoring of the session on next startup.  My guess is that is what you've seen.

In order to reproduce this problem before this patch landed, you need to enter PB mode, close the browser from there, and open it again.  Sorry for not including an actual STR in comment 0.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #17)
> In order to reproduce this problem before this patch landed, you need to enter
> PB mode, close the browser from there, and open it again.  Sorry for not
> including an actual STR in comment 0.

That are the steps I've done. Entering PB mode and selecting "Minefield | Quit Minefield" from the menu. And after the restart the old session was restored.

Do I have to explicitly set "Show my home/blank page" in the preferences? Inserting this step I can reproduce it and with the build later then the 11/06 the last session is restored correctly. If these are the right steps we have to update the Litmus test.

Comment 19

[(no longer active)]

(In reply to comment #18)
> That are the steps I've done. Entering PB mode and selecting "Minefield | Quit
> Minefield" from the menu. And after the restart the old session was restored.
> 
> Do I have to explicitly set "Show my home/blank page" in the preferences?

If "Show my windows and tabs from the last time" setting is active in the preferences, then the user's old session is restored both before and after this patch.  To see the problem, either of the "Show my home page" or "Show a blank page" options should be selected in the preferences.

> Inserting this step I can reproduce it and with the build later then the 11/06
> the last session is restored correctly. If these are the right steps we have to
> update the Litmus test.

Yes, the tests needs to make sure one of those two settings are active before starting the testing process.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with the additional steps mentioned in comment 18 / comment 19.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090101 Shiretoko/3.1b3pre ID:20090101020559

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090101 Shiretoko/3.1b3pre ID:20090101020559

Comment 21

[Marcia Knous [:marcia]]

Litmus test case has been updated per Comment 19.