Closed Bug 1274712 Opened 4 years ago Closed 3 years ago

Copy for string changes to DNT dialog

Categories

(Firefox :: Security, defect, P1)

49 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 49
Iteration:
49.3 - Jun 6
Tracking Status
firefox49 blocking fixed

People

(Reporter: tanvi, Assigned: johannh)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxprivacy])

Attachments

(2 files)

This bug can be used to decide on the new string to use the the DNT dialog.

Some options:
When tracking protection is on, Firefox will send the DNT header.
The DNT header is sent when Tracking Protection is enabled.
When Tracking Protection is enabled, the DNT header is sent.
Whenever Tracking Protection is enabled, the DNT header is sent.

We could potentially add a "Note:" at the beginning of the string.

The last string uses "whenever" to indicate that if Tracking Protection is just on in Private Browsing mode, then the DNT isn't sent automatically in regular mode.

+++ This bug was initially created as a clone of Bug #1274654 +++

When TP is enabled, the DNT header is going to be sent per bug 1258033.

In order to communicate this to users, we need to make changes to the dialog that pops up when you click about:preferences#privacy "manage your DNT settings".  If TP is enabled globally, the DNT checkbox should be checked and grayed out so that the user can't uncheck it.  In addition, there should be some messaging to indicate that DNT is automatically turned on when TP is on.  There was a previous mockup for this here:

https://mozilla.invisionapp.com/share/QS7DW14A2#/screens/117954448_Dnt_State_2

Proposed string from previous mockup:
When tracking protection is always on, Firefox will send the DNT header.

The above works when TP is enabled globally.  I'm not sure how we communicate that DNT is sent in private browsing mode.
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Iteration: --- → 49.2 - May 23
Flags: qe-verify-
Iteration: 49.2 - May 23 → 49.3 - Jun 6
Flags: needinfo?(jmoradi)
Javaun to get us someone to provide copy.
> When Tracking Protection is enabled, the DNT header is sent.

