Do not track settings results in wrong value for navigator.doNotTrack

RESOLVED FIXED in mozilla32

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: Georg Brzyk, Assigned: mounir)

Tracking

({dev-doc-complete})

Trunk
mozilla32
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 768214 [details]
donottrack.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.116 Safari/537.36

Steps to reproduce:

1. Set Tracking option to "Tell sites that I want to be tracked"
2. Restart browser
3. Retrieve value of navigator.doNotTrack



Actual results:

Value returned was "yes", same as "Tell sites that I do not want to be tracked" setting.


Expected results:

Value should have been "no"

Comment 1

4 years ago
I get the same result on firefox 21 "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:21.0) Gecko/20100101 Firefox/21.0"

Updated

4 years ago
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Looks like navigator.doNotTrack just shows whether the header is being sent at all, not what value is being sent.  

Did we change the UI pref to a tristate at some point when it used to be a boolean?  The devmo docs at https://developer.mozilla.org/en-US/docs/Web/API/navigator.doNotTrack claim there are only two possible states here, and the navigator code sure seems to do that.
Flags: needinfo?(mounir)
Flags: needinfo?(gavin.sharp)
Summary: Do not track settings results in wrong value → Do not track settings results in wrong value for navigator.doNotTrack
(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #2)
> Did we change the UI pref to a tristate at some point when it used to be a
> boolean?

Yes, in bug 765398.
Flags: needinfo?(gavin.sharp)
Blocks: 765398
(Assignee)

Comment 4

4 years ago
This should be pretty simple to fix but IIRC, we have a long running issue with the .doNotTrack return value when we do not send a header. Some browsers returns |null| and some return |"undefined"|. Firefox does the later. The rational behind that was to prevent consumers to check if .doNotTrack is true which would be the case for "1" and "0" but not |null| while "undefined" would match too.

Justin, I think you were involved with that, is that correct? Do you have some updates?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mounir) → needinfo?(justin.lebar+bug)
OS: Windows 7 → All
Hardware: x86_64 → All
(Assignee)

Comment 5

4 years ago
Created attachment 768964 [details] [diff] [review]
Patch

A simple patch that should fix the bug but might not pass our tests. I am waiting for Justin to know if I should fix the "undefined" behaviour as long as I am here.
FWIW, this is where the spec is headed WRT this (if it is ever finished):
http://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#js-dom
I don't have an opinion about null vs. undefined; they're both falsey values anyway.  My beef was that we shouldn't use 0 and 1, since 0 is falsey.  So long as we use "no" and "yes", I'm happy.

