1841408 - Unify rich suggestions, best match, Firefox Suggest sponsored results, and Pocket Suggestions

Status: VERIFIED FIXED

(Firefox :: Address Bar, task, P1)

Description

[Drew Willcoxon :adw]

See revision summary in https://phabricator.services.mozilla.com/D182537.

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1841408 - Unify rich suggestions, best match, Firefox Suggest sponsored results, and Pocket Suggestions.

This does a few things:

• Unify the view implementations of rich suggestions, Firefox Suggest sponsored
  results, and best match. I did this by using the best match implementation
  and extending it to rich suggestions and Suggest sponsored.
• Use the unified implementation for Pocket suggestions too.
• Add a bottom-text concept since Pocket suggestions shown as top picks need to
  show both a description and some text below it.

I have a couple motivations for these changes:

• I'm implementing Pocket suggestions, which need to show some text below the
  suggestion title as well as the URL. I was going to just use the Firefox
  Suggest sponsored approach, where the action text is wrapped below the title,
  but that doesn't work because it can't show both the wrapped action text and
  the URL.
• IMO we should use rich suggestions as the basis for all rows going forward,
  i.e., unify the different row implementations around rich suggestions.

The reason I chose the best match implementation instead of the rich suggestions
implementation is because the grid-based approach of rich suggestions doesn't
work well when the URL also needs to be shown. The URL should be
baseline-aligned with the row title, which isn't easy to do when the URL is
outside the grid. The rich suggestions implementation also doesn't wrap the URL.

Other details:

• The rich-suggestion=no-icon attribute value is only used for styling, so we
  can replace it with @supports -moz-bool-pref(). That lets us make the
  rich-suggestion attribute a simple boolean.
• I kept the isBestMatch property for results since
  searchEngagementTelemetryGroup() uses it to return "top_pick". It still has
  semantic meaning so I think that's OK. It's no longer used by the view to
  create different DOM or styling.
• Move isRichSuggestion from the payload to the result itself, since it's no
  longer used for only one type of result. It's like isBestMatch, which is
  also on the result.
• Add richSuggestionIconSize to the result too. The best match icon size
  is 52. The Pocket best match icon size is 24 (but will have added padding and
  a background color to make it appear 52px).
• Not directly related, but I'm adding a new l10n payload property, and I
  noticed the schema of l10n properties is wrong. The args property should be
  an object with arbitrary properties, not an array. No idea why this is wrong,
  but apparently we don't currently use args at all. (We will with Pocket
  results.) I added a new additionalProperties property. I chose that name
  because it's what the actual JSON schema standard uses.

Depends on D182580

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5387b06281e0
Unify rich suggestions, best match, Firefox Suggest sponsored results, and Pocket Suggestions. r=dao,daleharvey

Comment 5

[Drew Willcoxon :adw]

STR for QA

This bug modified the implementation of rich suggestions and best match (top pick), so please smoke test these features as time allows.

Rich suggestions: Set browser.urlbar.richSuggestions.featureGate = true. (It's true by default on Nightly. This pref now requires a restart after you change it.) Type some search strings to trigger Google rich suggestions, like: "kate", "betty", etc. You may need to VPN into the U.S., I'm not sure

Best match: You can try testing the navigational suggestions feature (requires Firefox Suggest), which uses the best match UI treatment and implementation. Set these prefs:

browser.urlbar.bestMatch.enabled = true
browser.urlbar.quicksuggest.enabled = true
browser.urlbar.suggest.quicksuggest.nonsponsored = true
browser.urlbar.quicksuggest.dataCollection.enabled = true
browser.urlbar.merino.providers = "top_picks"

Type some navigational suggestions keywords to trigger them: "reddit", "rei", "amazon", etc.

Comment 6

[Drew Willcoxon :adw]

Comment on attachment 9342039 [details]
Bug 1841408 - Unify rich suggestions, best match, Firefox Suggest sponsored results, and Pocket Suggestions.

Beta/Release Uplift Approval Request

• User impact if declined: This bug is necessary for the Pocket suggestions feature we intend to ship as an experiment in 116.
• 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 STR in previous comment
• List of other uplifts needed: These bugs must be uplifted first: 1841354, Bug 1841295, Bug 1841553
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch only affects rich suggestions, top picks, and Firefox Suggest sponsored suggestions. Rich suggestions and top picks are disabled by default. Firefox Suggest sponsored suggestions are enabled by default for U.S. users only. In all cases, there are no expected user-visible changes. Has test coverage, and I verified it manually.
• String changes made/needed:
• Is Android affected?: No

Comment 7

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/5387b06281e0

Comment 8

[Dianna Smith [:diannaS]]

Comment on attachment 9342039 [details]
Bug 1841408 - Unify rich suggestions, best match, Firefox Suggest sponsored results, and Pocket Suggestions.

Approved for 116.0b2

Comment 9

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/59aa76b8e6b1

Comment 10

[Carmen Fat [:cfat] - Ecosystem QA]

We verified this issue on the latest Firefox Nightly 117.0a1 (Build ID: 20230707092800) and Beta 116.0b2 (Build ID: 20230706193912), on Windows 10 x64, macOS 13.4.1, and Ubuntu 20.04 x64.

• We performed smoke testing on the Rich Suggestions, Best Match, and Firefox Suggest suggestions and we didn’t notice any issues with the results. The smoke runs can be found in TestRail.