1480900 - Update Content Blocking section copy and position (should be at the top)

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Tanvi Vyas[:tanvi]]

As discussed with UX, the Content Blocking section should be the first thing in about:preferences#privacy.  Bryan, please confirm.

Also, Bryan - should Cookies and Site Data be the second thing in about:preferences#privacy?

Comment 1

[Tanvi Vyas[:tanvi]]

Needinfo Bryan ...

(In reply to Tanvi Vyas[:tanvi] from comment #0)
> As discussed with UX, the Content Blocking section should be the first thing
> in about:preferences#privacy.  Bryan, please confirm.
> 
> Also, Bryan - should Cookies and Site Data be the second thing in
> about:preferences#privacy?

Comment 2

[bbell]

Yeah is we can manage it we should lead with our best security feature. So Content Blocking and Cookies should be section one and two.

Comment 3

[Tanvi Vyas[:tanvi]]

Johann, can you take this?

Comment 4

[Johann Hofmann [:johannh]]

Yup, but I'm waiting on bug 1483378 to land since this one is simple and I don't want to mess up Ehsan's patch.

Comment 5

[mheubusch]

For preferences strings, please use the following: 

Content Blocking 
Block third-party content, like ads or cookies, that can slow your browsing and follow you around the web. Customize your settings for the best balance of protection and performance. 

[Restore Defaults] [Manage Exceptions . . .]
[link Learn more https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/content-blocking]

[toggle ON/OFF]

Choose what to block

[turtle]Slow-Loading Trackers
Block trackers that take longer than 5 seconds to load. 

[feet]All Detected Trackers
Block all detected trackers (May prevent some pages from loading.)
Only in Private Browsing mode
Always

Change block list

Third-Party Cookies
Block all third-party cookies or just those set by trackers and websites you haven't visited. 
Trackers (recommended)
Trackers and cookies from unvisited sites
All third-party cookies (may cause websites to break)

Send websites a Do Not Track signal. Learn more


Cookies and site data has new options in the radio buttons and drop down menu:
Accept cookies and site data
Block cookies and site data

Type blocked
* Third-party trackers (recommended)
* Third-party trackers and cookies from unvisited websites
* Cookies from unvisited websites [deprecated in Firefox 65]
* All third-party cookies 
* All cookies (may cause websites to break)

Comment 6

[(no longer active)]

(In reply to mheubusch from comment #5)
> For preferences strings, please use the following: 
> 
> Third-Party Cookies
> Block all third-party cookies or just those set by trackers and websites you
> haven't visited. 
> Trackers (recommended)
> Trackers and cookies from unvisited sites
> All third-party cookies (may cause websites to break)
> 
> Type blocked
> * Third-party trackers (recommended)
> * Third-party trackers and cookies from unvisited websites
> * Cookies from unvisited websites [deprecated in Firefox 65]
> * All third-party cookies 
> * All cookies (may cause websites to break)

Hi Michelle,

It seems like in the Cookies & Site Data section, we're saying that blocking "all cookies" may cause websites to break, but in the Content Blocking section, we're saying that blocking "all third-party cookies" may cause websites to break.  This seems inconsistent, since these two settings do very different things.

Is this intentional?  Thanks!

Comment 7

[mheubusch]

Thank you, Ehsan - Good catch.  I believe that both are true - if this is the case we should append

(may cause websites to break) to the All third-party cookies item in the Cookies section and also keep (may cause websites to break) in the two places it already exists.

Comment 8

[Tanvi Vyas[:tanvi]]

(In reply to :Ehsan Akhgari from comment #6)
> Is this intentional?  Thanks!

It was intentional.  The idea was that the audience that would change "Content Blocking" would be larger and less experienced than the audience that would change the dropdown in "Cookies and Site Data".  We can discuss further in our meeting today.

Comment 9

[Tanvi Vyas[:tanvi]]

Also note, this bug is not for copy changes.  It is just about moving the sections around.

Comment 10

[Johann Hofmann [:johannh]]

(In reply to Tanvi Vyas[:tanvi] from comment #9)
> Also note, this bug is not for copy changes.  It is just about moving the
> sections around.

I think it's fine, I'll just morph the bug title to fit both.

Comment 12

[(no longer active)]

Attachment: Part 1: Update the Content Blocking section copy based on the latest strings

Comment 13

[(no longer active)]

Attachment: Part 2: Reorder the Privacy pane of the Preferences UI to move Content Blocking to the top, followed by Cookies & Site Data

The reason why I chose to do this dynamically instead of modifying the XUL markup
was to ensure that no changes are yet made in case the content blocking UI pref is
disabled.

Comment 14

[(no longer active)]

Comment on attachment 9005417 [details] [diff] [review]
Part 1: Update the Content Blocking section copy based on the latest strings

This patch has some unresolved XUL layout problem (the warning becomes way too wide) so it can't land in its current form, but it should be good for a review.

Any fixes I can apply on top of this.

I asked jaws for help on IRC, he wasn't sure where the problem is coming from.  He suggested asking help from Enn or dao...

Comment 15

[(no longer active)]

Attachment: Part 1: Update the Content Blocking section copy based on the latest strings

This patch does not yet replace content-blocking-reject-trackers-warning-all-cookies-blocked
and content-blocking-reject-trackers-warning-from-unvisited-cookies-blocked with the new
string content-blocking-reject-trackers-warning-your-settings-prevent-changes because of a
pending XUL layout issue that hasn't been resolved yet.

Comment 16

[Johann Hofmann [:johannh]]

Comment on attachment 9005498 [details] [diff] [review]
Part 1: Update the Content Blocking section copy based on the latest strings

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

Thanks!

::: browser/locales/en-US/browser/preferences/preferences.ftl
@@ +858,5 @@
> +# This is a warning message shown next to a yellow warning icon when the Third-Party Cookies subsection
> +# of the Content Blocking UI in Preferences has been disabled due to the either the "All cookies" option
> +# or the "Cookies from unvisited websites" option being selected in the Cookies and Site Data section of
> +# the UI.
> +content-blocking-reject-trackers-warning-your-settings-prevent-changes = Your settings in Cookies and Site Data are preventing changes to Third-Party Cookies settings.

flod may have an opinion on whether it's necessary to call out in a comment that this is not in product yet

Comment 17

[(no longer active)]

(In reply to Johann Hofmann [:johannh] from comment #16)
> Comment on attachment 9005498 [details] [diff] [review]
> Part 1: Update the Content Blocking section copy based on the latest strings
> 
> Review of attachment 9005498 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks!
> 
> ::: browser/locales/en-US/browser/preferences/preferences.ftl
> @@ +858,5 @@
> > +# This is a warning message shown next to a yellow warning icon when the Third-Party Cookies subsection
> > +# of the Content Blocking UI in Preferences has been disabled due to the either the "All cookies" option
> > +# or the "Cookies from unvisited websites" option being selected in the Cookies and Site Data section of
> > +# the UI.
> > +content-blocking-reject-trackers-warning-your-settings-prevent-changes = Your settings in Cookies and Site Data are preventing changes to Third-Party Cookies settings.
> 
> flod may have an opinion on whether it's necessary to call out in a comment
> that this is not in product yet

Good point, I would probably do that anyway, it is a good idea since otherwise the localizers may be mislead.

BTW, here is a talos run on part 2: <https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=0f52427ed264&newProject=try&newRevision=20f578768bb3&framework=1&filter=preferences>.  There seems to be no regression as a result of that approach, so I think the patch is OK to land performance wise.

Comment 18

[Francesco Lodolo [:flod]]

Comment on attachment 9005498 [details] [diff] [review]
Part 1: Update the Content Blocking section copy based on the latest strings

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

Technically it looks good, but I'd like to have copy questions answered before landing, to avoid even more churn.

::: browser/locales/en-US/browser/preferences/preferences.ftl
@@ +109,5 @@
>  extension-controlled-websites-tracking-protection-mode = An extension, <img data-l10n-name="icon"/> { $name }, is controlling tracking protection.
>  
> +# This string is shown to notify the user that their content blocking "All Detected Trackers"
> +# preferences are being controlled by an extension.
> +extension-controlled-websites-content-blocking-all-trackers = An extension, <img data-l10n-name="icon"/> { $name }, is controlling this setting.

Why "this setting", when all other similar messages call out the feature?
https://transvision.mozfr.org/?recherche=extension-controlled&repo=gecko_strings&sourcelocale=en-US&locale=en-CA&search_type=strings_entities

@@ +836,3 @@
>  content-blocking-tracking-protection-all-label = All Detected Trackers
>    .accesskey = T
> +content-blocking-tracking-protection-new-description = Block all known trackers. (May prevent some pages from loading.)

Is the period before the parenthetical expected?

@@ +841,5 @@
>    .accesskey = A
>  content-blocking-tracking-protection-option-private =
>    .label = Only in private windows
>    .accesskey = p
> +content-blocking-tracking-protection-change-block-list = Change block list

Is the capitalization change expected?

@@ +858,5 @@
> +# This is a warning message shown next to a yellow warning icon when the Third-Party Cookies subsection
> +# of the Content Blocking UI in Preferences has been disabled due to the either the "All cookies" option
> +# or the "Cookies from unvisited websites" option being selected in the Cookies and Site Data section of
> +# the UI.
> +content-blocking-reject-trackers-warning-your-settings-prevent-changes = Your settings in Cookies and Site Data are preventing changes to Third-Party Cookies settings.

I would avoid adding information that changes (like availability in specific versions) to comments.

Comment 19

[Johann Hofmann [:johannh]]

Comment on attachment 9005454 [details] [diff] [review]
Part 2: Reorder the Privacy pane of the Preferences UI to move Content Blocking to the top, followed by Cookies & Site Data

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

I'm generally not a big fan of doing the reordering this way, but it's probably the only way to do it given that with the old TP UI we want things to stay the same.

Thanks for running this through Talos!

We have a lot to clean up soon.

Comment 20

[(no longer active)]

(In reply to Francesco Lodolo [:flod] from comment #18)
> Comment on attachment 9005498 [details] [diff] [review]
> Part 1: Update the Content Blocking section copy based on the latest strings
> 
> Review of attachment 9005498 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Technically it looks good, but I'd like to have copy questions answered
> before landing, to avoid even more churn.
> 
> ::: browser/locales/en-US/browser/preferences/preferences.ftl
> @@ +109,5 @@
> >  extension-controlled-websites-tracking-protection-mode = An extension, <img data-l10n-name="icon"/> { $name }, is controlling tracking protection.
> >  
> > +# This string is shown to notify the user that their content blocking "All Detected Trackers"
> > +# preferences are being controlled by an extension.
> > +extension-controlled-websites-content-blocking-all-trackers = An extension, <img data-l10n-name="icon"/> { $name }, is controlling this setting.
> 
> Why "this setting", when all other similar messages call out the feature?
> https://transvision.mozfr.org/?recherche=extension-
> controlled&repo=gecko_strings&sourcelocale=en-US&locale=en-
> CA&search_type=strings_entities
> 
> @@ +836,3 @@
> >  content-blocking-tracking-protection-all-label = All Detected Trackers
> >    .accesskey = T
> > +content-blocking-tracking-protection-new-description = Block all known trackers. (May prevent some pages from loading.)
> 
> Is the period before the parenthetical expected?
> 
> @@ +841,5 @@
> >    .accesskey = A
> >  content-blocking-tracking-protection-option-private =
> >    .label = Only in private windows
> >    .accesskey = p
> > +content-blocking-tracking-protection-change-block-list = Change block list
> 
> Is the capitalization change expected?

These three copy questions are probably directed at Michelle.

Michelle I would appreciate if you can get back to us at some point today since I need to land this patch today in order for it to make it in Firefox 63.  Thanks!

> @@ +858,5 @@
> > +# This is a warning message shown next to a yellow warning icon when the Third-Party Cookies subsection
> > +# of the Content Blocking UI in Preferences has been disabled due to the either the "All cookies" option
> > +# or the "Cookies from unvisited websites" option being selected in the Cookies and Site Data section of
> > +# the UI.
> > +content-blocking-reject-trackers-warning-your-settings-prevent-changes = Your settings in Cookies and Site Data are preventing changes to Third-Party Cookies settings.
> 
> I would avoid adding information that changes (like availability in specific
> versions) to comments.

Sure, I assume this was in response to adding a note about this not having landed in product yet.

Comment 21

[Francesco Lodolo [:flod]]

(In reply to :Ehsan Akhgari from comment #20)
> > I would avoid adding information that changes (like availability in specific
> > versions) to comments.
> 
> Sure, I assume this was in response to adding a note about this not having
> landed in product yet.

Yes. The confusing part is due to Johann's comment showing up in Splinter Review but not here.

Comment 22

[mheubusch]

@Flod - thank you for your careful review!

1. The warning string we use to alert users to an extension controlling TP has to change because we no longer call it Tracking Protection and it's no longer a stand-alone feature.  We went with something generic because the WebExtensions team will expand the API to control a setting or multiple settings. Ehsan is working on the placement of the string so the relationship between the warning and the sub-setting is clear. (Breaking the existing rule was a better solution than adhering to it)

2. The period after the sentence is correct. Phrases that are labels for controls (like check boxes or radio buttons) don't take a period, but descriptive copy does. 

3. The lowercase block list is also correct (and the Learn More above should also be Learn more) following our standard that hyperlinks are sentence case. Ehsan is changing the m in More to correct it)

Comment 23

[(no longer active)]

(In reply to mheubusch from comment #22)
> @Flod - thank you for your careful review!
> 
> 1. The warning string we use to alert users to an extension controlling TP
> has to change because we no longer call it Tracking Protection and it's no
> longer a stand-alone feature.  We went with something generic because the
> WebExtensions team will expand the API to control a setting or multiple
> settings. Ehsan is working on the placement of the string so the
> relationship between the warning and the sub-setting is clear.

In bug 1487218 BTW.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eaa2d0e9b62d
Part 1: Update the Content Blocking section copy based on the latest strings; r=johannh,flod
https://hg.mozilla.org/integration/mozilla-inbound/rev/a55d81761fc4
Part 2: Reorder the Privacy pane of the Preferences UI to move Content Blocking to the top, followed by Cookies & Site Data; r=johannh

Comment 25

[Andreea Pavel [:apavel]]

Backed out for failing browser chrome at browser/extensions/formautofill/test/browser/browser_privacyPreferences.js

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=a55d81761fc404a666b3268450e3e79443d63bbf

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=196942809&repo=mozilla-inbound&lineNumber=2918

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/d974ef1647f3a78e04a7e9d9950b5794bf5e04d6

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e60483ec824
Part 1: Update the Content Blocking section copy based on the latest strings; r=johannh,flod
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1c29bfb5ea9
Part 2: Reorder the Privacy pane of the Preferences UI to move Content Blocking to the top, followed by Cookies & Site Data; r=johannh

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a55e5b74107
follow-up: Fix lint failure

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aeabd71dc6a4
follow-up: Fix lint failure

Comment 29

[Andreea Pavel [:apavel]]

Relanded due to https://hg.mozilla.org/mozilla-central/rev/6c83f735355d being merged into inbound.

Comment 30

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/aeabd71dc6a4

Comment 31

[Francesco Lodolo [:flod]]

Is this bug really in mozilla-central? I only see a "follow-up: Fix lint failure" at this point

Consider for example preferences.ftl
https://hg.mozilla.org/mozilla-central/log/aeabd71dc6a4/browser/locales/en-US/browser/preferences/preferences.ftl

Comment 32

[Cosmin Sabou [:CosminS]]

Francesco, from what I can see only the fix was relanded after the merge that cause these bugs to be backed out https://hg.mozilla.org/mozilla-central/rev/6c83f735355d. Will try to reland https://hg.mozilla.org/integration/mozilla-inbound/rev/8e60483ec824 and https://hg.mozilla.org/integration/mozilla-inbound/rev/d1c29bfb5ea9 so those also get into mozilla central.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/121622de894e
Part 1: Update the Content Blocking section copy based on the latest strings; r=johannh,flod
https://hg.mozilla.org/integration/mozilla-inbound/rev/34d0f9bc490c
Part 2: Reorder the Privacy pane of the Preferences UI to move Content Blocking to the top, followed by Cookies & Site Data; r=johannh

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee6621253805
follow-up: Fix failures caused by partly resolving reland conflicts.

Comment 35

[Eiichi]

The patches seem to break "All Detected Trackers" part.
I'm now reporting Built from https://hg.mozilla.org/integration/mozilla-inbound/rev/0add114333b9f0ada167bcfcda9c6167fe9079be.

After landing the patches "All Detected Trackers" part of "Content Blocking" section is automatically unchecked and I can NOT check it again.

Comment 36

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/121622de894e
https://hg.mozilla.org/mozilla-central/rev/34d0f9bc490c
https://hg.mozilla.org/mozilla-central/rev/ee6621253805

Comment 37

[Cristian Fogel [:cfogel]]

Trying to check the changes on beta(63.0b6), but the section is not updated yet.
Is there any pref that needs to be enabled or will land on another version?

Comment 38

[Johann Hofmann [:johannh]]

(In reply to Cristi Fogel [:cfogel] from comment #37)
> Trying to check the changes on beta(63.0b6), but the section is not updated
> yet.
> Is there any pref that needs to be enabled or will land on another version?

browser.contentblocking.ui.enabled = true

Comment 39

[Cristian Fogel [:cfogel]]

Thank you, verified with 63.0b6 over win10x64, macOS 10.13, Ubuntu 16.04LTS.
Can confirm that the section is updated and moved at the top.

The fact that if accessed from the menu, after changing the pref; the focus is set to the position of the "Tracking protection" and not "Content Blocking" ... is it something expected/known?

Comment 40

[(no longer active)]

(In reply to Cristi Fogel [:cfogel] from comment #39)
> The fact that if accessed from the menu, after changing the pref; the focus
> is set to the position of the "Tracking protection" and not "Content
> Blocking" ... is it something expected/known?

Not 100% sure what you mean but it doesn't sound familiar.  Please file a new bug.  Thanks!

Comment 41

[Johann Hofmann [:johannh]]

Yeah, would be interesting to see a recording or something of that, thanks!

Comment 42

[Cristian Fogel [:cfogel]]

marked it as bug 1493185
Setting proper flags for this issue based on my previous comment.