Last Comment Bug 767835 - new tabs show about:privatebrowsing with "Never remember history"
: new tabs show about:privatebrowsing with "Never remember history"
Status: RESOLVED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: Firefox 16
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
Depends on:
Blocks: 763468
  Show dependency treegraph
 
Reported: 2012-06-24 15:10 PDT by Christian Ascheberg
Modified: 2012-09-11 07:32 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified


Attachments
Patch (v1) (1.42 KB, patch)
2012-06-25 10:00 PDT, :Ehsan Akhgari (busy, don't ask for review please)
ttaubert: review+
Details | Diff | Review
Patch (v2) (2.77 KB, patch)
2012-06-26 19:42 PDT, :Ehsan Akhgari (busy, don't ask for review please)
ttaubert: review+
Details | Diff | Review

Description Christian Ascheberg 2012-06-24 15:10:09 PDT
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] 2012-06-24 15:21:42 PDT
(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 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-25 10:00:44 PDT
Created attachment 636362 [details] [diff] [review]
Patch (v1)
Comment 3 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-25 10:01:22 PDT
(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] 2012-06-25 10:55:39 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-25 11:00:33 PDT
I think so, yes.
Comment 6 Tim Taubert [:ttaubert] 2012-06-25 11:15:39 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-25 11:29:46 PDT
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] 2012-06-25 11:40:34 PDT
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 2012-06-25 12:31:48 PDT
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] 2012-06-25 12:34:40 PDT
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 2012-06-25 12:44:17 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-25 21:06:07 PDT
(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 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-25 21:07:32 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e195dd3a989
Comment 15 Tim Taubert [:ttaubert] 2012-06-26 06:32:07 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-26 19:42:38 PDT
Created attachment 636978 [details] [diff] [review]
Patch (v2)
Comment 17 Tim Taubert [:ttaubert] 2012-07-03 02:28:55 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-03 07:14:59 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca8d61d7993
Comment 19 Ryan VanderMeulen [:RyanVM] 2012-07-03 16:07:56 PDT
https://hg.mozilla.org/mozilla-central/rev/0ca8d61d7993
Comment 20 Virgil Dicu [:virgil] [QA] 2012-09-11 07:32:38 PDT
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.

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