This removes "best match" as its own separate Firefox Suggest feature. In the
future, whether or not a suggestion is a best match (a.k.a. top pick) will be
determined by relevant product requirements. I've confirmed this with Nive.
Here's a summary of changes:
* Removes prefs and Nimbus variables related to best match
* Removes the "Top pick" checkbox in about:preferences
* Removes support for the `best_match` quick suggest config property. This
property was removed from the config in remote settings a while ago.
* Removes legacy telemetry scalars related to best match. These scalars were
added years ago for the original best match experiment and before we started
using Glean. In the case of non-sponsored suggestions, the scalars have not
been recorded at all for some time. In the case of sponsored suggestions, they
can now be recorded again due to the recent addition of sponsored priority
suggestions, but they are superseded by Glean.
Differential Revision: https://phabricator.services.mozilla.com/D190516
This uses the update timer manager (`nsIUpdateTimerManager`) to periodically
ingest. This is the same mechanism the desktop remote settings client uses.
Please see the bug for more info on the update timer manager. I left some notes
there.
I chose the same 24-hour interval that the remote settings client uses. So
overall, this should give us pretty much the same update behavior we have with
remote settings in the JS backend.
Differential Revision: https://phabricator.services.mozilla.com/D190384
This converts addon suggestions from dynamic to standard results and removes
support for review stars. Other things worth noting:
* I removed `suggestion.is_top_pick` support because addons should always be top
picks. This lets us simplify the code and tests.
* This changes the overall icon box size for addons from 60px to 52px, since it
combines the icon rules for Pocket and addons. I checked with Josh, and he
says that's fine ([Slack](https://mozilla.slack.com/archives/C03LYL79G82/p1696471328852089?thread_ts=1696446640.986769&cid=C03LYL79G82))
* Currently the default `-webkit-line-clamp` is 3 and MDN and Pocket rows
override it by setting it to 2. I think the opposite makes more sense, i.e.
the default should be 2, since there are no row types that need it to be 3.
This lets me avoid adding a rule for addon rows. (`-webkit-line-clamp` was
added in D185247)
Depends on D190138
Differential Revision: https://phabricator.services.mozilla.com/D190145
This converts `QuickSuggestRemoteSettings` into a `BaseFeature` so that it can
be managed by `QuickSuggest` and easily enabled and disabled depending on
whether the new Rust component is enabled.
Summary of major changes:
* Rename `QuickSuggestRemoteSettings` to `SuggestBackendJs` and make it a
`BaseFeature`. In D188684 I'll also add a new `SuggestBackendRust` feature.
* Introduce a `quickSuggestRustEnabled` Nimbus variable. The JS backend will be
disabled if this variable is true. Nothing else uses the variable in this
patch but D188684 does.
* Move `DEFAULT_SUGGESTION_SCORE` to `UrlbarProviderQuickSuggest` and make the
provider ensure all suggestions have scores.
Differential Revision: https://phabricator.services.mozilla.com/D188681
This enables addon suggestions by default (treatment B, no stars) for all
Suggest users. As discussed on Slack, I did not remove the related prefs and
Nimbus variables in case something goes wrong and we need to disable the feature
ASAP. In that case, we can ship a Nimbus rollout to re-disable it. After a
release cycle or two without any problems, we can do a follow-up that removes
them. I also left the treatment A implementation. We can remove that in a
follow-up too.
Since I did leave the prefs and variables, I only had to change their default
values to enable the feature with treatment B.
One problem is that addon suggestions should be best matches, but we don't want
to set `bestMatch.enabled` to true by default. To work around that, I set the
related best-match properties on the `UrlbarResult` inside `AddonSuggestions`
instead of delegating it to the quick suggest provider. Product has requested
that we remove the "Top pick" checkbox from about:preferences, and at that time
we should also remove the `bestMatch.enabled` pref. In the future, results
should be best matches on a case-by-case basis.
This makes a few UI changes that aren't strictly related but I wanted to fix
since more users will now see these suggestions:
* Cache the "Firefox extension" group label to prevent pop-in.
* Align the URL and row title by baseline. Right now the URL is too high above
the title.
* Set `inline-margin-end` of the icon so it's symmetrical with the starting
margin/padding. The current value of 8px is hardcoded and too small.
Differential Revision: https://phabricator.services.mozilla.com/D187640
This adds a `quickSuggestScoreMap` Nimbus variable that lets experiments
override suggestion scores. It maps from telemetry types to score values. For
example:
```
"quickSuggestScoreMap": {
"amo": 0.25,
"adm_sponsored": 0.3
}
```
In this example, addon suggestions will always have a score of 0.25, and
sponsored suggestions will always have a score of 0.3. Of course, different
branches within an experiment and different experiments can set different
scores.
While working on this, I saw we have a bug when we try to look up the
`BaseFeature` for a result. To do the lookup, we look up the result's
`telemetryType` in `FEATURE_NAMES_BY_TELEMETRY_TYPE`. That's a problem for `adm`
suggestions because the `telemetryType` will be either `adm_sponsored` or
`adm_nonsponsored`, but neither of those is present in
`FEATURE_NAMES_BY_TELEMETRY_TYPE` -- only `adm` is.
To fix it, I added back the `provider` property to result payloads that I
previously removed, and I added `BaseFeature.merinoProvider` so each feature can
specify its Merino provider. Then, `QuickSuggest` can build a map from Merino
provider names to features, allowing us to look up features without needing to
hardcode something like `FEATURE_NAMES_BY_TELEMETRY_TYPE` or
`FEATURE_NAMES_BY_MERINO_PROVIDER`.
Since I added back the `provider` property, I had to update a lot of tests. (As
a follow up, it would be nice to centralize the creation of expected result
objects in the test helper.)
I also added `BaseFeature.getSuggestionTelemetryType()` to help implement the
score map and to better formalize the idea that telemetry types are an important
property that all quick suggest results should include.
Differential Revision: https://phabricator.services.mozilla.com/D181709
This goes through the previous changes in the dependencies of bug 877389, and does two things:
1) Remove instances of \n
2) Change reporting of exceptions so that they are passed as separate arguments. This should result
in an improved display of the exception in the browser console, should it occur.
Differential Revision: https://phabricator.services.mozilla.com/D180843
This implements most parts of Pocket suggestions. They don't need any special UI
or a dynamic result type because they're only shown as the usual best match rows
or non-best match rows.
Still to do:
* Implement the "Show less frequently" behavior once we decide what the keywords
will be and how that will work.
* Implement the bottom "Mozilla Pocket" text inside the suggestion row once it's
finalized. We can use the same technique we use to show the "Sponsored" bottom
text for adM suggestions.
Other changes this makes:
* Replace the `type=bestmatch` attribute with an `bestmatch` attribute. That
lets best-match rows have a `type` too, in this case `"pocket"` (actually
either `"rs_pocket"` or `"merino_pocket"`, since I'm using the telemetry
result type).
* Improve how UrlbarProviderQuickSuggest delegates to individual features when
getting result commands, view updates, handling commands, etc., so that we
don't need to add new `case` statements for each new type of suggestion.
Differential Revision: https://phabricator.services.mozilla.com/D180059
This implements the new required matching behavior, which isn't based on min
keyword length anymore. This is how it works:
* Use the full keywords in remote settings to generate keywords that contain the
first word plus each possible substring after the first word. For example if a
full keyword is "video download", then generate these keywords: "video",
"video ", "video d", "video do", etc. If a full keyword is only one word, then
use it as is. The keywords never change even when the user clicks "Show less
frequently". This is implemented in `onRemoteSettingsSync()`, and I modified
`SuggestionsMap.add()` to make it easy to generate new keywords from the
strings in `suggestion.keywords`.
* Keep track of the number of times the user clicked "Show less frequently" in
`showLessFrequentlyCount`.
* When a suggestion is fetched from the suggestions map, filter it out if the
search string isn't long enough given the `showLessFrequentlyCount`. This is
done in `makeResult()`.
Other changes:
* I made some of the private properties in `AddonSuggestions` public so that the
xpcshell test can easily use them. I think it's OK for them to be public.
* Added `show_less_frequently_cap` to the RS config object so that we can
specify a cap in RS as well as Nimbus.
* mv'ed test_quicksuggest_addResults.js to test_suggestionsMap.js, since I
modified this file. I should have done that back when I replaced `addResults()`
with `SuggestionsMap`.
* Fixed a bug in `SuggestionsMap.add()` where the same suggestion could be added
multiple times to the array stored in the map, if it had duplicate keywords.
Differential Revision: https://phabricator.services.mozilla.com/D179867
