Entering Private Browsing via command line shows tabs from previous session

RESOLVED FIXED in Firefox 3.7a1

Status

()

Firefox
Private Browsing
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: Jim Jeffery not reading bug-mail 1/2/11, Assigned: Ehsan)

Tracking

Trunk
Firefox 3.7a1
Points:
---
Bug Flags:
blocking-firefox3.6 +
wanted-firefox3.6 +
in-litmus ?

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.57 KB, patch
mconnor
: review+
Benjamin Smedberg
: review+
Details | Diff | Splinter Review
Open several tabs.  Close Firefox/Minefield and if prompted use: Save & Quit
Enter into Private Browsing via command line:  firefox.exe -private
Note all your prior tabs are displayed. 

Part of the design considerations in bug 471997 was to open the command line session to the Homepage as stated in:
https://bugzilla.mozilla.org/show_bug.cgi?id=471997#c51

The fact that is shows prior tabs is not working IMO if entering via command line is to show Homepage or at least about:blank (especially in a kiosk setting)
Flags: blocking-firefox3.6?
(Assignee)

Comment 1

9 years ago
CCing Alex here to get input on the desired UE here.
Keywords: uiwanted
We've been down this discussion path before wrt autostart and previous session... what did we decide then?
That discussion was wrt autostart as a permanent decision (pref-based), which makes a lot more sense to not show the tabs for the previous session. Additionally, that setting takes affect immediately from the previous session, whereas here this is a startup command-line flag *after* the session has already been saved, so arguably the session should start as it always does, only in private browsing mode.

See bug 481786, bug 467565 comment 11 and others, bug 482334, and bug 482430 comment 12 and onwards. All deal with this issue in some way or another.
As a matter of intrest perhaps IE8 on Win7 RC you can use the 'jumplist', I know another bug on that exists, but for simplicity and comparison, when entering into: "InPrivate" from the jumplist it brings up the InPrivate page and no offers to 'restore' last session or links to that last session. 

