Closed Bug 1370190 Opened 8 years ago Closed 8 years ago

Updated Preferences tracking protection section based on re-org v2

Categories

(Firefox :: Settings UI, defect, P1)

Product:
Firefox
Component:
Settings UI
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 56
Tracking Flags:
Tracking Status
firefox56 --- fixed
firefox57 --- verified

People

(Reporter: rickychien, Assigned: rickychien)

References

Details

(Whiteboard: [photon-preference])

Attachments

(1 file, 1 obsolete file)

Bug 1370190 - Updated Preferences tracking protection section
59 bytes, text/x-review-board-request
jaws
: review+
Details
Reorg v2 spec https://mozilla.invisionapp.com/share/P4ACQT1E3#/screens/237159280 Tracking Protection section is going to follow above spec (ref to Firefox release styling).
Blocks: 1365133
No longer depends on: 1365133
No longer blocks: photon-preference
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Flags: qe-verify+
Whiteboard: [photon-preference][triage] → [photon-preference]
New spec of Tracking Protection is targeting on privacy.trackingprotection.ui.enabled = false which value is "false" on Firefox release but "true" on Firefox Nightly. We're targeting on Firefox release, so any code changes and QA verification should be under privacy.trackingprotection.ui.enabled = false.
See Also: → 1371344
Comment on attachment 8879046 [details] Bug 1370190 - Updated Preferences tracking protection section https://reviewboard.mozilla.org/r/150364/#review155008 ::: browser/locales/en-US/chrome/browser/preferences/privacy.dtd:30 (Diff revision 1) > <!-- LOCALIZATION NOTE (doNotTrack.pre.label): include a trailing space as needed --> > <!-- LOCALIZATION NOTE (doNotTrack.post.label): include a starting space as needed --> > <!ENTITY doNotTrack.pre.label "You can also "> > <!ENTITY doNotTrack.settings.label "manage your Do Not Track settings"> > <!ENTITY doNotTrack.post.label "."> > +<!ENTITY doNotTrack.description "Send website a “Do Not Track” signal that you don’t want to be tracked"> websites
Comment on attachment 8879046 [details] Bug 1370190 - Updated Preferences tracking protection section https://reviewboard.mozilla.org/r/150364/#review155024 ::: browser/locales/en-US/chrome/browser/preferences/privacy.dtd:19 (Diff revision 2) > <!ENTITY trackingProtectionLearnMore.label "Learn more"> > <!ENTITY trackingProtectionExceptions.label "Exceptions…"> > <!ENTITY trackingProtectionExceptions.accesskey "x"> > > -<!ENTITY tracking.label "Tracking"> > -<!ENTITY trackingProtectionPBM5.label "Use Tracking Protection in Private Windows"> > +<!ENTITY tracking.description "Tracking is the collection of your browsing data across multiple sites. Tracking can be used to build a profile and display content based on your browsing and personal information."> > +<!ENTITY trackingProtectionPBM5.label "Use Tracking Protection in Private Browsing to block known tracking companies"> You need to change this string ID, and also the accesskey (not the Learn more one)
Comment on attachment 8879046 [details] Bug 1370190 - Updated Preferences tracking protection section https://reviewboard.mozilla.org/r/150364/#review155024 > You need to change this string ID, and also the accesskey (not the Learn more one) Oh, thanks for reminding.
Comment on attachment 8879046 [details] Bug 1370190 - Updated Preferences tracking protection section https://reviewboard.mozilla.org/r/150364/#review155034 ::: browser/locales/en-US/chrome/browser/preferences/privacy.dtd:18 (Diff revision 3) > <!ENTITY trackingProtectionNever.accesskey "n"> > <!ENTITY trackingProtectionLearnMore.label "Learn more"> > <!ENTITY trackingProtectionExceptions.label "Exceptions…"> > <!ENTITY trackingProtectionExceptions.accesskey "x"> > > -<!ENTITY tracking.label "Tracking"> > +<!ENTITY tracking.description2 "Tracking is the collection of your browsing data across multiple sites. Tracking can be used to build a profile and display content based on your browsing and personal information."> This string is brand new, no need for 2.
Comment on attachment 8879046 [details] Bug 1370190 - Updated Preferences tracking protection section https://reviewboard.mozilla.org/r/150364/#review155062 ::: browser/locales/en-US/chrome/browser/preferences/privacy.dtd:31 (Diff revision 4) > <!-- LOCALIZATION NOTE (doNotTrack.post.label): include a starting space as needed --> > <!ENTITY doNotTrack.pre.label "You can also "> > <!ENTITY doNotTrack.settings.label "manage your Do Not Track settings"> > <!ENTITY doNotTrack.post.label "."> > +<!ENTITY doNotTrack.description "Send websites a “Do Not Track” signal that you don’t want to be tracked"> > +<!ENTITY doNotTrack.learnMore.label "Learn More"> Sorry, this is some very poor approach to patch review on my side .-\ When doing a final check I noticed that the case here is wrong, it should be "Learn more" according to specs and other strings in the file.
Comment on attachment 8879046 [details] Bug 1370190 - Updated Preferences tracking protection section https://reviewboard.mozilla.org/r/150364/#review157246 r- mainly because of the first issue. ::: browser/components/preferences/in-content-new/findInPage.js:278 (Diff revision 8) > - if (nodeObject.childElementCount == 0 || nodeObject.tagName == "menulist") { > + if (nodeObject.childElementCount == 0 || > + nodeObject.tagName == "menulist" || > + nodeObject.tagName == "label" || > + nodeObject.tagName == "description") { Why is this change part of this patch? It doesn't appear to have anything specific to Tracking and if it's fixing a different bug then it should be attached to that bug. ::: browser/locales/en-US/chrome/browser/preferences/privacy.dtd:6 (Diff revision 8) > <!-- This Source Code Form is subject to the terms of the Mozilla Public > - License, v. 2.0. If a copy of the MPL was not distributed with this > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > > <!ENTITY trackingProtectionHeader2.label "Tracking Protection"> > -<!ENTITY trackingProtection.description "Tracking is when companies collect information about you to build a profile and display content based on your browsing and personal data."> > +<!ENTITY trackingProtection.description "Tracking is the collection of your browsing data across multiple sites. Tracking can be used to build a profile and display content based on your browsing and personal information."> Please replace "sites" with "websites". See bug 1375883 for more information. ::: browser/locales/en-US/chrome/browser/preferences/privacy.dtd:20 (Diff revision 8) > +<!ENTITY trackingProtectionPBM.new.label "Use Tracking Protection in Private Browsing to block known trackers"> > +<!ENTITY trackingProtectionPBM.new.accesskey "v"> Please don't use "new" within the entity name as it won't be "new" forever. I would prefer instead that you use trackingProtectionPBM6.
Attachment #8879046 - Flags: review?(jaws) → review-
Comment on attachment 8879046 [details] Bug 1370190 - Updated Preferences tracking protection section https://reviewboard.mozilla.org/r/150364/#review157504
Attachment #8879046 - Flags: review?(jaws) → review+
Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c41d4abf284 Updated Preferences tracking protection section r=jaws
https://hg.mozilla.org/mozilla-central/rev/5c41d4abf284
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
> -<!ENTITY trackingProtection.description "Tracking is when companies collect information about you to build a profile and display content based on your browsing and personal data."> > -<!ENTITY trackingProtection.radioGroupLabel "Block known tracking companies from displaying content"> > +<!ENTITY trackingProtection.description "Tracking is the collection of your browsing data across multiple websites. Tracking can be used to build a profile and display content based on your browsing and personal information."> > +<!ENTITY trackingProtection.radioGroupLabel "Use Tracking Protection to block known trackers"> Shouldn’t these 2 entities be renamed?
(In reply to Ton from comment #21) > Shouldn’t these 2 entities be renamed? It certainly does (and it wasn't there when I looked at the patch) :-\ I don't want this backed out, given how much it took to land it, but please fix it as soon as possible.
Flags: needinfo?(rchien)
Attached file bug1370190.patch (obsolete) —
Rename trackingProtection.description and trackingProtection.radioGroupLabel to trackingProtection2.description and trackingProtection2.radioGroupLabel.
(In reply to Onno Ekker [:nONoNonO UTC+1] from comment #23) > Created attachment 8881374 [details] > bug1370190.patch > > Rename trackingProtection.description and trackingProtection.radioGroupLabel > to trackingProtection2.description and trackingProtection2.radioGroupLabel. I appreciate the effort (really), but let's not attach patches to bugs assigned to other people and without mozreview ;-) @rickychien Please also add an explanation on why we have trackingProtectionPBM5.label and trackingProtectionPBM6.label, because it's all but clear from the localization file if you don't have access to the mock-ups. Next time, please ask for feedback when you're touching so many strings.
Attachment #8881374 - Attachment is obsolete: true
Attachment #8881374 - Attachment is patch: false
Depends on: 1376420
Moving NI to bug 1376420
Flags: needinfo?(rchien)
Depends on: 1379210
On latest Nightly build 56.0a1, from 07.26.2017, verified following scenarios: Scenario1 1.Launch Firefox 2.Go to "about:config" and make sure parameter "privacy.trackingprotection.ui.enabled" is false. 3.Restart browser 4.Go to "about:preferences" 5.In the Privacy & Security tab - Browser Privacy check the "Tracking Protection" section Expected Result Verify this section looks like in the specs - https://mozilla.invisionapp.com/share/P4ACQT1E3#/screens/237159280 Scenario2 1.Launch Firefox 2.Go to "about:config" and make sure parameter "privacy.trackingprotection.ui.enabled" is true. 3.Restart browser 4.Go to "about:preferences" 5.In the Privacy & Security tab - Browser Privacy check the "Tracking Protection" section Expected Result Verify this section looks like in the specs - Nightly version - https://mozilla.invisionapp.com/share/P4ACQT1E3#/screens/239744864 Should I cover other scenarios for this bug?
Flags: needinfo?(rchien)
> Should I cover other scenarios for this bug? No, Scenario 1 + 2 look great to me and they are definitely able to cover our new tracking protection spec. Thanks
Flags: needinfo?(rchien)
Based on comment 26 and comment 27 I'll mark this as verified as fixed.
Status: RESOLVED → VERIFIED
status-firefox57: --- → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: