854077 - Change - Use radio buttons for "Do Not Track" options and add a neutral option

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Flyouts, defect, P1)

Description

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Privacy is pushing "Do No Track" to have 3 exclusive mutual options instead of 2 options(On/Off).

We need to use a drop-down menu instead.

Title: Do Not Track

Options:
1. Do not tell sites my tracking preferences [Default]

2. Tell sites that I do not want to be tracked

3. Tell sites that I want to be tracked

Comment 1

[Sid Stamm [:geekboy or :sstamm]]

I had proposed both the three radios and a drop down in bug 765398, but dolske suggested the three radios approach in bug 765398 comment 3.  

(other mock-up: [obsolete] attachment 633705 [details] on bug 765398)

I'm a little concerned that a drop-down with just the three options listed above in the description would make it such that the other two options are not discoverable; it's not clearly obvious when reading "Do not tell sites my tracking preference" that the other options exist and are different.

Just my two cents... I'll defer to the UX folks for design and such, but wanted to weigh in a little since I've spent lots of time on this.

Comment 2

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

(In reply to Sid Stamm [:geekboy or :sstamm] from comment #1)
> I had proposed both the three radios and a drop down in bug 765398, but
> dolske suggested the three radios approach in bug 765398 comment 3.  
> 
> (other mock-up: [obsolete] attachment 633705 [details] on bug 765398)
> 
> I'm a little concerned that a drop-down with just the three options listed
> above in the description would make it such that the other two options are
> not discoverable; it's not clearly obvious when reading "Do not tell sites
> my tracking preference" that the other options exist and are different.
> 
> Just my two cents... I'll defer to the UX folks for design and such, but
> wanted to weigh in a little since I've spent lots of time on this.

Thanks a lot for the feedback, Sid. 
To understand the context, I went through the discussions you guys have had in Bug 765398. Now I think using Radio buttons make better sense here. 

Windows 8 apps often prefer drop-down menus over radio buttons, in order to keep the flyout clean and simple. But in this particular case, to make a sound decision, the users often need to read through 3 options first. Using radio buttons to show 3 options together could provide the users more clarity. 


I have a question regarding to attachment 633705 [details] you posted:
The order on this mockup is [Yes, No, Neutral]. But in the comments Bug 765398, Matej was using the order [Yes, Neutral, No] all the time. 
Could you please clarify the rationale of that change?

In my opinion, since "Yes" and "No" both starts with "Tell sites that I..", placing them together might make it harder to scan and tell the difference. 
Regardless of the order, I think at least for Windows 8 FX, we would like to list the options under one phrase: "Tell sites". 

Tell sites:
- I don't want to be tracked
- Nothing about my tracking preferences
- I want to be tracked

Let me know if you have other preference on the wording of the neutral option.

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

If it is clear what the "default" is, I am not too sure order matters, though I'm willing to bet that top-to-bottom scan order will bias choice of the first option.  (This is, I think, why the "send nothing" option is default in the patch to mainstream Firefox.)

I think as long as the wording is reasonably honest that this feature is just "telling sites" things, it's up to you, Matej, and the other wordsmiths out there to make it clear.  :)  Your proposal above sounds pretty good.  The one thing I ask is that "do not" remain separate and not contracted to "don't", since it will sound more like the feature's name ("do not track") this way.

Comment 4

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: [Mockup] Do No Track with 3 options

Comment 5

[Brian R. Bondy [:bbondy]]

it5?

Comment 6

[Marco Mucci [:MarcoM]]

Moving to Iteration #5 for consideration as a Change.

Comment 7

[Marco Mucci [:MarcoM]]

Being placed into Story Backlog based on Asa's review.

Comment 8

[Marco Mucci [:MarcoM]]

For future consideration: p=1.

Defect Stories will be taking priority over Change Stories.

Comment 9

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Patch v1

Comment 10

[Allison Naaktgeboren :ally]

Comment on attachment 737196 [details] [diff] [review]
Patch v1

Review of attachment 737196 [details] [diff] [review]:
-----------------------------------------------------------------

looks pretty good overall; a couple of nits and a follow up question.

::: browser/metro/base/content/bindings/toggleswitch.xml
@@ +20,5 @@
>          <xul:description anonid="onlabel" class="onlabel" value="&checkbox.on.label;" xbl:inherits="value=onlabel"/>
>          <xul:description anonid="offlabel" class="offlabel" value="&checkbox.off.label;" xbl:inherits="value=offlabel"/>
>          <xul:radiogroup anonid="group" xbl:inherits="disabled">
> +          <xul:radio type="toggle" anonid="on" class="checkbox-radio-on"/>
> +          <xul:radio type="toggle" anonid="off" class="checkbox-radio-off"/>

Limitation of reviewer's brain: Why was this change necessary?

::: browser/metro/base/content/preferences.js
@@ +15,5 @@
> +  },
> +  onDNTPreferenceChanged: function onDNTPreferenceChanged() {
> +    let dntValue = document.getElementById("prefs-dnt-options").selectedItem.value;
> +
> +    Services.prefs.setBoolPref("privacy.donottrackheader.enabled", dntValue !== "-1");

is there any way we can get around using a magic number? (-1)?

::: browser/metro/base/tests/mochitest/browser_sanitize_ui.js
@@ +98,2 @@
>  
> +function checkDNTPrefs(aExpectedEnabled, aExpectedValue) {

Thank you so much for slogging through getting a test going.

@@ +100,5 @@
> +  let currentEnabled = Services.prefs.getBoolPref("privacy.donottrackheader.enabled");
> +  let currentValue = Services.prefs.getIntPref("privacy.donottrackheader.value");
> +
> +  ok(aExpectedEnabled === currentEnabled,
> +    "testing privacy.donottrackheader.enabled, expected " + aExpectedEnabled + " got " + currentEnabled);

nit: Can we break out the long strings into their own variables?
let enabledTestMsg =  "testing privacy.donottrackheader.enabled, expected " + aExpectedEnabled + " got " + currentEnabled;
ok(aExpectedEnabled === currentEnabled, enabledTestMsg);

@@ +119,3 @@
>      yield promise;
>  
> +    noPref.click();

quick nit: could you add a comment indicating where the magic numbers are set? That way if someone who isn't us breaks this test when we are not around, they can find things faster.

Comment 11

[Rodrigo Silveira [:rsilveira] [:rodms]]

(In reply to :Ally Naaktgeboren from comment #10)

Thanks for the review!

> ::: browser/metro/base/content/bindings/toggleswitch.xml
> @@ +20,5 @@
> >          <xul:description anonid="onlabel" class="onlabel" value="&checkbox.on.label;" xbl:inherits="value=onlabel"/>
> >          <xul:description anonid="offlabel" class="offlabel" value="&checkbox.off.label;" xbl:inherits="value=offlabel"/>
> >          <xul:radiogroup anonid="group" xbl:inherits="disabled">
> > +          <xul:radio type="toggle" anonid="on" class="checkbox-radio-on"/>
> > +          <xul:radio type="toggle" anonid="off" class="checkbox-radio-off"/>
> 
> Limitation of reviewer's brain: Why was this change necessary?
> 

Radiogroups were not rendering the radio circle, that was because the metro toggle switch (the on/off) uses radio but doesn't want to render the circles. I've added this new attribute to distinguish them when adding the binding at content/browser.css

> ::: browser/metro/base/content/preferences.js
> @@ +15,5 @@
> > +  },
> > +  onDNTPreferenceChanged: function onDNTPreferenceChanged() {
> > +    let dntValue = document.getElementById("prefs-dnt-options").selectedItem.value;
> > +
> > +    Services.prefs.setBoolPref("privacy.donottrackheader.enabled", dntValue !== "-1");
> 
> is there any way we can get around using a magic number? (-1)?
> 

The values of the radio items are automatically sent to the appropriate pref by the setting tag, so it makes things easier to have them match what the pref expects. I'll change this code to see if the radio item with id "prefs-dnt-nopref" is selected instead of checking the value, that would make it clearer.

> ::: browser/metro/base/tests/mochitest/browser_sanitize_ui.js
> @@ +98,2 @@
> >  
> > +function checkDNTPrefs(aExpectedEnabled, aExpectedValue) {
> 
> Thank you so much for slogging through getting a test going.
> 
> @@ +100,5 @@
> > +  let currentEnabled = Services.prefs.getBoolPref("privacy.donottrackheader.enabled");
> > +  let currentValue = Services.prefs.getIntPref("privacy.donottrackheader.value");
> > +
> > +  ok(aExpectedEnabled === currentEnabled,
> > +    "testing privacy.donottrackheader.enabled, expected " + aExpectedEnabled + " got " + currentEnabled);
> 
> nit: Can we break out the long strings into their own variables?
> let enabledTestMsg =  "testing privacy.donottrackheader.enabled, expected "
> + aExpectedEnabled + " got " + currentEnabled;
> ok(aExpectedEnabled === currentEnabled, enabledTestMsg);
> 

Done.

> @@ +119,3 @@
> >      yield promise;
> >  
> > +    noPref.click();
> 
> quick nit: could you add a comment indicating where the magic numbers are
> set? That way if someone who isn't us breaks this test when we are not
> around, they can find things faster.

Sure.

Comment 12

[Rodrigo Silveira [:rsilveira] [:rodms]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0536fbab2e8c

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0536fbab2e8c

Comment 14

[Marco Mucci [:MarcoM]]

For Kamil to test and verify.

Comment 15

[Kamil Jozwiak [:kjozwiak]]

Used the following build to verify and test the following "Change":

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-04-18-03-10-48-mozilla-central/

Went through the following test cases:

- Ensured that the feature in Firefox Metro matches the mockup that has been attached
- Ensured that the selections are radio buttons as mentioned in Comment 2
- Ensured that the correct order appears in "Options" as mentioned in Comment 2
- Ensured that toggling DNT on the Firefox Desktop doesn't affect Firefox Metro
- Installed Firefox Metro several times and ensured that "I do not want to be tracked" was set as [Default] every single time
- Ensured that all three options worked without any issues (checking using http://donottrack.us/)
- Ensured that you can tap & click the selections without any problems (radio button selecting the correct item)
- Ensured that selecting the back button under "Options" went back to the "Settings" charm
- Ensured that the radio buttons worked without any issues when in Filled View
- Ensured that the "Do Not Track" area aligned with the rest of the options

Comment 16

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Verified for it8

Comment 17

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Change" for iteration #9 testing without any issues. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-06-27-03-10-27-mozilla-central/

- Went through the original attached story without any issues
- Went through the test cases added in comment 15 without any issues

Comment 18

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130826074752
Built from http://hg.mozilla.org/mozilla-central/rev/14b1e8c2957e

WFM
Tested on windows 8 using latest nightly for iteration-12. I saw radio buttons and neutral option for tracking preferences.

Comment 19

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

We need to adapt the recent changes for DNT on Android and Firefox OS to stay consistent. 

Notes from Alina's privacy review: https://bugzilla.mozilla.org/show_bug.cgi?id=885129#c13

* DNT three-state (pg 6) - We should make this consistent with how DNT is presented on Fennec and Fx OS v1.2. Here is the copy for the three-state setting in the order in which it should be presented (which is the same across Desktop, Fennec and Fx OS):
--
O Tell websites that I do not want to be tracked.
O Tell websites that I do want to be tracked.
O Do not tell websites anything about my tracking preferences.
--
The default above should be set to the last ('Do not tell...'). 
A screenshot of Firefox OS DNT:
https://bug885129.bugzilla.mozilla.org/attachment.cgi?id=805689&t=BUfdYvdXSK

*If there is no room to include copy about 'How does Do Not Track work', you can include a "Learn More" link to https://www.mozilla.org/en-US/dnt/ under the three options.

Comment 20

[Marco Mucci [:MarcoM]]

Hi Yuan, please file a new follow up bug for Comment #19.

Comment 21

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Filed a new follow up bug at Bug 917422

Comment 22

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Change" for iteration #16 testing without any issues. Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-10-22-03-02-02-mozilla-central/

- Went through the original attached story without any issues
- Went through the test cases added in comment 15 without any issues
- Also went through Bug 917422 as it basically also tests this feature (changing the wording, adding a link)