Closed Bug 1735976 Opened 9 months ago Closed 8 months ago

Update Firefox Suggest preferences: Add a pref for data sharing/connected services and decouple the two existing suggestion prefs

Categories

(Firefox :: Address Bar, task, P1)

task
Points:
5

Tracking

()

VERIFIED FIXED
96 Branch
Iteration:
96.1 - Nov 1 - Nov 14
Tracking Status
firefox94 + verified
firefox95 --- verified
firefox96 --- verified

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(7 files, 1 obsolete file)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
3.99 KB, text/plain
Details
184.71 KB, patch
Details | Diff | Splinter Review
16.02 KB, patch
Details | Diff | Splinter Review

This is a big revision that makes a lot of changes. The core change is adding a
new pref to control category-3 data collection for Firefox Suggest and making
the existing non-sponsored and sponsored prefs independent of each other.

This is only part 1 of 3 I'm planning. Part 2 updates the prefs UI and part 3
adds new telemetry for the new data-collection pref (event telemetry when it's
toggled, similar to the existing prefs, and telemetry environment).

For review, I'd suggest looking at the following parts in order:

  1. BrowserGlue, UrlbarPrefs.jsm, and test_quicksuggest_migrate.js. I added a UI
    migration to BrowserGlue so that prefs in existing profiles are
    updated. There are two things we need to do: decouple the existing
    suggest.quicksuggest prefs and initialize the new data-collection
    pref. (For brevity I'll omit the browser.urlbar prefix to all these pref
    names in this commit message.)

    The suggest.quicksuggest pref now controls non-sponsored suggestions, and
    the suggest.quicksuggest.sponsored now controls sponsored suggestions
    independent of the other pref.

    The new data-collection pref is named
    quicksuggest.category3DataCollection.enabled. That's verbose but I think
    it's important to make it clear what it means to everyone who sees it and to
    be precise about it. This new pref controls both the data sent in telemetry
    pings and whether we can query Merino.

    For the current online rollout on 92.0.1, we don't record the user's choice
    to the opt-in modal, so the initial value of the data-collection pref for
    existing users is debatable. I Slacked with Natalie about it, and we think
    it's OK to initialize it to allow data collection as long as the
    suggest.quicksuggest pref is true. The user may not have opted in to the
    modal in that case, but if not, they enabled suggestions in
    about:preferences, where we show a description under the main Firefox Suggest
    checkbox about data sharing.

    Finally, I added the notion of quick suggest prefs migrations. The motivation
    is related to the fact that we want to uplift this to earlier Firefox
    versions. In m-c, the current BrowserGlue UI version is 120, but in earlier
    versions it may be a smaller value. We don't want to re-migrate the user when
    they update from an earlier version to what is currently on m-c.

    For new profiles (i.e., profiles that are created in versions of Firefox
    where this revision has landed), none of the migration stuff matters.

  2. UrlbarProviderQuickSuggest.jsm and UrlbarProviderQuickSuggest.jsm. The
    changes here are again related to decoupling the suggest.quicksuggest prefs
    and checking the data-collection pref before recording data in telemetry and
    fetching from Merino.

  3. All the other tests. I ended up adding some checks to a lot of them and
    simplifying some, especially browser_urlbar_telemetry_quicksuggest.js.

This updates the Privacy pane for the new Firefox Suggest preferences spec.

We're replacing the current two Firefox Suggest checkboxes with three toggle
buttons. The first two toggle buttons correspond to the existing
browser.urlbar.suggest.quicksuggest and
browser.urlbar.suggest.quicksuggest.sponsored prefs. However, the second pref
is no longer dependent on the first, and it can be toggled regardless of whether
the first is enabled. The third toggle corresponds to a new pref,
browser.urlbar.quicksuggest.category3DataCollection.enabled. It can also be
toggled independently of the others.

In addition, we're adding an info bar/box below the toggles to explain to the
user the effect of their toggle selection. The text in the box depends on the
state of the toggles. The box itself is hidden when all three toggles are off.

There are three possible strings in the info box. To implement it, I have three
html:spans and use JS to hide the two that shouldn't be shown. The other
obvious implementation would be to use a deck, but I don't know how much we
want to rely on XUL these days. Actually I'd like to show/hide the spans using
CSS, but that doesn't seem to be typical in the preferences UI, and there's only
theme CSS AFAICT.

Depends on D128579

This adds telemetry for the new data-collection pref:

  • Adds a data_collect_toggled object to the contextservices.quicksuggest
    telemetry event
  • Adds the new pref to telemetry environment