If you close Firefox and Save&Close your session, then later decide to open via command line a PB session, it should be basically the same as using the jumplist in IE8 on Win7, blank - Plus there is the indicator you have PB enabled.
Launching Firefox into private browsing mode from the command line is a one time thing (it won't launch into private browsing mode again unless you use the command line argument again), so it seems like this is action more analogous to tools > start private browsing, as opposed to the autostart pref.  Under those assumptions, the most logical thing to do is not to display the user's session (also externally consistent with IEs behavior described in comment #4).
(Assignee)

Comment 6

9 years ago
(In reply to comment #2)
> We've been down this discussion path before wrt autostart and previous
> session... what did we decide then?

We decided not to restore the user's old session in that case (bug 482334).

Taking...
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
(Assignee)

Comment 7

9 years ago
Created attachment 395282 [details] [diff] [review]
Patch (v1)

This patch makes command-line-startup a globally observable notification so that the browser startup component can gain access to command line params before final-ui-startup.
Attachment #395282 - Flags: review?(zeniko)
Attachment #395282 - Flags: review?(bsmedberg)
(Assignee)

Updated

9 years ago
Keywords: uiwanted

Comment 8

9 years ago
Is there any reason for not setting nsIPrivateBrowsingService:autoStarted when this command line flag is set (so that all callers that already handle starting up in PB mode will also handle this flag correctly without any further change)?
OS: Windows NT → All
Hardware: x86 → All
(Assignee)

Comment 9

9 years ago
(In reply to comment #8)
> Is there any reason for not setting nsIPrivateBrowsingService:autoStarted when
> this command line flag is set (so that all callers that already handle starting
> up in PB mode will also handle this flag correctly without any further change)?

I do set it - the problem is that the command line arguments are handled after final-ui-startup, which means at that point nsSessionStartup.init has already been called.  :-(

Comment 10

9 years ago
(In reply to comment #9)
> I do set it - the problem is that the command line arguments are handled after
> final-ui-startup

So wouldn't the solution be to set it already when observing "command-line-startup" instead of waiting as long as you currently do? IOW: The code you're adding to nsSessionStartup.js could just as well go into nsPrivateBrowsingService in order to make autoStarted right soon enough.
(Assignee)

Comment 11

9 years ago
(In reply to comment #10)
> (In reply to comment #9)
> > I do set it - the problem is that the command line arguments are handled after
> > final-ui-startup
> 
> So wouldn't the solution be to set it already when observing
> "command-line-startup" instead of waiting as long as you currently do? IOW: The
> code you're adding to nsSessionStartup.js could just as well go into
> nsPrivateBrowsingService in order to make autoStarted right soon enough.

Yes, I think that would work.  I'll give it a shot and submit a new patch.
(Assignee)

Comment 12

9 years ago
Created attachment 396129 [details] [diff] [review]
Patch (v2)

Move the command-line-startup handling to nsPrivateBrowsingService
Attachment #395282 - Attachment is obsolete: true
Attachment #396129 - Flags: review?(mconnor)
Attachment #396129 - Flags: review?(bsmedberg)
Attachment #395282 - Flags: review?(zeniko)
Attachment #395282 - Flags: review?(bsmedberg)
Comment on attachment 396129 [details] [diff] [review]
Patch (v2)

r=me on the browser bits, will let bsmedberg weigh in on the rest, but seems kosher to me.
Attachment #396129 - Flags: review?(mconnor) → review+
Flags: wanted-firefox3.6+
Flags: blocking-firefox3.6?
Flags: blocking-firefox3.6-
(Assignee)

Comment 14

9 years ago
I think the blocking-firefox3.6 decision needs to be reconsidered now that bug 471997 has been deemed as a 3.6 blocker.
Flags: blocking-firefox3.6- → blocking-firefox3.6?

Comment 15

9 years ago
Comment on attachment 396129 [details] [diff] [review]
Patch (v2)

I really don't like this: I wish session restore was able to hook in to something later, like when the first browser window opens, or the default command-line handling happens.

Anyway, this is good enough and prevents our house of cards from tumbling down.
Attachment #396129 - Flags: review?(bsmedberg) → review+

Comment 16

9 years ago
This really really needs to end up in litmus.
Flags: in-litmus?
Flags: blocking-firefox3.6? → blocking-firefox3.6+
(Assignee)

Comment 17

9 years ago
http://hg.mozilla.org/mozilla-central/rev/cdcf1519121f
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Testing with latest hourly, this seems to be fixed, no longer seeing prvious tab set loaded, and get the Minefield Start page. 

I am seeing in Console2 however two errors:
Error: this._prefBranch is undefined
Source file: file:///J:/Program%20Files%20(x86)/Minefield/components/nsSessionStore.js
Line: 2616


Error: Exception thrown while processing the private browsing mode change request: [Exception... "'[JavaScript Error: "this._prefBranch is undefined" {file: "file:///J:/Program%20Files%20(x86)/Minefield/components/nsSessionStore.js" line: 2616}]' when calling method: [nsISessionStore::setBrowserState]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///J:/Program%20Files%20(x86)/Minefield/components/nsPrivateBrowsingService.js :: PBS__onAfterPrivateBrowsingModeChange :: line 238"  data: yes]
Source file: file:///J:/Program%20Files%20(x86)/Minefield/components/nsPrivateBrowsingService.js
Line: 390

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20090915 Minefield/3.7a1pre Firefox/3.7 (.NET CLR 3.5.30729) ID:20090915050128

cset: http://hg.mozilla.org/mozilla-central/rev/cdcf1519121f
(Assignee)

Comment 19

9 years ago
Do you see the same error in hourlies without this patch?  I seem to recall this error happening before...
(Assignee)

Comment 20

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/40ca2b5cc972
status1.9.2: --- → beta1-fixed
(In reply to comment #19)
> Do you see the same error in hourlies without this patch?  I seem to recall
> this error happening before...

Yes indeed.  Same error shows in an hourly without patch.  Guess its harmless, as the session appears to not be broken or anything.
You need to log in before you can comment on or make changes to this bug.