My vote goes to this one.
Tracking/marking as a blocker for 49 since we want to make sure this lands before the merge (because of the string freeze in aurora)
(In reply to Panos Astithas [:past] from comment #2)
> > When Tracking Protection is enabled, the DNT header is sent.
> 
> My vote goes to this one.

I'm fine with that too!
Is the string going to go above or below the checkbox?

Does the Learn More link stay where it is (next to the "Use Do Not Track" text) or does it move?

Thanks!
Flags: needinfo?(agrigas)
(In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #5)
> Is the string going to go above or below the checkbox?
> 
> Does the Learn More link stay where it is (next to the "Use Do Not Track"
> text) or does it move?
> 
> Thanks!
It should look something like this:
https://www.dropbox.com/s/sac3x5u51wfz5zf/Screenshot%202016-05-24%2014.57.03.png?dl=0
Flags: needinfo?(agrigas)
(In reply to agrigas from comment #4)
> (In reply to Panos Astithas [:past] from comment #2)
> > > When Tracking Protection is enabled, the DNT header is sent.
> > 
> > My vote goes to this one.
> 
> I'm fine with that too!

(Sorry to bikeshed.)  Since we are noting something about TP and DNT here, and the sentence isn't about DNT in general, should this be:
Note that when Tracking Protection is enabled, the DNT header is sent.
Note: When Tracking Protection is enabled, the DNT header is sent.

If we feel the "Note" is too technical or awkward, then I'd be fine with out it too.

Thanks!
(In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #7)
> (In reply to agrigas from comment #4)
> > (In reply to Panos Astithas [:past] from comment #2)
> > > > When Tracking Protection is enabled, the DNT header is sent.
> > > 
> > > My vote goes to this one.
> > 
> > I'm fine with that too!
> 
> (Sorry to bikeshed.)  Since we are noting something about TP and DNT here,
> and the sentence isn't about DNT in general, should this be:
> Note that when Tracking Protection is enabled, the DNT header is sent.
> Note: When Tracking Protection is enabled, the DNT header is sent.
> 
> If we feel the "Note" is too technical or awkward, then I'd be fine with out
> it too.
> 
> Thanks!

I think using Note is fine... we don't do it elsewhere in prefs that I can see so this is a bit of an exception if we add it. I don't feel strongly either way since most people won't probably open this dialogue.
(In reply to Panos Astithas [:past] from comment #2)
> > When Tracking Protection is enabled, the DNT header is sent.

Should we say "DNT signal" instead of "DNT header"?

It seems a bit less technical and it's also more accurate since DNT is both a header and a DOM API (and we'll enable both when TP is enabled).
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

https://reviewboard.mozilla.org/r/55426/#review52212

::: browser/locales/en-US/chrome/browser/preferences/donottrack.dtd:6
(Diff revision 1)
> -<!ENTITY window.width                 "40em">
> -<!ENTITY window.height                "8em">
> +<!ENTITY window.width                 "45em">
> +<!ENTITY window.height                "9em">

I tried this on Ubuntu and the Learn more link is cropped to the right. Linux uses slightly larger fonts, so either increase the dialog size or make the label wrap.

::: browser/themes/shared/incontentprefs/preferences.inc.css:228
(Diff revision 1)
>  /* Privacy pane */
>  
>  .doNotTrackLearnMore,
>  #trackingProtectionPBMLearnMore,
>  #trackingProtectionLearnMore {
> -  margin-inline-start: 1.5em !important;
> +  margin-inline-start: 2.5em !important;

Using a value of 2em makes the note align with the checkbox label above. Is there a reason you went with 2.5?
Attachment #8756821 - Flags: review?(past)
(In reply to Panos Astithas [:past] from comment #11)
> Comment on attachment 8756821 [details]
> MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT
> settings. r?past
> 
> https://reviewboard.mozilla.org/r/55426/#review52212
> 
> ::: browser/locales/en-US/chrome/browser/preferences/donottrack.dtd:6
> (Diff revision 1)
> > -<!ENTITY window.width                 "40em">
> > -<!ENTITY window.height                "8em">
> > +<!ENTITY window.width                 "45em">
> > +<!ENTITY window.height                "9em">
> 
> I tried this on Ubuntu and the Learn more link is cropped to the right.
> Linux uses slightly larger fonts, so either increase the dialog size or make
> the label wrap.

Damn, I only tested on OSX and Windows. I'll see if I can come up with a mozscreenshots test for this. (Maybe in a separate bug so that it doesn't block us).

> 
> ::: browser/themes/shared/incontentprefs/preferences.inc.css:228
> (Diff revision 1)
> >  /* Privacy pane */
> >  
> >  .doNotTrackLearnMore,
> >  #trackingProtectionPBMLearnMore,
> >  #trackingProtectionLearnMore {
> > -  margin-inline-start: 1.5em !important;
> > +  margin-inline-start: 2.5em !important;
> 
> Using a value of 2em makes the note align with the checkbox label above. Is
> there a reason you went with 2.5?

I think em is just the wrong measure if we want it to perfectly align, px might be better. The checkbox plus margin measures 32px, minus 4px of the default label margin is 28px. I'll use that number if there are no complaints.
I sent this copy doc to brand yesterday, they'll review it today:
https://docs.google.com/document/d/1r2RbYFQSOuBTkdGkr6p7DQFwcZHbjg4fNthrUgYj4o8/edit
Flags: needinfo?(jmoradi)
(In reply to Johann Hofmann [:johannh] from comment #12)
> > 
> > Using a value of 2em makes the note align with the checkbox label above. Is
> > there a reason you went with 2.5?
> 
> I think em is just the wrong measure if we want it to perfectly align, px
> might be better. The checkbox plus margin measures 32px, minus 4px of the
> default label margin is 28px. I'll use that number if there are no
> complaints.

Ugh, so the problem seems to be that checkbox has a different margin/padding width on different platforms. I'd say we use the 28px that I mentioned (which looks best on Windows) and live with it being 1 or 2 px off on the other platforms.
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55426/diff/1-2/
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

https://reviewboard.mozilla.org/r/55426/#review52416
Attachment #8756821 - Flags: review?(past) → review+
Final Copy = "Firefox will send a signal that you don't want to be tracked whenever Tracking Protection is on"

(this has been approved by the list of folks who needed to).
Comment on attachment 8757026 [details]
MozReview Request: Bug 1274712 - Add a mozscreenshots configuration for the DNT dialog. r=MattN

https://reviewboard.mozilla.org/r/55536/#review52646
Attachment #8757026 - Flags: review?(MattN+bmo) → review+
There are two changes in the DNT dialog. 

1. The checkbox copy now says 
"Always apply Do Not Track" 

2. We are adding a second line of text below the checkbox line that says:
"Firefox will send a signal that you don't want to be tracked whenever Tracking Protection is on. [Learn More"

The Learn More link destination remains the same, which is the SUMO DNT page.
Flags: needinfo?(past)
Johann, can you update the patch per comment 22?
Flags: needinfo?(past)
Comment on attachment 8757026 [details]
MozReview Request: Bug 1274712 - Add a mozscreenshots configuration for the DNT dialog. r=MattN

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55536/diff/1-2/
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55426/diff/2-3/
With the longer text I decided to break the description, it looked goofy with a really long dialog box. That comes with some more changes that I'd like reviewed. Panos, can you take another quick look?
Attachment #8756821 - Flags: review+ → review?(past)
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

https://reviewboard.mozilla.org/r/55426/#review52866

::: browser/components/preferences/donottrack.xul:37
(Diff revision 3)
> -      <checkbox label="&doNotTrackCheckbox.label;"
> +      <vbox>
> +        <checkbox label="&doNotTrackCheckbox2.label;"
> -                accesskey="&doNotTrackCheckbox.accesskey;"
> +                  accesskey="&doNotTrackCheckbox.accesskey;"
> -                preference="privacy.donottrackheader.enabled"/>
> +                  preference="privacy.donottrackheader.enabled"/>
> -      <label class="text-link doNotTrackLearnMore"
> +        <description flex="1"
> +          style="width: calc(&window.width; - 10em);"

This is still not good on Linux, "Tracking" is cut off (I only test on Linux as I assume you make sure OS X and Windows look good). Subtracting 11 em instead of 10 em fixes it though.

::: browser/locales/en-US/chrome/browser/preferences/donottrack.dtd:10
(Diff revision 3)
>  <!ENTITY window.title                 "Do Not Track">
> -<!ENTITY window.width                 "40em">
> -<!ENTITY window.height                "8em">
> +<!ENTITY window.width                 "45em">
> +<!ENTITY window.height                "10em">
>  
> -<!ENTITY doNotTrackCheckbox.label     "Use Do Not Track">
> +<!ENTITY doNotTrackCheckbox2.label    "Always apply Do Not Track">
>  <!ENTITY doNotTrackCheckbox.accesskey "U">

U is no longer a good access key. How about A?

::: browser/themes/shared/incontentprefs/preferences.inc.css:233
(Diff revision 3)
>    margin-top: 0;
>    font-weight: normal;
>  }
>  
> +.doNotTrackLearnMore {
> +  margin-inline-start: 28px;

This margin breaks the layout on Linux, causing the OK button to be clipped on the right.
Attachment #8756821 - Flags: review?(past)
https://reviewboard.mozilla.org/r/55426/#review52866

> This is still not good on Linux, "Tracking" is cut off (I only test on Linux as I assume you make sure OS X and Windows look good). Subtracting 11 em instead of 10 em fixes it though.

Yeah sorry I should test on Linux as well. I did a push to mozscreenshots (https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=c57a0e71e7cdf62ba3d443e250e6086368ad0833&newProject=try&newRev=867cfbcb5484b610621476b0b6864e2d3e630a7a&filter=dnt) before pushing to check if Linux is fine. Turns out you're right :)
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55426/diff/3-4/
Attachment #8756821 - Flags: review?(past)
The previous solution was pretty awkward, I wasn't really happy with it. This one is much simpler and I've tested it on all 3 platforms now. Although you might want to wait for the mozscreenshot result[0] to come in (should take about an hour) before doing another review :)

[0]https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=10a2a0ed4d329e7509883acb17d8817319e28017&newProject=try&newRev=57b3019adc34cdbe64538ca79301a96a07621a3f&filter=dnt
Attachment #8756821 - Flags: review?(past) → review+
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

https://reviewboard.mozilla.org/r/55426/#review53090

r=me with the change below.

::: browser/themes/shared/incontentprefs/preferences.inc.css:238
(Diff revision 4)
> +  margin-inline-start: calc(1em + 30px);
> +  margin-bottom: 1em;
> +  font-weight: normal;
> +}
> +
> +.doNotTrackLearnMore label {

This is the only bit that I'm not happy with: the descendant selector is not good for performance:

https://wiki.mozilla.org/Firefox/CSS_Tips#Performance

Either use an ID or a child selector like:
.doNotTrackLearnMore > .text-link
Comment on attachment 8757026 [details]
MozReview Request: Bug 1274712 - Add a mozscreenshots configuration for the DNT dialog. r=MattN

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55536/diff/2-3/
Attachment #8757026 - Attachment description: MozReview Request: Bug 1274712 - Add a mozscreenshots configuration for the DNT dialog. r?MattN → MozReview Request: Bug 1274712 - Add a mozscreenshots configuration for the DNT dialog. r=MattN
Attachment #8756821 - Attachment description: MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r?past → MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past
Comment on attachment 8756821 [details]
MozReview Request: Bug 1274712 - Add Tracking Protection info to DNT settings. r=past

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55426/diff/4-5/
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/b4a3f56b263f
Add a mozscreenshots configuration for the DNT dialog. r=MattN
https://hg.mozilla.org/integration/fx-team/rev/fae4da38e0b4
Add Tracking Protection info to DNT settings. r=past
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b4a3f56b263f
https://hg.mozilla.org/mozilla-central/rev/fae4da38e0b4
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Sadly this needs a fix as soon as possible, given we're a few days from merge day
https://hg.mozilla.org/mozilla-central/diff/fae4da38e0b4/browser/locales/en-US/chrome/browser/preferences/donottrack.dtd

You changed the ID for the string (good) but reused the existing accesskey (very bad).
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

The existing acceskey might not be available in the new string, and you broke the relation between the two strings.

Glad to file a follow-up bug if you prefer.
Flags: needinfo?(jhofmann)
(In reply to Francesco Lodolo [:flod] from comment #39)
> Sadly this needs a fix as soon as possible, given we're a few days from
> merge day
> https://hg.mozilla.org/mozilla-central/diff/fae4da38e0b4/browser/locales/en-
> US/chrome/browser/preferences/donottrack.dtd
> 
> You changed the ID for the string (good) but reused the existing accesskey
> (very bad).
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/
> Localization_content_best_practices#Changing_existing_strings
> 
> The existing acceskey might not be available in the new string, and you
> broke the relation between the two strings.
> 
> Glad to file a follow-up bug if you prefer.

Yes a follow-up would be nice. I'll take care of it. Thank you!
Flags: needinfo?(jhofmann)
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/fx-team/rev/6833ddd73063
Follow-up to also change doNotTrackCheckbox.accesskey. rs=flod,johannh
You need to log in before you can comment on or make changes to this bug.