1370801 - Add Shield Opt-out Preference

Status: RESOLVED INVALID

(Firefox :: Settings UI, enhancement)

Description

[Osmose [:osmose, :mkelly]]

Attachment: Data Choices Preferences UI

To allow users to opt-out of Shield studies (bug 1370799), we need to add a new preference to the Preferences page.

Mossop said pref names don't particularly matter, so we're going with app.shield.optoutstudies.enabled. It should be a boolean pref that defaults to true (so users, by default, are opted-in to Shield studies).

Matt Grimes has talked with legal about their requirements for this preference, and based on those discussions the checkbox should appear in the Advanced -> Data Choices section of the preferences, nested underneath the "Enable Firefox Health Report" checkbox like the "Share additional data" checkbox. It should also behave similarly, in that unchecking the FHR checkbox should disable the Shield preference checkbox, and Shield should treat it as disabled.

In Normandy we're tracking this in a Github issue: https://github.com/mozilla/normandy/issues/741

Comment 1

[Osmose [:osmose, :mkelly]]

Cindy: Mossop/dolske mentioned that you're the person to ask about adding a preference to the new, in-progress preferences UI. In the new UI, the Data Choices section is relabeled as "Reports", which doesn't quite fit the purpose of this preference, which is to control whether users will have Shield study add-ons installed automatically as part of opt-out studies[1].

Where do you think we should put the preference in the new preferences UI?

[1] Feel free to ping me or email if you need more info on what opt-out Shield studies are. In short, they're add-ons that we automatically install for a small percentage of the user population to test out new features that will eventually land in Firefox.

Comment 2

[Osmose [:osmose, :mkelly]]

Oh yeah, and the copy for the preference that legal provided:

"Help make Firefox better! Let us study how you use new features and services. Learn more", with Learn more linking to the Firefox privacy notice.

Comment 3

[Matt Grimes [:Matt_G]]

+ michelle since she worked on the original copy with us and has been a part of the preference panel redesign. Where should this pref now live?

Comment 4

[chsiang]

Hi Michael,
I'm not sure which version of the UI you saw, but here is the latest Preferences specs from Tina and it looks to me that this can fit in under Data Choices. And I don't see a choice of "Enable Firefox Health Report" under Data Choices. 
https://mozilla.invisionapp.com/d/main#/console/9996617/233886993/preview
I will discuss with Tina tomorrow as she's on leave today and get back to you.

Curious about execution details- say a user unchecks the box after a period of time, what happens to the add-on?
More importantly, when do you plan to release this feature?

Comment 5

[chsiang]

sorry this link
https://mozilla.invisionapp.com/share/5RA0R4HAE#/233886993_Privacy_-_Security-ReorgV2

Comment 6

[chsiang]

ni micheal for my questions recapped below-

Curious about execution details- say a user unchecks the box after a period of time, what happens to the add-on?
More importantly, when do you plan to release this feature?

Comment 7

[Tina Hsieh[:Tina_Hsieh] UX]

Hi Michael and Cindy,
I think the best place for the shield study option is the "Firefox Data Collection and Use", which is previously named as "Data Choices" or "Reports". We can have the shield study option nested under the first option "Allow Firefox to automatically send technical...to Mozilla". However, I have some questions about it:

1. For the tone of the copy, I'm not sure if the copy that legal provided is aligned with the other copy strings. Michelle, could you please review the copy: "Help make Firefox better! Let us study how you use new features and services. Learn more"?

2. We currently have a Privacy Notice link next to the intro of Firefox Data Collection. Perhaps we can link to a page which introduces how shield study works instead of a duplicated Privacy Notice link?

Comment 8

[Matt Grimes [:Matt_G]]

@Tina 

1. The existing copy was provided by Michelle. She's been working on this project with us for quite a while. 

2. Legal has requested that we link to the privacy notice so that it aligns with the other options. There will be a special section of the privacy policy that covers these studies. There will also be an about:studies (or something similar) page that users can go to. It will show them if they are currently enrolled in any studies and display any studies that have ended. There will be a link on this page that points to a SUMO article that outlines the entire program. 


@chsiang

If a users unchecks the box they will automatically be removed from any currently running studies and any future studies. We are *hoping* to land this feature somewhere in the 55/56 release timeframe. That would be the point where we actually start enrolling users in these studies. Since the work requires both user visible changes and infrastructure changes in Shield, osmose is trying to land the user visible changes separately. There would be maybe 1 release cycle in between.

@osmose - feel free to correct anything above if I am off the mark

Comment 9

[Osmose [:osmose, :mkelly]]

