Closed
Bug 1274712
Opened 8 years ago
Closed 8 years ago
Copy for string changes to DNT dialog
Categories
(Firefox :: Security, defect, P1)
Tracking
()
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.
Updated•8 years ago
|
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jhofmann
Updated•8 years ago
|
Status: NEW → ASSIGNED
Iteration: --- → 49.2 - May 23
Flags: qe-verify-
Updated•8 years ago
|
Iteration: 49.2 - May 23 → 49.3 - Jun 6
Updated•8 years ago
|
Flags: needinfo?(jmoradi)
Comment 1•8 years ago
|
||
Javaun to get us someone to provide copy.
Comment 2•8 years ago
|
||
> When Tracking Protection is enabled, the DNT header is sent.
My vote goes to this one.
Comment 3•8 years ago
|
||
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)
tracking-firefox49:
--- → blocking
Comment 4•8 years ago
|
||
(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!
Reporter | ||
Comment 5•8 years ago
|
||
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)
Comment 6•8 years ago
|
||
(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)
Reporter | ||
Comment 7•8 years ago
|
||
(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!
Comment 8•8 years ago
|
||
(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.
Comment 9•8 years ago
|
||
(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).
Assignee | ||
Comment 10•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55426/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55426/
Attachment #8756821 -
Flags: review?(past)
Comment 11•8 years ago
|
||
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)
Assignee | ||
Comment 12•8 years ago
|
||
(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.
Comment 13•8 years ago
|
||
I sent this copy doc to brand yesterday, they'll review it today: https://docs.google.com/document/d/1r2RbYFQSOuBTkdGkr6p7DQFwcZHbjg4fNthrUgYj4o8/edit
Flags: needinfo?(jmoradi)
Assignee | ||
Comment 14•8 years ago
|
||
(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.
Assignee | ||
Comment 15•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55536/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55536/
Attachment #8757026 -
Flags: review?(MattN+bmo)
Attachment #8756821 -
Flags: review?(past)
Assignee | ||
Comment 16•8 years ago
|
||
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/
Assignee | ||
Comment 17•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d15826a00be
Assignee | ||
Comment 18•8 years ago
|
||
Screenshots on https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=49beae1792077eba5b790baa7d3c1a37ca3eb609&newProject=try&newRev=4d15826a00be471a58b7a3b7f6a36d5691f0faff&filter=dnt Looks fine to me now
Comment 19•8 years ago
|
||
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+
Comment 20•8 years ago
|
||
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 21•8 years ago
|
||
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+
Comment 22•8 years ago
|
||
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)
Assignee | ||
Comment 24•8 years ago
|
||
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/
Assignee | ||
Comment 25•8 years ago
|
||
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/
Assignee | ||
Comment 26•8 years ago
|
||
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?
Assignee | ||
Updated•8 years ago
|
Attachment #8756821 -
Flags: review+ → review?(past)
Assignee | ||
Comment 27•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=867cfbcb5484
Comment 28•8 years ago
|
||
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)
Assignee | ||
Comment 29•8 years ago
|
||
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 :)
Assignee | ||
Comment 30•8 years ago
|
||
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)
Assignee | ||
Comment 31•8 years ago
|
||
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
Updated•8 years ago
|
Attachment #8756821 -
Flags: review?(past) → review+
Comment 32•8 years ago
|
||
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
Assignee | ||
Comment 33•8 years ago
|
||
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
Assignee | ||
Comment 34•8 years ago
|
||
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/
Assignee | ||
Comment 35•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbe43290f24a
Assignee | ||
Comment 36•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1bfe66af3593
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 37•8 years ago
|
||
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
Comment 38•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b4a3f56b263f https://hg.mozilla.org/mozilla-central/rev/fae4da38e0b4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment 39•8 years ago
|
||
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)
Assignee | ||
Comment 40•8 years ago
|
||
(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)
Comment 41•8 years ago
|
||
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/fx-team/rev/6833ddd73063 Follow-up to also change doNotTrackCheckbox.accesskey. rs=flod,johannh
Comment 42•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6833ddd73063
You need to log in
before you can comment on or make changes to this bug.
Description
•