Closed Bug 1060852 Opened 6 years ago Closed 6 years ago

expose privacy.trackingprotection.enabled in privacy preferences and account for removal of do-not-track options

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(seamonkey2.32 unaffected, seamonkey2.33 fixed)

RESOLVED FIXED
seamonkey2.33
Tracking Status
seamonkey2.32 --- unaffected
seamonkey2.33 --- fixed

People

(Reporter: philip.chee, Assigned: rsx11m.pub)

References

Details

Attachments

(3 files, 3 obsolete files)

See Firefox Bug 1031033 - expose privacy.trackingprotection.enabled in privacy preferences
Assignee: nobody → rsx11m.pub
Summary: expose privacy.trackingprotection.enabled in privacy preferences → expose privacy.trackingprotection.enabled in privacy preferences and account for removal of do-not-track options
Per discussion in today's status meeting, this should also cover bug 1042135 removing support for the privacy.donottrackheader.value preference (thus the radiogroup needs to be removed as well, on the other hand freeing up some space for the new preference).

Screenshot with proposed documentation is shown in bug 1029193 attachment 8459849 [details], additional work may need to be done to expose the function itself in an icon or doorhanger/notification bar if it hasn't been covered yet (but this should be a separate bug).
Neither of the blocking Firefox/Core bugs has yet checked in, but per discussion in today's status meeting, we need to be ready in case they land just before the merge in 14 days.

While waiting for bug 1031033 isn't necessarily blocking us (the backend code for this feature has checked in already, so that bug covers only the Firefox front-end changes), bug 1042135 for the DNT prefs would remove the 3-way option entirely along with the front-end changes.

Consequently, this bug here may have to proceed in three steps:
  (1a) String additions for these two options
  (1b) Code changes to the Privacy & Security prefpane
  (2) Updated to the Help content

where (1a) only should happen if bug 1042135 hasn't checked in before the merge, otherwise (1a+b) can land as a single step. Help updates (2) are time-uncritical and can check in later.
Status: NEW → ASSIGNED
Attached image Screenshot (Linux)
This is what I have in mind. The first part is straight-forward and just removes the DNT radiogroup while reinstating the checkbox label prior to bug 835134 (with some minor fixes). The second checkbox is the equivalent to Firefox bug 1031033, with their screenshot proposed in attachment 8478735 [details].

Reading through the patches associated with this feature, it is tied to Safe Browsing and apparently receives a list of tracking domains from a server, fortunately located at mozilla.org itself (thus no API key issues, I hope). However, its scope is closer to User Tracking than Safe Browsing, thus I'll keep it in the upper groupbox.

Firefox labelled this option "Prevent sites from tracking me" which appears a bit bold as it won't block *all* tracking attempts. I've rephrased it to a more careful (and more specific) "Prevent tracking activities by known sites" given that it's basically just a list of domains.
This is the full patch implementing attachment 8497685 [details]. Given that toggling the radiogroup was the only function of pref-security.js, it could be removed.
Attachment #8497694 - Flags: ui-review?(neil)
Attachment #8497694 - Flags: review?(iann_bugzilla)
Attached patch Part 1a: String additions only (obsolete) — Splinter Review
... and in case this is getting too close to the merge day without any checkins from bug 1042135, here the patch which contains string additions only. In that case, we can check this one in and I'll provide an unbitrotted patch to get to attachment 8497694 [details] [diff] [review] in a second step (for comm-central and -aurora).

Part 2 for the Help updates will follow once we've settled on the strings.
Attachment #8497697 - Flags: ui-review?(neil)
Attachment #8497697 - Flags: review?(iann_bugzilla)
Comment on attachment 8497694 [details] [diff] [review]
Part 1: Code changes (with strings)

>+<!ENTITY trackProtect.label               "Prevent tracking activities by known sites">
>+<!ENTITY trackProtect.accesskey           "n">

Should I add another checkbox for the option to notify if content has (or might have been?) blocked by tracking protection when I port bug 1043801? (It's slightly different to the SSL warning though; the SSL code always notifies about mixed content (but we have a pref to ignore that) and blocks it globally based on a preference, while the tracking protection code has a global pref to disable protection and notification, and uses the permission manager to disable protection on a per-host basis (confusingly this generates notifications when content might or might not have been blocked). It does mean however that I'll need a Startup() function to ensure that my checkbox is disabled appropriately...)
Sure, we can leave pref-security.js around with an empty Startup() function to avoid having to recreate that file later again in another bug. However, if there isn't another pref, what would that checkbox toggle? I'm not sure if I understand correctly.

