Add a Nimbus variable and pref for configuring Yelp suggestion index/position
Categories
(Firefox :: Address Bar, task, P1)
Tracking
()
People
(Reporter: adw, Assigned: adw)
References
Details
(Whiteboard: [sng])
Attachments
(1 file)
48 bytes,
text/x-phabricator-request
|
diannaS
:
approval-mozilla-beta+
|
Details | Review |
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 howquickSuggestScoreMap
maps telemetry types to desired scores. We can keepquickSuggestSponsoredIndex
andquickSuggestNonSponsoredIndex
(and their fallback prefs) for now as fallbacks for when a particular type isn't defined inquickSuggestIndexMap
. 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 abrowser.urlbar.yelp.index
pref.
Updated•9 months ago
|
Assignee | ||
Comment 1•9 months ago
|
||
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 | ||
Updated•9 months ago
|
Assignee | ||
Comment 2•9 months ago
|
||
Comment 4•9 months ago
|
||
bugherder |
Comment 5•9 months ago
|
||
:adw, was reading the QA report, does this need to be uplifted to 124?
Assignee | ||
Comment 6•9 months ago
|
||
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.
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Comment 7•9 months ago
|
||
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.)
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Comment 8•9 months ago
|
||
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
Comment 9•9 months ago
|
||
Comment on attachment 9382752 [details]
Bug 1881606 - Add a Nimbus variable and pref for configuring Yelp suggestion index/position.
Approved for 124.0b6
Comment 10•9 months ago
|
||
uplift |
Updated•9 months ago
|
Updated•9 months ago
|
Comment 11•9 months ago
|
||
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”);
Comment 12•9 months ago
|
||
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”)
Description
•