This is done but I'll wait to request review until the part-1 patch is r+'ed and
we settle on whether we want to rename the browser.urlbar.suggest.quicksuggest
pref.

This is a big revision that makes a lot of changes. The core change is adding a
new pref to control "Connected Services" for Firefox Suggest and making the
existing non-sponsored and sponsored prefs independent of each other. The
services pref defaults to false in every scenario; in online, the user is opted
in to it when they opt in to the modal, and in both online and offline the user
will be able to enable it in the preferences UI (part 2 of this bug, see below).

A new requirement for this bug that came up after I wrote my original patch in
D128579 is that we want the ability to enable services (Merino) via an
experiment. That's tricky because services will be exposed in the preferences
UI. If the user disables services, we don't want to override that. There's a
distinction to draw between services being disabled/enabled by default and
services being disabled/enabled by the user. It's OK to override the default but
not the user's choice, and that's true not only for this new services pref but
for all prefs exposed in the UI.

We can handle that by taking care to set prefs on the default and user branches
as appropriate. If a pref exposed in the UI has a value on the user branch, then
we can assume the user modified it, and ideally that value should stick around
indefinitely. On the other hand we can set default values as configured by
Nimbus on the default branch. If the user modified a pref, then it'll take
precedence since it's on the user branch.

To make user-modified preferences stick around indefinitely, I made them sticky
in firefox.js. That way even when user values are the same as default values,
the user-branch values will be preserved, which is not true of non-sticky prefs.

I added a new Nimbus variable for services, so we can use that to enable
services by default. It was trivial to add similar variables for the other
UI-exposed prefs, the two suggestions types, so I did that too.

This is all tested pretty thoroughly in browser_quicksuggest_configuration.js.

This is only part 1 of 3. Part 2 updates the prefs UI and part 3 adds new
telemetry for the new data-collection pref (event telemetry when it's toggled,
similar to the existing prefs, and telemetry environment).

For review, I'd suggest looking at parts in the following order:

  1. UrlbarPrefs.jsm, browser_quicksuggest_configuration.js, and
    test_quicksuggest_migrate.js.

    The core changes are in UrlbarPrefs.jsm. The two test files document the
    expected behavior of different scenarios/enrollments and prefs migrations.

    browser_quicksuggest_configuration.js tests different enrollments and the
    prefs stuff I mentioned above.

    test_quicksuggest_migrate.js tests prefs migrations. I added a migration step
    that's run on startup when the scenario is updated. There are two things we
    need to do: decouple the existing suggest.quicksuggest prefs and initialize
    the new services pref. (For brevity I'll omit the browser.urlbar prefix to
    all these pref names in this commit message.)

    The suggest.quicksuggest pref now controls non-sponsored suggestions, and
    the suggest.quicksuggest.sponsored now controls sponsored suggestions
    independent of the other pref. The new services pref is
    quicksuggest.services.enabled.

    For the current online rollout on 92.0.1, we don't record the user's choice
    to the opt-in modal, so the initial value of the data-collection pref for
    existing users is debatable. I Slacked with Natalie about it, and we think
    it's OK to initialize it to allow data collection as long as the main
    suggest.quicksuggest pref is true. The user may not have opted in to the
    modal in that case, but if not, they enabled suggestions in
    about:preferences, where we show a description under the main Firefox Suggest
    checkbox about data sharing.

    For new profiles (i.e., profiles that are created in versions of Firefox
    where this revision has landed), the migration step runs but is effectively a
    no-op.

  2. UrlbarProviderQuickSuggest.jsm and UrlbarProviderQuickSuggest.jsm. The
    changes here are again related to decoupling the suggest.quicksuggest prefs
    and checking the services pref before recording data in telemetry and
    fetching from Merino.

  3. All the other tests. I ended up adding some checks to a lot of them and
    simplifying some, especially browser_urlbar_telemetry_quicksuggest.js.

Attachment #9246098 - Attachment is obsolete: true
Attachment #9246254 - Attachment description: Bug 1735976 - Update Firefox Suggest preferences: Part 3 - Add telemetry for the new data-collection pref. → Bug 1735976 - Update Firefox Suggest preferences: Part 3 - Add telemetry for the new services pref.
Blocks: 1737374

A changelog for how the meaning of these preferences has changed is required. Documenting on firefox-source-docs.mozilla.org or another Mozilla documentation platform (not a google doc) is fine. Without this documentation, future data users won't understand how to use the data collected by these probes, so there will be an perpetual maintenance cost of engineering time to explain to data users what these probes mean.

