Closed Bug 1196017 Opened 5 years ago Closed 5 years ago

about:privatebrowsing should check if Tracking Protection is turned on globally

Categories

(Firefox :: Private Browsing, defect)

42 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 43
Tracking Status
firefox42 --- verified
firefox43 --- verified

People

(Reporter: tanvi, Assigned: francois)

Details

Attachments

(1 file)

1) Go to about:config and set privacy.trackingprotection.enabled to true.  Optionally also set privacy.trackingprotection.ui.enabled to true.
2) Turn off tracking protection in private browsing mode by 
a) setting privacy.trackingprotection.pbmode.enabled to false
b) unchecking the "Use Tracking Protection Mode in Private Windows" in about:preferences under Privacy

3) Open a private window.  Observe that the about:privatebrowsing page says Tracking Protection is OFF.
4) Go to a page that has trackers in private browsing mode - example: https://blog.mozilla.org/futurereleases/2015/08/14/new-experimental-private-browsing-and-add-ons-features-ready-for-pre-beta-testing-in-firefox/
5) Observe the shield - tracking protection is in fact enabled (globally) and is blocking trackers.

Hence the UI in step 3 should indicate that Tracking Protection is ON not OFF.

This is probably an easy fix.  Add a check for the privacy.trackingprotection.enabled pref in the code that decides whether tracking protection is on or off.
Whiteboard: [fxprivacy] [triage]
Bug 1196017 - Deal with TP turned on globally in about:privatebrowsing. r?paolo
Attachment #8649622 - Flags: review?(paolo.mozmail)
Paolo, I've changed the check for TP to make it aware of the global pref (which takes precedence over the pbmode one).

My assumption is that if a user wants to turn TP off, we should do it regardless of how they turned it on in the first place.

If they want to turn it on again, then we can flip the one pref that is exposed in the UI (pbmode.enabled). The global one is for advanced users who know about it.
Assignee: nobody → francois
Status: NEW → ASSIGNED
Whiteboard: [fxprivacy] [triage]
This solution doesn't look bad but, for users who enabled the global Tracking Protection preference manually, it might allow them to disable it unknowingly by clicking the link in the new about:privatebrowsing interface twice.

We should show Tracking Protection as enabled if the global preference is enabled, regardless of the state of the Private Browsing only preference, but in that case it might be better to just hide the link to disable Tracking Protection in Private Browsing - if you went to about:config to enable it globally you can still go to about:config to disable.
Attachment #8649622 - Flags: review?(paolo.mozmail)
Note that you can just set the "hidden" attribute on the link element at load time - no need to the track changes to the global preference in the observer. That would allow the logic for this edge case to be very simple, which is a win.
Comment on attachment 8649622 [details]
MozReview Request: Bug 1196017 - Deal with TP turned on globally in about:privatebrowsing. r?paolo

Bug 1196017 - Deal with TP turned on globally in about:privatebrowsing. r?paolo
Attachment #8649622 - Flags: review?(paolo.mozmail)
(In reply to :Paolo Amadini from comment #3)
> This solution doesn't look bad but, for users who enabled the global
> Tracking Protection preference manually, it might allow them to disable it
> unknowingly by clicking the link in the new about:privatebrowsing interface
> twice.
> 
> We should show Tracking Protection as enabled if the global preference is
> enabled, regardless of the state of the Private Browsing only preference,
> but in that case it might be better to just hide the link to disable
> Tracking Protection in Private Browsing - if you went to about:config to
> enable it globally you can still go to about:config to disable.

You're right that's a much better solution. I've updated the patch.
Comment on attachment 8649622 [details]
MozReview Request: Bug 1196017 - Deal with TP turned on globally in about:privatebrowsing. r?paolo

https://reviewboard.mozilla.org/r/16449/#review15093

Ship It!

::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.js:23
(Diff revision 2)
> +      document.body.setAttribute("globalTpEnabled", "true");
> +    } else {
> +      document.body.removeAttribute("globalTpEnabled");
> +    } 

Oh no, whitespace at the end of the line!
Attachment #8649622 - Flags: review?(paolo.mozmail) → review+
(In reply to :Paolo Amadini from comment #7)
> ::: browser/components/privatebrowsing/content/aboutPrivateBrowsing.js:23
> (Diff revision 2)
> > +      document.body.setAttribute("globalTpEnabled", "true");
> > +    } else {
> > +      document.body.removeAttribute("globalTpEnabled");
> > +    } 
> 
> Oh no, whitespace at the end of the line!

Fixed in my push to inbound :)
Comment on attachment 8649622 [details]
MozReview Request: Bug 1196017 - Deal with TP turned on globally in about:privatebrowsing. r?paolo

Approval Request Comment
[Feature/regressing bug #]: 1177156
[User impact if declined]: users that have tracking protection enabled globally will see the wrong status in about:privatebrowsing
[Describe test coverage new/current, TreeHerder]: manual testing of the 4 pref combinations
[Risks and why]: it could break the indicator and the toggle button state in about:privatebrowsing (no impact to the pref in about:preferences)
[String/UUID change made/needed]: none
Attachment #8649622 - Flags: approval-mozilla-aurora?
QA Contact: mwobensmith
By "manual testing of the 4 pref combinations", I mean:

- privacy.trackingprotection.enabled = true; privacy.trackingprotection.pbmode.enabled = true
- privacy.trackingprotection.enabled = true; privacy.trackingprotection.pbmode.enabled = false
- privacy.trackingprotection.enabled = false; privacy.trackingprotection.pbmode.enabled = true
- privacy.trackingprotection.enabled = false; privacy.trackingprotection.pbmode.enabled = false
https://hg.mozilla.org/mozilla-central/rev/2f84e6e88b82
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Comment on attachment 8649622 [details]
MozReview Request: Bug 1196017 - Deal with TP turned on globally in about:privatebrowsing. r?paolo

Seems ok in Nightly and we want private browsing to be fabulous for 42. Approved for uplift to aurora.
Attachment #8649622 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed in Fx42/Fx43, 2015-09-01.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.