1177156 - Provide a button to re-enable Tracking Protection in about:privatebrowsing

Status: VERIFIED FIXED

(Firefox :: Private Browsing, defect, P1)

Description

[Matthew N. [:MattN]]

Add a link to about:privatebrowsing so users can disable tracking protection in private browsing.

Depending on the about page flags we may not be able to directly link to the page and instead could use UITour (after whitelisting the page) for this.

Comment 1

[Marco Mucci [:MarcoM]]

Hi Matt, is Bug 1177156 related to any of the user stories?

Comment 2

[Matthew N. [:MattN]]

Yep, bug 1175014, not sure how I missed that.

Comment 3

[:Paolo Amadini]

Taking this bug as well, since it's closely related to bug 1177152.

Comment 4

[:Paolo Amadini]

Repurposing this bug to handle the case described in bug 1177152 comment 33:

https://www.dropbox.com/s/tg3ux7iss856k7a/TP-disabled-built-in-page?dl=0

Comment 5

[:Paolo Amadini]

Attachment: MozReview Request: Bug 1177156 - Provide a button to re-enable Tracking Protection in about:privatebrowsing. r=ttaubert

Bug 1177156 - Differentiate tour and preferences link in about:privatebrowsing based on whether TP is enabled. r=ttaubert

Comment 6

[Ash Grigas]

Can you ping me when you're ready to start on this because I want to clean up the visual design of this page since we decided on this so quickly yesterday...

Comment 7

[:Paolo Amadini]

(In reply to agrigas from comment #6)
> Can you ping me when you're ready to start on this because I want to clean
> up the visual design of this page since we decided on this so quickly
> yesterday...

Yesterday's mockup is already implemented, but it will be easy to make changes if you want to share a cleaned up design!

Comment 8

[Ash Grigas]

(In reply to :Paolo Amadini from comment #7)
> (In reply to agrigas from comment #6)
> > Can you ping me when you're ready to start on this because I want to clean
> > up the visual design of this page since we decided on this so quickly
> > yesterday...
> 
> Yesterday's mockup is already implemented, but it will be easy to make
> changes if you want to share a cleaned up design!

Ok I'll post something in the morning since I need to review it today with a visual designer...

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8634694 [details]
MozReview Request: Bug 1177156 - Provide a button to re-enable Tracking Protection in about:privatebrowsing. r=ttaubert

https://reviewboard.mozilla.org/r/13433/#review12127

LGTM, but waiting for Ash to get back with the final design.

Comment 10

[Ash Grigas]

(In reply to Tim Taubert [:ttaubert] from comment #9)
> Comment on attachment 8634694 [details]
> MozReview Request: Bug 1177156 - Differentiate tour and preferences link in
> about:privatebrowsing based on whether TP is enabled. r=ttaubert
> 
> https://reviewboard.mozilla.org/r/13433/#review12127
> 
> LGTM, but waiting for Ash to get back with the final design.

Here's the final design. It may need copy review but Matej is still out so will have to do for now.
https://www.dropbox.com/s/tg3ux7iss856k7a/TP-disabled-built-in-page?dl=0

Comment 11

[:Paolo Amadini]

(In reply to agrigas from comment #10)
> Here's the final design. It may need copy review but Matej is still out so
> will have to do for now.
> https://www.dropbox.com/s/tg3ux7iss856k7a/TP-disabled-built-in-page?dl=0

Hm, so the "Turn on Tracking Protection" button would flip the preference rather than going to the Privacy Preferences, and the the user would see the version of the page with the "See what's new" button again?

Comment 12

[Ash Grigas]

(In reply to :Paolo Amadini from comment #11)
> (In reply to agrigas from comment #10)
> > Here's the final design. It may need copy review but Matej is still out so
> > will have to do for now.
> > https://www.dropbox.com/s/tg3ux7iss856k7a/TP-disabled-built-in-page?dl=0
> 
> Hm, so the "Turn on Tracking Protection" button would flip the preference
> rather than going to the Privacy Preferences, and the the user would see the
> version of the page with the "See what's new" button again?

Yes. We want to encourage people to use this feature - its something Firefox is recommended is the best private browsing experience so having it easy to turn back on, in case it got turned off accidentally or by someone else using your browser, we think is important.

Comment 13

[:Paolo Amadini]

(In reply to agrigas from comment #12)
> Yes. We want to encourage people to use this feature - its something Firefox
> is recommended is the best private browsing experience so having it easy to
> turn back on, in case it got turned off accidentally or by someone else
> using your browser, we think is important.

Okay, thanks for explaining the rationale!

Comment 14

[:Paolo Amadini]

Comment on attachment 8634694 [details]
MozReview Request: Bug 1177156 - Provide a button to re-enable Tracking Protection in about:privatebrowsing. r=ttaubert

Bug 1177156 - Provide a button to re-enable Tracking Protection in about:privatebrowsing. r=ttaubert

Comment 15

[:Paolo Amadini]

Updated to provide the button and added a test for it, should be ready for review.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=08c37be952f5

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8634694 [details]
MozReview Request: Bug 1177156 - Provide a button to re-enable Tracking Protection in about:privatebrowsing. r=ttaubert

https://reviewboard.mozilla.org/r/13433/#review12217

r=me with the two tiny issues fixed.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js:125
(Diff revision 2)
> +        prefBranch.removeObserver("enabled", prefObserver, false);

removeObserver() doesn't take a third argument.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js:129
(Diff revision 2)
> +    prefBranch.addObserver("enabled", prefObserver, false);

So it's not actually added as a weak ref, good. Then we'll just have to remove the extra QI entry above.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js:121
(Diff revision 2)
> +    let prefObserver = {
> +      QueryInterface : XPCOMUtils.generateQI([Ci.nsIObserver,
> +                                              Ci.nsISupportsWeakReference]),
> +      observe: function () {

This must not be a weak ref or we'll create another intermittent failure. Nothing holds onto |prefObserver|.

Comment 17

[:Paolo Amadini]

https://hg.mozilla.org/integration/fx-team/rev/34427253904c

Comment 18

[Ash Grigas]

https://www.dropbox.com/s/tg3ux7iss856k7a/TP-disabled-built-in-page?dl=0

New version based on Matej's copy feedback

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34427253904c

Comment 21

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified in Nightly 42.0a1, 2015-07-27.