Now that we're likely not targeting a 94 dot release anymore, I'd like to go
ahead and rename the suggest.quicksuggest pref to
suggest.quicksuggest.nonsponsored as we considered doing before. There won't
be a better time to do it.

Depends on D128665

Attached file request.md

Data review for the following:

  • New contextservices.quicksuggest.data_collect_toggled telemetry event
  • Recording the new pref browser.urlbar.quicksuggest.dataCollection.enabled in telemetry environment data
  • The existing browser.urlbar.suggest.quicksuggest pref is currently recorded in telemetry environment data, and in this bug we are changing the name of this pref to browser.urlbar.suggest.quicksuggest.nonsponsored
Attachment #9249849 - Flags: data-review?(cdowhygelund)
Attachment #9246254 - Attachment description: Bug 1735976 - Update Firefox Suggest preferences: Part 3 - Add telemetry for the new services pref. → Bug 1735976 - Update Firefox Suggest preferences: Part 3 - Add telemetry for the new data-collection pref.

request.md

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, it will be published at https://firefox-source-docs.mozilla.org/browser/urlbar/telemetry.html and https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/environment.html.

Is there a control mechanism that allows the user to turn the data collection on and off?

Clients may use the Firefox telemetry opt-out mechanism.

If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, Drew Willcoxon Contexual Services team.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical data
The actual data to be collected is covered under other requests.

Is the data collection request for default-on or default-off?

Default on for all channels.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Yes

Does the data collection use a third-party collection tool?

No

Attachment #9249849 - Flags: data-review?(cdowhygelund) → data-review+
Attachment #9247206 - Attachment description: Bug 1735976 - Update Firefox Suggest preferences: Part 1 - Add a pref for services, decouple the two existing suggestion prefs, and rework pref and Nimbus handling. → Bug 1735976 - Update Firefox Suggest preferences: Part 1 - Add a pref for data collection, decouple the two existing suggestion prefs, and rework pref and Nimbus handling.
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efd0a6f70ff9
Update Firefox Suggest preferences: Part 1 - Add a pref for data collection, decouple the two existing suggestion prefs, and rework pref and Nimbus handling. r=nanj,mak
https://hg.mozilla.org/integration/autoland/rev/d9093f96237d
Update Firefox Suggest preferences: Part 2 - Update the preferences UI. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/436a81dddfd7
Update Firefox Suggest preferences: Part 3 - Add telemetry for the new data-collection pref. r=nanj
https://hg.mozilla.org/integration/autoland/rev/1c739ce689ee
Update Firefox Suggest preferences: Part 4 - Migrate suggest.quicksuggest to suggest.quicksuggest.nonsponsored. r=nanj
Blocks: 1740345

STR for QA: The test plan doc covers everything we need to test.

Flags: qe-verify+
Flags: in-testsuite+

Comment on attachment 9247206 [details]
Bug 1735976 - Update Firefox Suggest preferences: Part 1 - Add a pref for data collection, decouple the two existing suggestion prefs, and rework pref and Nimbus handling.

Beta/Release Uplift Approval Request

  • User impact if declined: We need this for the Firefox Suggest preferences redesign targeting 95/94.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Please see the test plan doc
  • List of other uplifts needed: Please see uplift spreadsheet: https://docs.google.com/spreadsheets/d/1LavihS-VOPFYEyum7mrx6FKXmuQeHi9xQHfGNSxjnoY/edit?usp=sharing
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): These are large patches that affects the Firefox Suggest feature, which is currently enabled by default for all U.S. users. There isn't much if any risk w/r/t affecting other Firefox features. For Firefox Suggest, the risk involves making sure we respect user choices and don't override them with future Firefox Suggest experiments or rollouts. These patches have comprehensive automated tests and we have a thorough test plan for making sure they work correctly and we respect user choice.
  • String changes made/needed:
Attachment #9247206 - Flags: approval-mozilla-beta?
Attachment #9246249 - Flags: approval-mozilla-beta?
Attachment #9246254 - Flags: approval-mozilla-beta?
Attachment #9249654 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]

Comment on attachment 9247206 [details]
Bug 1735976 - Update Firefox Suggest preferences: Part 1 - Add a pref for data collection, decouple the two existing suggestion prefs, and rework pref and Nimbus handling.

Approved for 96.0b6.

Attachment #9247206 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9246249 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9246254 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9249654 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1740818

We have finished verifying these changes on the latest Nightly 96.0a1 build (Build ID: 20211111045525) on Windows 10 x64, macOS 10.15.7 and Linux Mint 20/ Ubuntu 20.0x64.

  • In order to verify this bug we have followed the scenarios: Gdoc.