So, privacy.trackingprotection.enabled globally disables both notifications and the actual protection; if it's set to "true" how are notifications then disabled while keeping the protection feature itself active? Or, is the a new SM-specific preference that you'd create? This would make sense.
(In reply to rsx11m from comment #7)
> new SM-specific preference that you'd create? This would make sense.

I think the security.warn_* prefs are also all SM_specific these days.
Fine with me, so pref-security.js will stay with the next iteration of the patch.
Any other comments/requests from your side?
Comment on attachment 8497694 [details] [diff] [review]
Part 1: Code changes (with strings)

In theory there's some migration code... or we could just relnote the change, after all, how many people will have requested to be tracked?
Attachment #8497694 - Flags: ui-review?(neil) → ui-review+
Attachment #8497697 - Flags: ui-review?(neil) → ui-review+
Firefox has some migration code included in bug 1042135 attachment 8460596 [details] [diff] [review]:
https://bugzilla.mozilla.org/attachment.cgi?id=8460596&action=diff#a/browser/components/nsBrowserGlue.js_sec3

which would fit in nsSuiteGlue.js::_updatePrefs() where the download-manager prefs are migrated:
http://hg.mozilla.org/comm-central/file/fedb0ee60207/suite/common/src/nsSuiteGlue.js#l919

I don't see any other obvious place where such a pref-specific migration is performed.

On the other hand, without that, the only consequence would be that a pick of "consent to tracking" is changed into a "don't track me" request rather than "don't tell anything" as the migration code does.
This patch retains the pref-security.js file for immediate future use and also includes some migration code from bug 1042135. For the latter, the situation here is a little different than the 3-state radiogroup Firefox employs now. Specifically the case "Tell websites" being unchecked while "consent to tracking" being selected shouldn't occur there. I've thus simplified the if() statement to be triggered already on .value != 1 alone.

Otherwise, we may run into the interesting situation that .enabled is unchecked but .value is '0' for "please track me" but not reset during migration. If the user checks "don't track me" later, the .enabled/.value semantics would actually be "please track me" (while "don't track me" is sent) until SeaMonkey is restarted, at which time the migration code triggers resetting both preferences. Thus, I think it's better to clean up a user-set .value upfront regardless of whether or not DNT itself is enabled at the time of startup.

On a side note, Firefox has a browser.migration.version preference setting to keep track of migrations and only execute those codes once. That's for another bug, but given that SeaMonkey doesn't perform such bookkeeping, it may be worth introducing some similar mechanism as well if more pref migrations are added.
Attachment #8497694 - Attachment is obsolete: true
Attachment #8497694 - Flags: review?(iann_bugzilla)
Attachment #8499098 - Flags: ui-review+
Attachment #8499098 - Flags: review?(iann_bugzilla)
How can I tell if the pref works? I tried toggling it and restarting but I don't get anything blocked.
Good question. The privacy.trackingprotection.enabled preference is provided by Toolkit's Safe Browsing code (hooked up in bug 1024610) and I've verified that the list is downloaded and cached (though I can't read its content, no clear text). Thus, I assumed that the backend is available to SeaMonkey as well (yet we will have to add notifications). I'm unable to locate any documentation or a test page.

Monica, what is the best way to test if the Tracking Protection is actually working as designed?
Are we missing anything beyond making the pref available in the UI? (FF bug 1031033)
Flags: needinfo?(mmc)
The unofficial test page is at http://people.mozilla.org/~mchew/test_tp.html. This UI for this feature requires the shield changes to over load the mixed content blocker doorhanger.

https://bugzilla.mozilla.org/show_bug.cgi?id=1043803
https://bugzilla.mozilla.org/show_bug.cgi?id=1043797
https://bugzilla.mozilla.org/show_bug.cgi?id=1043801

Here is how the initial implementation of the UI works: https://bug1029193.bugzilla.mozilla.org/attachment.cgi?id=8459849
Flags: needinfo?(mmc)
Thanks Monica, the test page works fine for me: without the box checked (default), the ad content appears in the page; with the box checked, those remain blank as desired.

As for the mixed content blocker doorhanger, it is my understanding that Neil is looking into that part as he has ported the original notification bars and doorhangers already.
Blocks: 1078740
Attachment #8499098 - Flags: review?(iann_bugzilla) → review+
Attachment #8497697 - Flags: review?(iann_bugzilla) → review+
Thanks, Ian. So, let's check in attachment 8497697 [details] [diff] [review] for the string changes first given that the Core changes in bug 1042135 didn't land yet. I'll unbitrot attachment 8499098 [details] [diff] [review] afterwards so that we are ready to land if the 3-state DNT is removed prior to next Monday's merge day.
Keywords: checkin-needed
Whiteboard: [c-n: attachment 8497697 (=1a) for comm-central][leave open]
Attachment #8499098 - Attachment description: Part 1: Code changes (v2) → Part 1: Code changes (v2) [pending bug 1042135]
Whiteboard: [c-n: attachment 8497697 (=1a) for comm-central][leave open] → [leave open]
Attachment #8497697 - Attachment is obsolete: true
Comment on attachment 8499098 [details] [diff] [review]
Part 1: Code changes (v2)

Bug 1042135 has landed the Core changes with mozilla-central changeset 9a16137bc7b4, consequently this complete patch now has to be checked in.
Attachment #8499098 - Attachment description: Part 1: Code changes (v2) [pending bug 1042135] → Part 1: Code changes (v2)
sigh. Bug 1042135 (Firefox::Preferences) : Revert do not track preferences to be a simple on/off switch
Blocks: 1087910
Attached patch Part 2: Help updates (obsolete) — Splinter Review
First take on the Help updates, the problem being that I'm not /exactly/ sure how it works. This doesn't include notification bars and the new pref proposed in bug 1078740 for the tracking protection, but should be complete with respect to the description of the feature as such.

Neil, can you check the content for technical correctness?
Attachment #8510560 - Flags: feedback?(neil)
Whiteboard: [leave open] → [c-n: Part 1, #8499098][leave open]
Comment on attachment 8510560 [details] [diff] [review]
Part 2: Help updates

>-    sending <q>Do Not Track</q> requests to websites, but they are not
>+    <q>Do Not Track</q> requests to websites, but those are <em>not</em>
[The "they/those" change looks wrong to me.]
Attachment #8510560 - Flags: feedback?(neil) → feedback+
Yeah, it appeared to me that it's a bit ambiguous what "they" refers to, but if "those" makes it worse let's stick to the current wording (I've kept adding the <em> though to emphasize the "not").
Attachment #8510560 - Attachment is obsolete: true
Attachment #8510754 - Flags: review?(iann_bugzilla)
No longer blocks: 1087910
Duplicate of this bug: 1087910
Depends on: 1042135
FYI: Bug 1031033 for the privacy.trackingprotection.enabled checkbox in the Firefox UI has now landed on as well, mozilla-central changeset 42a1ca26cbf9 (i.e., all dependencies here are satisfied).
I'm not sure if you were planning to do this or not, but if you could expose this preferences UI regardless of where browser.polaris.enabled is true, I think that would be a better user experience.
Both browser.polaris.enabled and privacy.trackingprotection.ui.enabled are specific to Firefox, as far as I can tell from reading bug 1031033 and bug 1081343, thus shouldn't have any impact on SeaMonkey. Consequently, the tracking-protection checkbox is always available in the SeaMonkey preferences UI, and should be functional unless Polaris has any impact on the tracking-protection backend function.
Ok, great. There is no effect of either of those preferences on the backend.
Thanks for confirming.
Attachment #8510754 - Flags: review?(iann_bugzilla) → review+
Thanks Ian, thus both patches can land now and this bug closed after that. At least Part 1 needs to be pushed before the upcoming merge around November 24th to avoid useless-UI or late-l10n issues.
Whiteboard: [c-n: Part 1, #8499098][leave open] → [c-n: push #8499098 and #8510754 to comm-central]
Pushed part 1 http://hg.mozilla.org/comm-central/rev/f4287827157c
Pushed part 2 http://hg.mozilla.org/comm-central/rev/27e18a0a0685
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: push #8499098 and #8510754 to comm-central]
Target Milestone: --- → seamonkey2.33
You need to log in before you can comment on or make changes to this bug.