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

[Fix] Update dnt codes #1755

Merged
1 commit merged into from Jul 28, 2018
Merged

[Fix] Update dnt codes #1755

1 commit merged into from Jul 28, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jul 28, 2018

  1. Remove useless website link.
  2. Simplify return value.

1) Remove useless website link.
2) Simplify return value.
@ghost ghost requested a review from phillipj July 28, 2018 05:54
@lpinca
Copy link
Member

lpinca commented Jul 28, 2018

Website link is not useless, but it no longer works.

@ghost ghost merged commit 34ad4dd into nodejs:master Jul 28, 2018
@ghost ghost deleted the FixDNS branch July 28, 2018 08:19
@Trott
Copy link
Member

Trott commented Aug 30, 2018

I wonder if the file can be removed entirely and any calls to _dntEnabled() replaced with navigator.doNotTrack || window.doNotTrack. (The window check is for IE 11. If you want to be REALLY accommodating and account for IE 9 and IE 10, throw in a || navigator.msDoNotTrack.)

This file seems to exist mostly for operating systems and browsers we probably don't need to take particular care in accommodating. (Windows NT 6.1??!! Firefox 31??!!) To get an idea of just how broadly the Do Not Track API is supported on current browsers: https://caniuse.com/#search=dnt and click "Show All":

screen shot 2018-08-29 at 11 13 35 pm

Note that even for browsers that show red in that chart, it's not a problem. It just means the browsers don't support Do Not Track, which just means it will never be enabled on those browsers and that's OK. We'll just get the falsy undefined for those browsers. If the user can't set Do Not Track in their browser, then we aren't failing to respect Do Not Track.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants