Closed Bug 1799264 Opened 1 year ago Closed 1 year ago

Refactor QuickSuggest features into their own classes and files


(Firefox :: Address Bar, task, P1)




108 Branch
Tracking Status
firefox108 --- verified


(Reporter: adw, Assigned: adw)




(3 files)

I'd like to refactor individual features of quick suggest into their own classes and files. That will make it easier to individually toggle them on and off depending on their Nimbus variables and prefs. It's also good for encapsulation.

My motivation for this is that I started adding the new weather feature to QuickSuggest and found that I was duplicating similar patterns I used with impression caps, blocked suggestions, and even remote settings. Each of these features requires some initialization when their Nimbus variables or prefs are enabled. Some also require uninitialization. The weather feature will need both. Any of these features can be enabled or disabled at any time since they depend on either Nimbus experiments or prefs that the user can toggle. It would be nice if there were a simple way to plug them into QuickSuggest and have QuickSuggest take care of enabling and disabling them appropriately.

This adds a new BaseFeature class that quick suggest features can extend.
BaseFeature encapsulates the logic for deciding whether a feature should be
enabled. Subclasses override a few methods and getters to describe their own
logic and to do initialization and uninitialization.

Any time a Nimbus variable (or a variable fallback pref) changes, QuickSuggest
iterates over each feature and enables or disables it appropriately.

As a first step, I ported impression caps to BaseFeature. Later patches in
this stack will port other features.

Please see bug 1799264 for details.

This ports blocked suggestions to BaseFeature.

Please see bug 1799264 for details.

Depends on D161366

This ports the remote settings client to BaseFeature.

The remote settings client is different from the other two features I ported in
this patch stack because it depends on preferences, not Nimbus variables:
suggest.quicksuggest.sponsored and suggest.quicksuggest.nonsponsored. To
support that, I modified BaseFeature so it can list the prefs it depends
on. When QuickSuggest observes a pref change, it checks whether it's a pref for
a feature, and if so, it updates it.

Please see bug 1799264 for details.

Depends on D161367

Pushed by
Refactor QuickSuggest features [Part 1]: Add BaseFeature and port impression caps to it. r=daisuke
Refactor QuickSuggest features [Part 2]: Port blocked suggestions to BaseFeature. r=daisuke
Refactor QuickSuggest features [Part 3]: Port the remote settings client to BaseFeature. r=daisuke
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch

For QA verification: This is another refactor that's a follow up to the previous refactor in bug 1798595. Please test with remote settings and Merino individually (STR below). As time allows please also do some smoke testing to make sure it didn't cause any obvious regressions. The code is still covered by the same automated tests.

Remote settings only

  1. Set browser.urlbar.quicksuggest.remoteSettings.enabled to true (should be true by default)
  2. Set browser.urlbar.quicksuggest.dataCollection.enabled to false (should be false by default)
  3. Try triggering suggestions like "amazon", "betty", etc.
  4. Restart Firefox and repeat the previous step

Merino only

  1. Set browser.urlbar.quicksuggest.remoteSettings.enabled to false
  2. Set browser.urlbar.quicksuggest.dataCollection.enabled to true
  3. Try triggering suggestions like "amazon", "betty", etc.
  4. Restart Firefox and repeat the previous step
Flags: qe-verify+
Flags: in-testsuite+

We have performed a Smoke test on the latest Nightly 108.0a1 (Build ID: 20221110092309) on Windows 10 x64, macOS 12.6 and Ubuntu 20.04.
You can find the Test Runs here: Test Rail.

  • We haven't found any issues during testing.
Flags: qe-verify+
Blocks: 1815018
You need to log in before you can comment on or make changes to this bug.