Matt_G covered all the info I would have shared.

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

The new Preferences page will get updated at bug 1365133 and to be shipped on Fx56. If you need the checkbox present on Fx56, Tina/Michelle should update the spec for you to patch the Preferences-to-ship in this bug (or, if it's easy enough and the spec is already ready, Evan could do it in bug 1365133 altogether -- I have no opinion).

Just to be on the safe side, if you do not wish to create a project dependency, you may patch the old Preferences page on your own in this bug.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(I assume this can't reach Fx55 on the old Preferences page given string freeze is days away...)

Comment 12

[Osmose [:osmose, :mkelly]]

Attachment: Bug 1370801: Add preference for controlling Opt-out Shield study enrollment.

Includes a checkbox in the old about:preferences UI for controlling the
preference.

Review commit: https://reviewboard.mozilla.org/r/148240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/148240/

Comment 13

[Osmose [:osmose, :mkelly]]

Attachment: Updated Data Choices with Shield Pref

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #10)
> The new Preferences page will get updated at bug 1365133 and to be shipped
> on Fx56. If you need the checkbox present on Fx56, Tina/Michelle should
> update the spec for you to patch the Preferences-to-ship in this bug (or, if
> it's easy enough and the spec is already ready, Evan could do it in bug
> 1365133 altogether -- I have no opinion).

I'm fine with any of these plans. I'll wait until Tina/Michelle to confirm that they're good with the answers in Comment 8 and then work on that, potentially in a followup bug.

> Just to be on the safe side, if you do not wish to create a project
> dependency, you may patch the old Preferences page on your own in this bug.

I've put up a patch for updating the old preferences page in this bug. Once Michelle can confirm that I used her copy correctly, I'll ask for a review.

Michelle: I was unsure how to split the text you provided up on the page, and split it based on preferences. Does the attached screenshot look correct to you?

Comment 14

[Osmose [:osmose, :mkelly]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #11)
> (I assume this can't reach Fx55 on the old Preferences page given string
> freeze is days away...)

flod: In terms of l10n, how feasible would it be to uplift this patch (with 2 new strings, 1 new accesskey, and 1 copy of another string) to Beta sometime later this week? The rationale here is that we are aiming to ship Shield opt-out studies to 55 via a system add-on update, and need this checkbox in place in 55 before we ship. 

Matt_G has also mentioned he's willing to ship to en-US only first, if that's an option. The studies that we run in 55 will probably be limited to en-US only anyway, so users in other locales won't have a use for this preference anyway. I dunno if that's a thing, but if that is a viable solution, I'm happy to update the patch (if anyone can tell me how to, that is).

Comment 15

[Osmose [:osmose, :mkelly]]

Comment on attachment 8876914 [details]
Bug 1370801: Add preference for controlling Opt-out Shield study enrollment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/148240/diff/1-2/

Comment 16

[Francesco Lodolo [:flod]]

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #14)
> flod: In terms of l10n, how feasible would it be to uplift this patch (with
> 2 new strings, 1 new accesskey, and 1 copy of another string) to Beta
> sometime later this week? The rationale here is that we are aiming to ship
> Shield opt-out studies to 55 via a system add-on update, and need this
> checkbox in place in 55 before we ship. 

I would really prefer to avoid landing stuff in Beta for 55, especially if this is targeting only en-US.

We moved all localizations to Nightly last cycle, and localizers are already struggling enough without adding more churn.

If the target is en-US only on 55, strings should be hard-coded, i.e. the patch attached here is good for mozilla-central (assuming 56 needs all locales) but not for uplift.

Comment 17

[Osmose [:osmose, :mkelly]]

Comment on attachment 8876914 [details]
Bug 1370801: Add preference for controlling Opt-out Shield study enrollment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/148240/diff/2-3/

Comment 18

[Osmose [:osmose, :mkelly]]

I've put up a patch with hard-coded English strings for the old preferences that are hidden outside of en-* locales. Once this lands, I'll request uplift to 55.

I'm also going to file a followup bug to make the strings localizable that we can land after uplift. That should accomplish the goal of making the pref en-* for 55 only.

Comment 19

[Osmose [:osmose, :mkelly]]

Oh, I also forgot to mention, I confirmed over Slack with Michelle that the strings in the screenshot were correct, except she requested I remove the punctuation at the end.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

It's pretty confusing to me why "Help make Nightly better!" and "Share additional data" would be two different checkmarks. The first one is quite broad, and the text descriptions between the two are vague enough that I don't see why a user would choose one but not the other.

Why can't this be controlled by the same checkbox? Doesn't "share additional data" also aid in helping make Nightly better?

Comment 21

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8876914 [details]
Bug 1370801: Add preference for controlling Opt-out Shield study enrollment.

https://reviewboard.mozilla.org/r/148240/#review155420

We shouldn't land any patches to mozilla-central that don't update both sets of preferences. Since this patch is targeting 55 then it should only be landed on Beta55.

::: browser/components/preferences/in-content/advanced.js:327
(Diff revision 3)
> +   * In all cases, set up the Learn More link sanely.
> +   */
> +  initOptOutShieldStudies() {
> +    // Hidden outside of en-* locales temporarily (bug 1370801)
> +    const appLocale = Services.locale.getAppLocaleAsLangTag();
> +    if (AppConstants.MOZ_TELEMETRY_REPORTING && appLocale.startsWith("en")) {

This will also be true for en-GB and en-ZA. Is that intentional?

::: browser/components/preferences/in-content/advanced.xul:225
(Diff revision 3)
> +                          label="Help make &brandShortName; better"
> +                          accesskey="s"/>

Please choose an accesskey that will appear in the label.

::: browser/components/preferences/in-content/tests/browser.ini:34
(Diff revision 3)
> +# Skip this test when FHR/Telemetry aren't available, or when not building desktop Firefox.
> +skip-if = !healthreport || !telemetry || !(buildapp == 'browser')

Please remove the !buildapp == 'browser'. All tests under /browser/ are only built when building desktop Firefox.

::: browser/components/preferences/in-content/tests/browser_optoutshieldstudies.js:20
(Diff revision 3)
> +function test() {
> +  waitForExplicitFinish();

All new tests should be using the add_task() method of writing the tests. See /browser/components/preferences/in-content/tests/browser_searchsuggestions.js for a good example.

::: browser/components/preferences/in-content/tests/browser_optoutshieldstudies.js:49
(Diff revision 3)
> +function resetPreferences() {
> +  Services.prefs.clearUserPref(OPTOUT_STUDIES_ENABLED);
> +  Services.prefs.clearUserPref(FHR_UPLOAD_ENABLED);

These should be using pushPrefEnv.

Comment 22

[Osmose [:osmose, :mkelly]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #20)
> It's pretty confusing to me why "Help make Nightly better!" and "Share
> additional data" would be two different checkmarks. The first one is quite
> broad, and the text descriptions between the two are vague enough that I
> don't see why a user would choose one but not the other.
> 
> Why can't this be controlled by the same checkbox? Doesn't "share additional
> data" also aid in helping make Nightly better?

Functionally, they're pretty different. The share additional data checkbox controls whether Firefox sends more info, whereas this new checkbox controls whether you allow us to install low-risk add-ons that modify Firefox behavior without prompting you. 

Michelle can speak to the wording better than I, but I know that we've found that calling them experiments scares users, because the word implies that we're shipping untested, half-baked features, and studies is a bit too generic of a word from a user perspective.  

I'm assuming there's also a legal difference based on Matt_G's meetings with the legal team, but I haven't explicitly confirmed that.

Comment 23

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #22)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #20)
> > It's pretty confusing to me why "Help make Nightly better!" and "Share
> > additional data" would be two different checkmarks. The first one is quite
> > broad, and the text descriptions between the two are vague enough that I
> > don't see why a user would choose one but not the other.
> > 
> > Why can't this be controlled by the same checkbox? Doesn't "share additional
> > data" also aid in helping make Nightly better?
> 
> Functionally, they're pretty different. The share additional data checkbox
> controls whether Firefox sends more info, whereas this new checkbox controls
> whether you allow us to install low-risk add-ons that modify Firefox
> behavior without prompting you. 
> 
> Michelle can speak to the wording better than I, but I know that we've found
> that calling them experiments scares users, because the word implies that
> we're shipping untested, half-baked features, and studies is a bit too
> generic of a word from a user perspective.  
> 
> I'm assuming there's also a legal difference based on Matt_G's meetings with
> the legal team, but I haven't explicitly confirmed that.

The URL that is used by the Learn More link is a generic link to the privacy page. The "share additional data" one links to a telemetry subsection of the page. I don't see how a user will be able to understand the implementation difference here.

My questions is why do we need to add another checkbox in the first place. Why isn't the first checkbox enough?

Comment 24

[Osmose [:osmose, :mkelly]]

Matt checked with legal and with the folks working on the prefs reorg, and they've decided to reuse the FHR pref like jaws suggested. They've also approved us using the existing wording in the old preferences organization, so there's nothing more to be done here. Hooray, thanks!