Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jul 11, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/_locales/en_US/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"description": ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

},
"options_dnt_policy_setting": {
"message": "Check if sites comply with EFF's Do Not Track policy",
"description": "Checkbox label on the general settings page"
Expand Down Expand Up @@ -439,4 +443,4 @@
"message": "Oops. Something went wrong.",
"description": ""
}
}
}
8 changes: 8 additions & 0 deletions src/js/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ Badger.prototype = {
showCounter: true,
showTrackingDomains: false,
socialWidgetReplacementEnabled: true,
DNTEnabled: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use a more descriptive name like sendDNTSignal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please insert the new setting in alphabetic order.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

},

/**
Expand Down Expand Up @@ -612,6 +613,13 @@ Badger.prototype = {
return this.getSettings().getItem("socialWidgetReplacementEnabled");
},

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DNTSignalEnabled?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return this.getSettings().getItem("DNTEnabled");
},

isCheckingDNTPolicyEnabled: function() {
return this.getSettings().getItem("checkForDNTPolicy");
},
Expand Down
19 changes: 19 additions & 0 deletions src/js/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ function loadOptions() {
$("#show_counter_checkbox").prop("checked", badger.showCounter());
$("#replace_social_widgets_checkbox").on("click", updateSocialWidgetReplacement);
$("#replace_social_widgets_checkbox").prop("checked", badger.isSocialWidgetReplacementEnabled());
$("#enable_dnt_checkbox").on("click", updateDNTCheckboxClicked);
$("#enable_dnt_checkbox").prop("checked", badger.isDNTEnabled());
$("#check_dnt_policy_checkbox").on("click", updateCheckingDNTPolicy);
$("#check_dnt_policy_checkbox").prop("checked", badger.isCheckingDNTPolicyEnabled());

Expand Down Expand Up @@ -316,6 +318,23 @@ function updateSocialWidgetReplacement() {
});
}

/**
* Update DNT checkbox clicked
*/
function updateDNTCheckboxClicked() {
const enabled = $("#enable_dnt_checkbox").prop("checked");

chrome.runtime.sendMessage({
type: "updateSettings",
data: {
DNTEnabled: enabled
}
});

$("#check_dnt_policy_checkbox").prop("checked", enabled).prop("disabled", enabled);
Copy link
Member

@ghostwords ghostwords Jul 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

updateCheckingDNTPolicy();
}

function updateCheckingDNTPolicy() {
const enabled = $("#check_dnt_policy_checkbox").prop("checked");

Expand Down
12 changes: 9 additions & 3 deletions src/js/webrequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@ function onBeforeSendHeaders(details) {
if (!isThirdPartyDomain(requestDomain, tabDomain)) {
if (badger.isPrivacyBadgerEnabled(tabDomain)) {
// Still sending Do Not Track even if HTTP and cookie blocking are disabled
details.requestHeaders.push({name: "DNT", value: "1"});
if (badger.isDNTEnabled()) {
details.requestHeaders.push({name: "DNT", value: "1"});
}
return {requestHeaders: details.requestHeaders};
} else {
return {};
Expand Down Expand Up @@ -233,13 +235,17 @@ function onBeforeSendHeaders(details) {
let newHeaders = details.requestHeaders.filter(function (header) {
return (header.name.toLowerCase() != "cookie" && header.name.toLowerCase() != "referer");
});
newHeaders.push({name: "DNT", value: "1"});
if (badger.isDNTEnabled()) {
newHeaders.push({name: "DNT", value: "1"});
}
return {requestHeaders: newHeaders};
}

// if we are here, we're looking at a third party
// that's not yet blocked or cookieblocked
details.requestHeaders.push({name: "DNT", value: "1"});
if (badger.isDNTEnabled()) {
details.requestHeaders.push({name: "DNT", value: "1"});
}
return {requestHeaders: details.requestHeaders};
}

Expand Down
6 changes: 6 additions & 0 deletions src/skin/options.html
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ <h1><span class="i18n_options_title"></span></h1>
<span class="i18n_options_social_widgets_checkbox"></span>
</label>
</div>
<div class="checkbox">
<label>
<input type="checkbox" id="enable_dnt_checkbox">
<span class="i18n_options_enable_dnt_checkbox"></span>
</label>
</div>
<div class="checkbox">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

<label>
<input type="checkbox" id="check_dnt_policy_checkbox">
Expand Down