During testing, we have encountered the following issue on Ubuntu 20.04 on a slower machine: Bug 1740818.
Also in scenario 35, after following the STR, the Firefox Suggest modal is triggered. However, we are waiting for Drew's confirmation if this is intended or if it's a bug.

We have also started testing these scenarios on Beta 95.0b6 and we will add a comment as soon as we finish.

Blocks: 1740965

We have finished verifying these changes on the latest Beta 95.0b7 build (Build ID: 20211114185943) on Windows 10 x64, macOS 11.6 and Linux Mint 20x64.

  • In order to verify this bug we have followed the scenarios: Gdoc.

Besides the known issue (Bug 1740818) we didn't encounter any new ones.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

[Tracking Requested - why for this release]: We need this for the Firefox Suggest preferences redesign targeting 95/94.

This is the part 1 patch for mozilla-release/94. For the patch that landed on m-c and m-b, there's a small conflict in browser/components/urlbar/tests/unit/xpcshell.ini due to bug 1739847.

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: We need this for the Firefox Suggest preferences redesign targeting 95/94.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes, please see the test plan doc
[List of other uplifts needed for the feature/fix]: Please see uplift spreadsheet: https://docs.google.com/spreadsheets/d/1LavihS-VOPFYEyum7mrx6FKXmuQeHi9xQHfGNSxjnoY/edit?usp=sharing
[Is the change risky?]: Medium risk
[Why is the change risky/not risky?]: These are large patches that affects the Firefox Suggest feature, which is currently enabled by default for all U.S. users. There isn't much if any risk w/r/t affecting other Firefox features. For Firefox Suggest, the risk involves making sure we respect user choices and don't override them with future Firefox Suggest experiments or rollouts. These patches have comprehensive automated tests and we have a thorough test plan for making sure they work correctly and we respect user choice.
[String changes made/needed]:

Attachment #9250954 - Flags: approval-mozilla-release?

This is the part 3 patch for mozilla-release/94. For the patch that landed on m-c and m-b, there's a conflict in toolkit/components/telemetry/Events.yaml.

Comment on attachment 9246249 [details]
Bug 1735976 - Update Firefox Suggest preferences: Part 2 - Update the preferences UI.

Beta/Release Uplift Approval Request

  • User impact if declined: We need this for the Firefox Suggest preferences redesign targeting 95/94.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Please see the test plan doc
  • List of other uplifts needed: Please see uplift spreadsheet: https://docs.google.com/spreadsheets/d/1LavihS-VOPFYEyum7mrx6FKXmuQeHi9xQHfGNSxjnoY/edit?usp=sharing
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): These are large patches that affects the Firefox Suggest feature, which is currently enabled by default for all U.S. users. There isn't much if any risk w/r/t affecting other Firefox features. For Firefox Suggest, the risk involves making sure we respect user choices and don't override them with future Firefox Suggest experiments or rollouts. These patches have comprehensive automated tests and we have a thorough test plan for making sure they work correctly and we respect user choice.
  • String changes made/needed:
Attachment #9246249 - Flags: approval-mozilla-release?
Attachment #9249654 - Flags: approval-mozilla-release?
Attachment #9250958 - Flags: approval-mozilla-release?

To be clear on which attachments are for 94: There are 4 patches/revisions total. 2 of the 4 have trivial conflicts on 94. I've attached rebased patches for those 2. The patches for 94 are:

  1. Part 1: attachment 9250954 [details] [diff] [review] (94-specific patch)
  2. Part 2: attachment 9246249 [details] (Phabricator)
  3. Part 3: attachment 9250958 [details] [diff] [review] (94-specific patch)
  4. Part 4: attachment 9249654 [details] (Phabricator)
Blocks: 1741476

We have finished verifying these changes on the Firefox 94.0.2 try build on Windows 10 x64, macOS 10.15.7 and Ubuntu 20.04 x64.

  • In order to verify this bug we have followed the scenarios: Gdoc.
    Besides the known issue (Bug 1740818) we didn't encounter any new ones.

Comment on attachment 9250954 [details] [diff] [review]
94/mozilla-release part 1 patch

Approved for 94.0.2.

Attachment #9250954 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #9246249 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #9250958 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #9249654 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: qe-verify+

Even though we verified all the scenarios using a Firefox 94.0.2 try build, to make sure that everything works we have re-run these scenarios also on Firefox 94.0.2 candidate build (Build ID: 20211117154346) on Windows 10 x6.

  • In order to verify this bug we have followed the scenarios: Gdoc.
Flags: qe-verify+
Depends on: 1745297
You need to log in before you can comment on or make changes to this bug.