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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Georg Brzyk]

Attachment: 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

[simeng]

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

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.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

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

Comment 4

[Mounir Lamouri (:mounir)]

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?

Comment 5

[Mounir Lamouri (:mounir)]

Attachment: 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.

Comment 6

[Sid Stamm [:geekboy or :sstamm]]

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

Comment 7

[Justin Lebar (not reading bugmail)]

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

Comment 8

[Justin Lebar (not reading bugmail)]

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.

Comment 9

[Mounir Lamouri (:mounir)]

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

Comment 10

[Sid Stamm [:geekboy or :sstamm]]

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.

Comment 11

[Justin Lebar (not reading bugmail)]

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

Comment 12

[Mounir Lamouri (:mounir)]

Attachment: Patch

Comment 13

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a35a5681fe2f

Comment 14

[Phil Ringnalda (:philor)]

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

Comment 15

[(no longer active)]

Mounir, do you still think we should take this patch?

Comment 16

[Mounir Lamouri (:mounir)]

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

Comment 17

[Mounir Lamouri (:mounir)]

Attachment: Patch with tests

Comment 18

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c9062e64d9c0

Comment 19

[Carsten Book [:Tomcat]]

(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 ?

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c9062e64d9c0

Comment 21

[Mounir Lamouri (:mounir)]

I meant to remove checkin-needed, to dev-doc-needed...

Comment 23

[hexalys]

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

Comment 24

[Sid Stamm [:geekboy or :sstamm]]

Bruno: You're right.  I filed bug 1023920 to track that work since this one is already resolved.

Comment 26

[Mark Banner (:standard8)]

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.

Comment 27

[Jean-Yves Perrier [:teoli]]

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.

Comment 28

[Schalk Neethling [:espressive]]

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

Comment 29

[Mark Banner (:standard8)]

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.

Comment 30

[Schalk Neethling [:espressive]]

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

Comment 31

[Boris Zbarsky [:bzbarsky]]

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