Add option to disable sending the Do Not Track (DNT) signal #2089

miketung · 2018-07-09T06:11:32Z

This creates an option to disable sending the DNT header per issue #2038

The default setting is set to enabled.

ghostwords

Hello and thank you for the PR!

In addition to the feedback below, we should also add a check to avoid setting navigator.doNotTrack when DNT signal sending is disabled.

ghostwords · 2018-07-09T15:13:22Z

src/_locales/en_US/messages.json

@@ -23,6 +23,10 @@
        "message": "Currently Privacy Badger only checks if third parties are using cookies, HTML5 local storage, or canvas fingerprinting to track your browsing. Some of these domains may be using tracking methods that Privacy Badger can't detect.",
        "description": ""
    },
+    "options_enable_dnt_checkbox": {
+        "message": "Enable sending DNT header",
+        "description": ""


Could you add a description to provide translators with context? You could describe where in Badger's UI this label is located.

ghostwords · 2018-07-09T15:24:46Z

src/_locales/en_US/messages.json

@@ -23,6 +23,10 @@
        "message": "Currently Privacy Badger only checks if third parties are using cookies, HTML5 local storage, or canvas fingerprinting to track your browsing. Some of these domains may be using tracking methods that Privacy Badger can't detect.",
        "description": ""
    },
+    "options_enable_dnt_checkbox": {
+        "message": "Enable sending DNT header",


Let's rename "DNT header" to "Do Not Track signal", and link it to https://www.eff.org/issues/do-not-track.

"Do Not Track" instead of "DNT" because we shouldn't assume familiarity with the abbreviation.

"Signal" instead of "header" because we also assert DNT in JavaScript (#1861).

ghostwords · 2018-07-09T15:27:09Z

src/js/background.js

@@ -459,6 +459,7 @@ Badger.prototype = {
    showCounter: true,
    showTrackingDomains: false,
    socialWidgetReplacementEnabled: true,
+    DNTEnabled: true


Should use a more descriptive name like sendDNTSignal.

Please insert the new setting in alphabetic order.

ghostwords · 2018-07-09T15:28:44Z

src/js/background.js

+  /**
+   * Check if social widget replacement functionality is enabled
+   */
+  isDNTEnabled: function() {


DNTSignalEnabled?

ghostwords · 2018-07-09T15:32:18Z

src/js/options.js

+    }
+  });
+
+  $("#check_dnt_policy_checkbox").prop("checked", enabled).prop("disabled", enabled);


When the options page first loads, "Check if sites comply with EFF's Do Not Track policy" is not grayed out, but toggling the send DNT signal checkbox grays it out. This is inconsistent.

If we are going to force-enable DNT checking when DNT sending is enabled, we should make it visually clear that DNT checking is a sub-option of DNT sending.

Thanks, I've fixed this inconsistency. The grey-out state of the DNT checking is persisted now on initial load. And disabling sending the DNT header automatically disables the DNT checking.

Renamed the setting to sendDNTSignal to follow convention. Fixed the consistency with grey out state of "Check if sites comply with EFF's Do Not Track policy" when setting is disabled. Added some description for i18n.

… is disabled

miketung · 2018-07-09T18:15:13Z

Thanks for the feedback @ghostwords. I believe I've made all the suggested changes now and also avoided setting the navigator.doNotTrack in the contentscripts/dnt.js when this new DNT sending option is disabled.

ghostwords

Thanks! Got some more feedback.

ghostwords · 2018-07-09T19:08:54Z

src/_locales/en_US/messages.json

@@ -23,6 +23,10 @@
        "message": "Currently Privacy Badger only checks if third parties are using cookies, HTML5 local storage, or canvas fingerprinting to track your browsing. Some of these domains may be using tracking methods that Privacy Badger can't detect.",
        "description": ""
    },
