Closed Bug 1370190 Opened 7 years ago Closed 7 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: 7 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: