Closed Bug 1674469 Opened 4 years ago Closed 4 years ago

Introduce new pref names for an easier rollout

Categories

(Firefox :: Address Bar, enhancement, P1)

enhancement
Points:
3

Tracking

()

VERIFIED FIXED
84 Branch
Iteration:
84.2 - Nov 2 - Nov 15
Tracking Status
firefox83 + verified
firefox84 --- verified

People

(Reporter: mikedeboer, Assigned: bugzilla)

References

Details

Attachments

(1 file)

[Tracking Requested - why for this release]:

The following prefs are currently used the control the enabled/ disabled state of QBv2:

browser.urlbar.update2
browser.urlbar.update2.oneOffsRefresh
browser.urlbar.update2.localOneOffs
browser.urlbar.update2.tabToComplete

In this bug, the end-result should be two-fold:

  1. Ensure that there's only one "master" pref that can control all these features.
  2. Introduce an alias to browser.urlbar.update2 that we can use to switch early on the current release, Firefox 82.

The reason is that we'd like to perform a hold-back study and for that we'll need to make sure that the cohorts are enrolled prior to upgrade to Firefox 83. This will have the practical effect that users will have the prefs set appropriately before the upgrade and the control branch will never see the new feature - as intended!

This will need to get uplifted to Fx83 to have an effect.

browser.urlbar.update2 is a global disabling toggle, the other prefs can be set before it, and should have no effect until browser.urlbar.update2 is flipped, if they do have an effect that'd be a bug, and fixing that bug is not more work than introducing another global disabling toggle.
Introducing a global enabling toggle sounds line prone to introduce logic bugs.
It sounds like introducing a pref that is only effective from Firefox 83 on would give all that we need.

But why can't we just flip browser.urlbar.update2 to false for the holdback group?

Flags: needinfo?(mdeboer)

Just flipping browser.urlbar.update2 should work. In our meeting about the holdback experiment, I just suggested we flip those other three prefs to be safe. We could turn this bug into an audit of whether disabling browser.urlbar.update2 disables all update2 features.

Assignee: nobody → htwyford
Status: NEW → ASSIGNED
Iteration: --- → 84.2 - Nov 2 - Nov 15

We only read browser.urlbar.update2.oneOffsRefresh, browser.urlbar.update2.localOneOffs, and browser.urlbar.update2.tabToComplete once each in-tree. In all three spots, we also check browser.urlbar.update2:

update2.tabToComplete
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/browser/components/urlbar/UrlbarProviderTabToSearch.jsm#135-136

update2.oneOffsRefresh
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/browser/components/urlbar/UrlbarView.jsm#106

update2.localOneOffs
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/browser/components/urlbar/UrlbarSearchOneOffs.jsm#345

I also did some manual testing on Beta and none of the update2 features work when browser.urlbar.update2 is disabled, even if the other browser.urlbar.update2.* prefs are enabled. We should be safe to just disable browser.urlbar.update2 for the treatment branch in the holdback study. Both goals in comment 0 are accomplished: browser.urlbar.update2 is a "master" pref and it is available to be turned off in 82.

This default-true pref has no effect when true and overrides update2 when false. This will allow us to set it to true for our Control branch so they will just fall back to the default-true update2 pref. We can set it to false for the Treatment branch, who will not get update2 features regardless of the value of urlbar.update2.

We want to uplift this patch to 83. That way, we can set browser.urlbar.experiment.update2 in Release 82, but it will have no effect either way until 83 merges to Release.

Pushed by mdeboer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/01138b19cd83 Introduce experimental pref that can override urlbar.update2 for a holdback study. r=mak,mikedeboer
Flags: needinfo?(mdeboer)

Comment on attachment 9185612 [details]
Bug 1674469 - Introduce experimental pref that can override urlbar.update2 for a holdback study. r?mak!

Beta/Release Uplift Approval Request

  • User impact if declined: We will be unable to run a holdback study on the update2 feature launching in 83. Update2 represents a significant change to address bar code and Product would like to measure its effectiveness with the study.
  • 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: 1. Add the Boolean pref browser.urlbar.experiment.update2 in about:config. Set it to false.
  1. Verify that browser.urlbar.update2 is set to true.
  2. Type something in the address bar.
  3. Click one of the engine one-off buttons at the bottom of the address bar dropdown.

If this patch works, a search will be executed immediately and you will navigate to the results page of the search engine button you clicked. If this patch does not work, the address bar will enter Search Mode (a blue indicator will be shown in the address bar)

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This adds an alias to an existing pref. There are no user-facing behavioural changes. There are tests.
  • String changes made/needed:
Attachment #9185612 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
QA Whiteboard: [qa-triaged]

Comment on attachment 9185612 [details]
Bug 1674469 - Introduce experimental pref that can override urlbar.update2 for a holdback study. r?mak!

Approved for our last beta.

Attachment #9185612 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Following up the sync meeting with :mikedeboer , :cmuntean will cover the uplift verification for this issue.

Flags: needinfo?(cmuntean)

I have verified this issue on the latest Nightly 84.0a1 build (Build ID: 20201105045247) on Windows 10 x64, macOS 10.15.6 and Linux Mint 20.
In order to verify this issue I have used the following scenarios:

  1. In the latest Nightly 84.0a1 build added the pref "browser.urlbar.experiment.update2" to "false" and restarted the browser
  • Verified that the browser.urlbar.update2 prefs remains set to "true".
  • Verified that the Quantumbar v2 functionalities are disabled.
  1. In the latest Nightly 84.0a1 build added the pref "browser.urlbar.experiment.update2" to "true" and restarted the browser
  • Verified that the browser.urlbar.update2 prefs remains set to "true".
  • Verified that the Quantumbar v2 functionalities are available in the browser.
  1. Created a Normandy stage recipe that adds the pref in the targeted profiles. Enrolled in control branch ("browser.urlbar.experiment.update2" to "false") on Firefox 82.0.2 release and opened the same profile in Nightly 84.0a1.
  • Verified that the experiment remains enabled.
  • Verified that the prefs remains set correctly:
    browser.urlbar.experiment.update2 = false
    browser.urlbar.update2 = true
  • Verified that the Quantumbar v2 functionalities are disabled.
  1. Created a Normandy stage recipe that adds the pref in the targeted profiles. Enrolled in treatment branch ("browser.urlbar.experiment.update2" to "true") on Firefox 82.0.2 release and opened the same profile in Nightly 84.0a1.
  • Verified that the experiment remains enabled.
  • Verified that the prefs remains set correctly:
    browser.urlbar.experiment.update2 = true
    browser.urlbar.update2 = true
  • Verified that the Quantumbar v2 functionalities are available in the browser.

I will also verify the issue on beta when the next Firefox Beta 83.0b9 is available.

I have verified this on the latest Beta 83.0b9 (Build ID: 20201105203649) on Windows 10 x64, macOS 10.15.6 and Ubuntu 20.04.

In order to verify this issue, I have used the same scenarios mentioned in comment 11.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(cmuntean)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: