767835 - new tabs show about:privatebrowsing with "Never remember history"

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Christian Ascheberg]

1. Set history settings to "Never remember history"
2. Open new tab
Result: about:privatebrowsing is shown
Expected: show about:blank for example, as changing the history mode permanently is not really the same as private browsing

Also, custom settings for the browser.newtab.url pref are not respected, as suggested in bug 762938 comment #13

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Christian Ascheberg from comment #0)
> Also, custom settings for the browser.newtab.url pref are not respected, as
> suggested in bug 762938 comment #13

Thank you for pointing this out. We totally overlooked it. I filed bug 767836 for this specific issue.

Comment 2

[(no longer active)]

Attachment: Patch (v1)

Comment 3

[(no longer active)]

(I'd include a test, but unfortunately it's not possible to test the autoStarted mode in browser-chrome, since we read the value of the respective pref on profile-after-change.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

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

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

Is showing about:newtab as the new tab page the right choice for this scenario?

Comment 5

[(no longer active)]

I think so, yes.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

I was just wondering what's the typical use case for this feature. If that's a specific profile always auto-run in PB mode, then showing about:newtab would be rather useless because there's nothing to show and we're not allowing additions to it, right? It's only useful if the user has gathered a "normal" history before he turned on this feature. Please correct me if I'm wrong.

Comment 7

[(no longer active)]

We have no way of knowing that.  The user may turn on that pref at any time, although the possibility of somebody installing Firefox for the first time and immediately toggling that pref is not great.

We can also default to about:blank in this case.  It's not immediately obvious if that would be an improvement though.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

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

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

Right, hard to tell... I think it's better than showing about:privatebrowsing, though. So r=me and we'll see how it goes.

Comment 9

[Christian Ascheberg]

about:newtab does not seem to display bookmarks (if there is no history). Is that correct? Could that be changed to fill empty cells?

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Right, there's no data if you never had any history in your profile. Not sure what we should fill the empty cells with...

Comment 11

[Christian Ascheberg]

What I tried to say is: include actual bookmarks, there might be some of them as they are persistent. It currently does not seem to do that.

Comment 12

[(no longer active)]

(In reply to Christian Ascheberg from comment #11)
> What I tried to say is: include actual bookmarks, there might be some of
> them as they are persistent. It currently does not seem to do that.

Please file another bug for that.

Comment 13

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e195dd3a989

Comment 14

[Ed Morley [:emorley]]

Backed out for mochitest-browser-chrome failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=4e195dd3a989

eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=12991945&tree=Mozilla-Inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c149cc0dd0c

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

Looks like this is called even before gPrivateBrowsingUI has been initialized. That also means we'll have to update the value of BROWSER_NEW_TAB_URL after it has been initialized.

Comment 16

[(no longer active)]

Attachment: Patch (v2)

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 636978 [details] [diff] [review]
Patch (v2)

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

Looks good! r=me assuming try is green.

::: browser/base/content/browser.js
@@ +6936,5 @@
> +  get initialized() {
> +    return this._inited;
> +  },
> +
> +  addInitializationCallback: function PBUI_addInitializationCallback(callback) {

Nit: aCallback.

Comment 18

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca8d61d7993

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0ca8d61d7993

Comment 20

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20100101 Firefox/16.0

Verified across platforms: Mac OS 10.7, Windows 7, Ubuntu 12.04. 
If a user has set a custom page for new tab, it will be displayed both in PB mode and with Clear Recent History option. With about:newtab, about:privatebrowsing isdisplayed in PB mode.