(We may have to switch to 0 and 1 because that's what other people are doing.  I've explained our position on the DNT mailing list and been met with overt hostility; that's not something I want to do again.  :)
Flags: needinfo?(justin.lebar+bug)
Ooh, no, it's not "undefined" -- it's "unspecified".  I remember this now.  Sorry, I take the first paragraph of comment 7 back.

The idea was that the return values should all have the same falsyness, so |if (navigator.doNotTrack)| would always return true, forcing people to explicitly test for the tri-state.

The second paragraph still holds; the spec people don't seem to care at all about this.  All they cared about when I went on the mailing list was making the DOM match the HTTP header.  Which is a bummer.
(Assignee)

Comment 9

4 years ago
(In reply to Justin Lebar [:jlebar] from comment #8)
> Ooh, no, it's not "undefined" -- it's "unspecified".  I remember this now. 
> Sorry, I take the first paragraph of comment 7 back.
> 
> The idea was that the return values should all have the same falsyness, so
> |if (navigator.doNotTrack)| would always return true, forcing people to
> explicitly test for the tri-state.
> 
> The second paragraph still holds; the spec people don't seem to care at all
> about this.  All they cared about when I went on the mailing list was making
> the DOM match the HTTP header.  Which is a bummer.

So, Justin, you would be happy with the following enums:
{ "yes", "no", "unspecified" }
and
{ "1", "0", "unspecified" }
?

Assuming that "1" and "0" convert to |true| as bool.

I would gladly try to send an email to the mailing-list even though it might be a waste of time.
(Assignee)

Updated

4 years ago
Flags: needinfo?(justin.lebar+bug)
The mailing list is bogged down in policy discussions now with a hard deadline and are not likely to take up something like this, so yeah, it's probably a waste of time.

I recommend going with the second set {"1", "0", "unspecified"} since it is closest to what the spec says.
> So, Justin, you would be happy with the following enums:
>   { "yes", "no", "unspecified" }
> and
> { "1", "0", "unspecified" }
> ?
>
> Assuming that "1" and "0" convert to |true| as bool.

Yes, and afaict that assumption is correct.
Flags: needinfo?(justin.lebar+bug)
(Assignee)

Comment 12

4 years ago
Created attachment 794664 [details] [diff] [review]
Patch
Assignee: nobody → mounir
Attachment #768964 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #794664 - Flags: review?(justin.lebar+bug)
Attachment #794664 - Flags: review?(justin.lebar+bug) → review+
(Assignee)

Comment 13

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a35a5681fe2f
Flags: in-testsuite+
Target Milestone: --- → mozilla26
(Assignee)

Updated

4 years ago
Keywords: dev-doc-needed
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/862513e70ed3 - b2g mochitest-8 said https://tbpl.mozilla.org/php/getParsedLog.php?id=26953387&tree=Mozilla-Inbound
Mounir, do you still think we should take this patch?
Flags: needinfo?(mounir)
(Assignee)

Comment 16

4 years ago
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #15)
> Mounir, do you still think we should take this patch?

Yes. We should just fix the B2G test.
Flags: needinfo?(mounir)
(Assignee)

Comment 17

3 years ago
Created attachment 8429565 [details] [diff] [review]
Patch with tests
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Target Milestone: mozilla26 → mozilla32
Version: 22 Branch → Trunk
(Assignee)

Comment 18

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9062e64d9c0
Keywords: dev-doc-needed
(In reply to Mounir Lamouri (:mounir) from comment #18)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c9062e64d9c0

so we can delete the checkin needed flag ?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c9062e64d9c0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 21

3 years ago
I meant to remove checkin-needed, to dev-doc-needed...
Keywords: dev-doc-needed
Duplicate of this bug: 1021227

Comment 23

3 years ago
(In reply to Mounir Lamouri (:mounir) from comment #18)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c9062e64d9c0

While you are fixing this, shouldn't you take the opportunity to place doNotTrack at the window level too (or only) as per the new standard https://twitter.com/adrianba/status/476586085422600192

According to http://compatibility.shwups-cms.ch/de/home?&property=doNotTrack Safari 7+ already did that.
Flags: needinfo?(mounir)
Bruno: You're right.  I filed bug 1023920 to track that work since this one is already resolved.
Depends on: 628197
Flags: needinfo?(mounir)

Updated

3 years ago
Duplicate of this bug: 1045678
I've just updated https://developer.mozilla.org/en-US/docs/Web/API/navigator.doNotTrack with some of the information on this bug. Please can folks review and make further edits if necessary.
Thanks a lot! I've also added a comment there: https://developer.mozilla.org/en-US/Firefox/Releases/32
I'm switching to dev-doc-complete, if anybody disagree, just flip the keyword back to dev-doc-needed.
Keywords: dev-doc-needed → dev-doc-complete
(In reply to Justin Lebar (not reading bugmail) from comment #8)
> 
> The idea was that the return values should all have the same falsyness, so
> |if (navigator.doNotTrack)| would always return true, forcing people to
> explicitly test for the tri-state.
> 

I know this is closed and has been resolved, I am just interested in the history here and what the correct interpretation is.

Curious about what this really means. It seems hat generally people are saying we cannot trust the return value in Fx 31 and lower so, just assume 'unspecified'. From reading this though, I think that might b an incorrect assumption.

What this actually says is that you could not just do `if (navigator.doNotTrack)`, as this would always return true because of the tristate nature of the return value so, on would need to be explicit and do 

`if (navigator.doNotTrack === 'yes') {
    // dnt is enabled
} else {
    // dnt is disabled or unspecified which should be taken as disabled/no user preference indicated.
}

Thanks in advance for any assistance.
Flags: needinfo?(standard8)
Flags: needinfo?(mounir)
Sorry, I don't quite understand the question, but I only updated the docs for this bug.  Mounir is probably best to answer your query.
Flags: needinfo?(standard8)
(In reply to Mark Banner (:standard8) from comment #29)
> Sorry, I don't quite understand the question, but I only updated the docs
> for this bug.  Mounir is probably best to answer your query.

Thanks Mark.

The tl;dr is, would doing, `if (navigator.doNotTrack === 'yes')` be an accurate indication of user intent in Firefox 31 and below?

It seems that it might be the case but, https://bugzilla.mozilla.org/show_bug.cgi?id=887703#c2 suggests that it is not the case as "yes" actually just means that the DNT header is sent not what the value of that header is.

I guess that is the confusion and why the value of the property is not trusted.
> would doing, `if (navigator.doNotTrack === 'yes')` be an accurate indication of user
> intent in Firefox 31 and below?

No.  That's precisely what this bug report is about.

In Firefox 31, the logic in the navigator.doNotTrack getter is:

   583   if (sDoNotTrackEnabled) {
   584     aResult.AssignLiteral("yes");
   585   } else {
   586     aResult.AssignLiteral("unspecified");
   587   }

sDoNotTrackEnabled is the "user has explicitly expressed intent" value.  It does not say what the intent is.

Note that the usage share of Firefox 31 and below is very small (less than 1% of Firefox market share last I checked), so you should probably just not worry about them and assume that "yes" means "yes".
(Assignee)

Updated

2 years ago
Flags: needinfo?(mounir)
You need to log in before you can comment on or make changes to this bug.