+    "options_enable_dnt_checkbox": {
+        "message": "Enable sending the <a href='https://www.eff.org/issues/do-not-track' target='_blank'>Do Not Track</a> signal",


Sorry, now that I look at this again, should this be more to the point and say, "Send the Do Not Track signal"?

ghostwords · 2018-07-09T19:19:55Z

src/js/webrequest.js

@@ -776,6 +782,14 @@ function dispatcher(request, sender, sendResponse) {
    badger.storage.getBadgerStorageObject("action_map").deleteItem(request.origin);

    sendResponse();
+  } else if (request.type == "checkDNT") {


This either has to be } else if (request.checkDNT) {, or you have to update the message you send from the content script.

Style nit: Could you add a blank line above the "else if"?

Good catch. Fixed.

ghostwords · 2018-07-09T19:21:29Z

src/skin/options.html

+          <input type="checkbox" id="enable_dnt_checkbox">
+          <span class="i18n_options_enable_dnt_checkbox"></span>
+        </label>
+      </div>
      <div class="checkbox">


Could you indicate that checking DNT policies is a sub-setting of sending the DNT signal? Perhaps by indenting the DNT policies checkbox a bit.

Yes, indented this option in, flush with the footer text.

… option

ghostwords · 2018-07-09T22:25:00Z

Thanks! I'll re-review tomorrow.

ghostwords

Thank you for your contribution! Merging.

Add option to disable sending the Do Not Track (DNT) signal.

ghostwords · 2018-07-11T16:47:36Z

Followup commits: 2577cc2~3...2577cc2

Mike Tung added 2 commits July 8, 2018 22:51

Adding option to disable sending the DNT signal. Fixes EFForg#2038

779ad89

Removing trailing space (thanks lint!)

80c173b

ghostwords added the translations label Jul 9, 2018

ghostwords requested changes Jul 9, 2018

View reviewed changes

Mike Tung added 2 commits July 9, 2018 10:51

Addressed feedback in pull request 2089.

7568b0d

Renamed the setting to sendDNTSignal to follow convention. Fixed the consistency with grey out state of "Check if sites comply with EFF's Do Not Track policy" when setting is disabled. Added some description for i18n.

Avoid setting navigator.doNotTrack in content script when DNT sending…

d6dcca8

… is disabled

ghostwords requested changes Jul 9, 2018

View reviewed changes

Mike Tung added 2 commits July 9, 2018 12:43

Make options message clearer for send DNT and indent the DNT checking…

440a12e

… option

25px left padding

02cb283

ghostwords approved these changes Jul 11, 2018

View reviewed changes

ghostwords merged commit 02cb283 into EFForg:master Jul 11, 2018

ghostwords added a commit that referenced this pull request Jul 11, 2018

Merge pull request #2089.

73a0756

Add option to disable sending the Do Not Track (DNT) signal.

ghostwords mentioned this pull request Jul 11, 2018

Update DNT policy checking checkbox label text #2094

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to disable sending the Do Not Track (DNT) signal #2089

Add option to disable sending the Do Not Track (DNT) signal #2089

miketung commented Jul 9, 2018

ghostwords left a comment

ghostwords Jul 9, 2018

miketung Jul 9, 2018

ghostwords Jul 9, 2018

miketung Jul 9, 2018

ghostwords Jul 9, 2018

ghostwords Jul 9, 2018

miketung Jul 9, 2018

ghostwords Jul 9, 2018

miketung Jul 9, 2018

ghostwords Jul 9, 2018 •

edited

Loading

miketung Jul 9, 2018

miketung commented Jul 9, 2018

ghostwords left a comment

ghostwords Jul 9, 2018

miketung Jul 9, 2018

ghostwords Jul 9, 2018

ghostwords Jul 9, 2018

miketung Jul 9, 2018

ghostwords Jul 9, 2018

miketung Jul 9, 2018

ghostwords commented Jul 9, 2018

ghostwords left a comment

ghostwords commented Jul 11, 2018 •

edited

Loading

Add option to disable sending the Do Not Track (DNT) signal #2089

Add option to disable sending the Do Not Track (DNT) signal #2089

Conversation

miketung commented Jul 9, 2018

ghostwords left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghostwords Jul 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miketung commented Jul 9, 2018

ghostwords left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghostwords commented Jul 9, 2018

ghostwords left a comment

Choose a reason for hiding this comment

ghostwords commented Jul 11, 2018 • edited Loading

ghostwords Jul 9, 2018 •

edited

Loading

ghostwords commented Jul 11, 2018 •

edited

Loading