Closed Bug 1087529 Opened 5 years ago Closed 5 years ago

Revert do not track preferences to be a simple on/off switch

Categories

(Thunderbird :: Preferences, defect)

defect
Not set

Tracking

(thunderbird35 unaffected, thunderbird36 fixed)

RESOLVED FIXED
Thunderbird 36.0
Tracking Status
thunderbird35 --- unaffected
thunderbird36 --- fixed

People

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

References

Details

Attachments

(2 files, 1 obsolete file)

Bug 1042135 is about to land on m-c for the Firefox side, which reverts bug 765398 that introduced the three radiobuttons for the DNT feature. Following reason is given in the description of that bug reversing this design again:

(Quoting Philipp Sackl [:phlsa] from bug 1042135 comment #0)
> The one saying that a user explicitly wants to be tracked seems to be
> superfluous. Being tracked is (unfortunately) the standard on the web. Even
> our SUMO page says that this option is effectively the same as not telling
> the website about tracking preferences. By presenting more options to the
> users here, we make it less likely for them to change the setting at all.

> The main reason for switching away from this design (bug 765398) seems to
> have been the ad industry wanting the explicit opt-in. Two years later, I
> don't think that's an issue anymore. Especially since we're the only browser
> offering the "Do track" option.

This in turn would result in loss of support for the privacy.donottrackheader.value preference, thus the need to reverse adaption of bug 765398 introduced in Thunderbird as part of bug 953426.

I'll have a look, given that I'm working on similar changes for SeaMonkey in bug 1060852 already. This will involve modification of the Privacy prefpane, adding some migration code to account for ambiguity of the old settings after removal of the privacy.donottrackheader.value preference, and removal of the test for verifying that the 3-state logic works given that it will be reduced to a simple checkbox covered by the general prefpane logic.
Core changes have landed as mozilla-central changeset 9a16137bc7b4.
Depends on: 1042135
Attached patch Proposed patch (obsolete) — Splinter Review
This is a fairly direct port of bug 1042135 attachment 8460596 [details] [diff] [review] for the Privacy prefpane changes. Furthermore,

 - the dntTrackingNotOkay label is reused for the checkbox (like Firefox), hoping that this won't have any l10n implications;

 - as there is no equivalent to browser.migration.version, the migration code added to msgMail3PaneWindow.js implicitly uses privacy.donottrackheader.value itself to determine if it was executed already (i.e., shouldn't exist then).

 - I have removed test-donottrack-prefs.js completely; this is now a simple checkbox, thus UI/preference relationship entirely within the scope of the prefpane object. Meaning, it should be tested in the XUL tests already, and if it's broken, it should result in a catastrophic failure of all simple prefs. Consequently, no need to carry around a redundant test.
Attachment #8515233 - Flags: ui-review?(richard.marti)
Attachment #8515233 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8515233 [details] [diff] [review]
Proposed patch

>+      <separator class="thin"/>

I've added the separator to better group the checkbox and the "Learn more" link, otherwise it's not clearly separated from the Cookies group.

>+      <hbox>
>+        <checkbox id="privacyDoNotTrackCheckbox"
>+                  label="&dntTrackingNotOkay.label2;"
>+                  accesskey="&dntTrackingNotOkay.accesskey;"
>+                  preference="privacy.donottrackheader.enabled"/>
>+        <spacer flex="1"/>
>+      </hbox>

The hbox and spacer additions are retained from bug 1025488 (also flushing to the left rather than the right like its Firefox counterpart).
Comment on attachment 8515233 [details] [diff] [review]
Proposed patch

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

(In reply to rsx11m from comment #2)

Looks good. Thank you.

> Created attachment 8515233 [details] [diff] [review]
> Proposed patch
> 
>  - as there is no equivalent to browser.migration.version, the migration
> code added to msgMail3PaneWindow.js implicitly uses
> privacy.donottrackheader.value itself to determine if it was executed
> already (i.e., shouldn't exist then).

There exists mail.ui-rdf.version which is used in mailMigrator.js. Bug 511625, awaiting check-in, will update it to 6. If you like to use it, please apply your patch on top of bug 511625.
Attachment #8515233 - Flags: ui-review?(richard.marti) → ui-review+
Yes, this looks like the better place for UI migration (though strictly speaking it's a preference migration). No clue how I have missed that, given the (rather obvious) name of the file...
Review comments addressed, this goes on top of bug 511625 attachment 8512124 [details] [diff] [review].
Attachment #8515233 - Attachment is obsolete: true
Attachment #8515233 - Flags: review?(mkmelin+mozilla)
Attachment #8515513 - Flags: ui-review+
Attachment #8515513 - Flags: review?(mkmelin+mozilla)
Bug 511625 has checked in, thus attachment 8515513 [details] [diff] [review] applies now directly.
Comment on attachment 8515513 [details] [diff] [review]
Proposed patch (v2)

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

Looks great, thx! r=mkmelin
Attachment #8515513 - Flags: review?(mkmelin+mozilla) → review+
Thanks, push for comm-central please (whenever possible).
Keywords: checkin-needed
Done! https://hg.mozilla.org/comm-central/rev/4dd3954fe924
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 36.0
Hmm, this comment doesn't make much sense, does it?
Just a quick fix if you want to correct this.

Sorry for that copy-pasting error...
Attachment #8521975 - Flags: review?(mkmelin+mozilla)
Attachment #8521975 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed
Whiteboard: [c-n: push #8521975 to comm-central]
Hmm. I know that it's just a typo in a comment, but is there any particular reason that the follow-up patch has been left hanging in the checkin queue for a month now? Just wondering what's going on...
Peoples queries might not include FIXED bugs I suppose. On a side note: if you don't have bug want hg access yourself, I'd be happy to vouch for you

https://hg.mozilla.org/comm-central/rev/dbab5a531594
Keywords: checkin-needed
Whiteboard: [c-n: push #8521975 to comm-central]
Thanks!
You need to log in before you can comment on or make changes to this bug.