Closed Bug 1881606 Opened 9 months ago Closed 9 months ago

Add a Nimbus variable and pref for configuring Yelp suggestion index/position

Categories

(Firefox :: Address Bar, task, P1)

task

Tracking

()

VERIFIED FIXED
125 Branch
Tracking Status
firefox124 --- verified
firefox125 --- verified

People

(Reporter: adw, Assigned: adw)

References

Details

(Whiteboard: [sng])

Attachments

(1 file)

Per the Yelp demo meeting today, we likely want to show non-top-pick Yelp suggestions at the top of the Suggest section, not the bottom where Suggest results are usually shown. We also want to continue showing sponsored AMP results at the usual bottom position, which means we can't use the existing quickSuggestSponsoredIndex for this. We'll need to add a new variable/pref for Yelp.

I'd like to do the following:

  • Add a quickSuggestIndexMap JSON variable that maps Suggest telemetry type names to desired indexes, similar to how quickSuggestScoreMap maps telemetry types to desired scores. We can keep quickSuggestSponsoredIndex and quickSuggestNonSponsoredIndex (and their fallback prefs) for now as fallbacks for when a particular type isn't defined in quickSuggestIndexMap. Maybe at some point we can remove them.
  • Add some kind of a pref so we can easily test/demo different Yelp positions during development, and for setting a final desired index if/when Yelp is enabled by default. I don't know if adding a fallback pref for quickSuggestIndexMap is right because we'd need to store JSON as the value. That's doable and it would probably make the implementation easier, but it would make testing/demoing a little harder. The alternative is to add a browser.urlbar.yelp.index pref.

We also have quickSuggestAllowPositionInSuggestions. There are a couple of drawbacks to using that here:

  • Now that we're using Rust, any change to the suggestion schema -- adding a position property in this caase -- requires a Rust change and then a re-vendor. It's a pain, and we need this for 124 so I'd like to avoid further Rust changes now.
  • It would be nice to be able to set the position itself in the Nimbus recipe. We can change it at the last minute, ensure it remains the same throughout the experiment, and even have more than one treatment branch with different positions if that becomes a requirement.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ef1260d1935d Add a Nimbus variable and pref for configuring Yelp suggestion index/position. r=daisuke
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 125 Branch

:adw, was reading the QA report, does this need to be uplifted to 124?

Flags: needinfo?(adw)

Yes, thanks for checking! All bugs blocking the Yelp meta bug 1875963 need to be uplifted. I'll request uplift today on bugs that have landed on m-c overnight.

Flags: needinfo?(adw)

Info for QA

This bug adds:

  • yelpSuggestNonPriorityIndex - Nimbus variable for setting the index of non-top-pick Yelp suggestions inside the Firefox Suggest section. 0 means the first index (top of the section), 1 means the second index, etc. -1 means the last index (bottom of the section).
  • browser.urlbar.yelp.suggestedIndex - Fallback pref for the Nimbus variable.

Both of these default to 0, so by default non-top-pick Yelp suggestions will now appear in the first index of the Firefox Suggest section, i.e., at the top of the section. This is a change from previous behavior and is different from all other non-top-pick Suggest suggestions, which appear at the bottom. We still support top pick Yelp suggestions using the yelpSuggestPriority variable (and its fallback pref browser.urlbar.yelp.priority).

For the experiment, we will now have at least one treatment branch that shows Yelp at the top of the section. Since yelpSuggestNonPriorityIndex defaults to showing Yelp at the top, that means this variable does not need to be specified in the Nimbus recipe for this treatment branch. It's possible we'll have a second treatment branch that shows Yelp at the bottom of the Firefox Suggest section. (In that case yelpSuggestNonPriorityIndex will need to be set to -1.)

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

Comment on attachment 9382752 [details]
Bug 1881606 - Add a Nimbus variable and pref for configuring Yelp suggestion index/position.

Beta/Release Uplift Approval Request

  • User impact if declined: This is necessary for the Yelp suggestions experiment that will target 124.
  • 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 previous comment
  • List of other uplifts needed: The following Yelp bugs need uplift in this order: bug 1881071, bug 1881606 (this bug), bug 1880667, bug 1882174, bug 1880862
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Only affects Yelp suggestions, which are disabled by default. And it only affects the position of Yelp suggestions inside the urlbar panel. Has tests.
  • String changes made/needed:
  • Is Android affected?: No
Attachment #9382752 - Flags: approval-mozilla-beta?

Comment on attachment 9382752 [details]
Bug 1881606 - Add a Nimbus variable and pref for configuring Yelp suggestion index/position.

Approved for 124.0b6

Attachment #9382752 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

I have verified this issue on the latest Firefox Nightly 125.0a1 (Build ID: 20240229093832) on Windows 10 x64, Ubuntu 20.04 x64, and macOS 12.6.1.
The Yelp suggestion can be placed as:

  • the first result in the Firefox Suggest section (pref value “0”);
  • the second result in the Firefox Suggest section (pref value “1”);
  • the last result in the Firefox Suggest section (pref value “-1”);

I have verified this issue on Firefox Beta 124.0b6 (Build ID: 20240301091852) on Windows 10 x64, Ubuntu 20.04 x64, and macOS 12.6.1.
The Yelp suggestion can be placed as:

  • the first result in the Firefox Suggest section (pref value “0”);
  • the second result in the Firefox Suggest section (pref value “1”);
  • the last result in the Firefox Suggest section (pref value “-1”)
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: