Closed Bug 2014296 Opened 1 month ago Closed 1 month ago

The `quicksuggest.online.available` pref is recorded incorrectly in telemetry

Categories

(Firefox :: Address Bar, defect, P1)

defect

Tracking

()

VERIFIED FIXED
149 Branch
Tracking Status
firefox147 --- wontfix
firefox148 --- verified
firefox149 --- verified

People

(Reporter: adw, Assigned: adw)

References

Details

(Whiteboard: [sng])

Attachments

(2 files)

Bug 1995362 added the quicksuggest.online.available pref to the group of prefs that TelemetryEvent records in the metrics ping, but it's not working right.

TelemetryEvent isn't a global, it's per browser window. So every time a window is opened, that code runs and records all those prefs again. (Which is a whole other problem, but I'm trying to ignore that for now.) The problem is that, when that happens, the code only looks at the pref value and not the Nimbus variable value. So if there's an experiment installed that sets the variable and then the user opens a new window, quicksuggest.online.available is recorded as false even though the Nimbus variable -- the effective value -- is true.

Nadia discovered the problem in the data when analyzing the new online Suggest M4 experiment.

This fixes the bug and makes a couple of other minor changes:

Remove the newValue param to #recordPref. It's unnecessary complexity and an
unnecessary secondary code path, and it doesn't really save anything perf-wise.

Don't get prefValue when it's not needed. onPrefChanged and
onNimbusChanged are called when any urlbar pref/variable changes, not just
the small number of prefs in #PING_PREFS.

firefox-beta Uplift Approval Request

  • User impact if declined: No user impact, but this affects some telemetry that we use in experiments. Uplifting this fix will allow us to run experiments on 148 instead of waiting for 149.
  • Code covered by automated testing: yes
  • Fix verified in Nightly: no
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: I'll add some STR to the bug
  • Risk associated with taking this patch: low
  • Explanation of risk level: Small patch that only affects telemetry, no user-visible changes. Has an updated test.
  • String changes made/needed: none
  • Is Android affected?: no
Attachment #9542386 - Flags: approval-mozilla-beta?
Flags: qe-verify+

This fixes the bug and makes a couple of other minor changes:

Remove the newValue param to #recordPref. It's unnecessary complexity and an
unnecessary secondary code path, and it doesn't really save anything perf-wise.

Don't get prefValue when it's not needed. onPrefChanged and
onNimbusChanged are called when any urlbar pref/variable changes, not just
the small number of prefs in #PING_PREFS.

Original Revision: https://phabricator.services.mozilla.com/D281692

Flags: in-testsuite+

STR for QA

This bug fixed a problem with how the urlbar.pref_suggest_online_available metric is recorded. Normally this metric's value should be the value of the browser.urlbar.quicksuggest.online.available pref. However, if an experiment is installed that sets the variable quickSuggestOnlineAvailable, then the metric's value should be the value of the variable. (The pref is a fallback pref for the variable.)

Here are all posssible combinations that can be tested, if you're able. "none" means no experiment installed.

Pref  | Variable | Expected metric value
------+----------+----------------------
false | none     | false
true  | none     | true
false | false    | false
false | true     | true
true  | false    | false
true  | true     | true

I created a stage experiment with a control branch that sets the variable to false and a treatment branch that sets it to true: https://stage.experimenter.nonprod.webservices.mozgcp.net/nimbus/adw-quicksuggestonlineavailable-test/summary/

For each row in the table, please run these STR:

  1. Start Firefox
  2. Open about:config and set the value of the pref to the value in the Pref column (it's false by default)
  3. Verify the metric's value is the value in the Pref column (see below for how to get the value)
  4. Open a new Firefox window (doesn't matter when/if you close it)
  5. Verify the metric's value hasn't changed
  6. If the value in the Variable column is false, install the control branch; if it's true, install the treatment branch; if it's "none", don't do anything
  7. Verify the metric's value is now the value in the Expected metric value column
  8. Open a new Firefox window (doesn't matter when/if you close it)
  9. Verify the metric's value hasn't changed
  10. Restart Firefox
  11. Verify the metric's value hasn't changed
  12. Before starting the next row in the table, you can either uninstall the experiment or use a new profile, doesn't matter which but reusing the profile will probably save you time

To get the metric's value, you can either run Glean.urlbar.prefSuggestOnlineAvailable.testGetValue() in the browser console, or you can check the events ping for urlbar.pref_suggest_online_available.

I've done all these STR myself and verified that they work.

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 149 Branch
QA Whiteboard: [qa-triage-done-c149/b148]
QA Contact: cmuntean
Attachment #9542386 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Thank you, Drew, for the steps and the created experiment; it made testing very easy.

I have verified this fix on the latest Nightly 149.0a1 (Build ID: 20260205213633) on Windows 10, macOS 26.2 and Ubuntu 24.0.

  • Verified the combinations from the table and got the expected metric value.

I will also verify the issue on Firefox 148 when the new Beta build is available.

Status: RESOLVED → VERIFIED

I have verified this fix on the latest Beta 148.0b12 (Build ID: 20260206090715) on Windows 10, macOS 26.2 and Ubuntu 24.0.

  • Verified the combinations from the table and got the expected metric value.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: