This integrates Rust exposure suggestions with desktop. Exposure suggestions are
a part of the replacement for the existing potential exposures feature
(bug 1881875). When we want to test potential exposures in the future, we can
add new exposure suggestions to remote settings and tell the Rust component to
ingest them and return them in queries. When the Rust component returns an
exposure suggestion, desktop will record it in exposure telemetry. (The other
part of the replacement is keyword exposure telemetry in bug 1915507 D220501).
More details about the design of exposure suggestions here:
* Bug 1893086
* https://github.com/mozilla/application-services/pull/6343
The way desktop tells the Rust component about different types of exposure
suggestions is through the new "provider constraints" feature. `ingest()` and
`query()` can take a `SuggestionProviderConstraints` object that changes what's
ingested and queried. Therefore, we need to re-ingest exposure suggestions when
the provider constraints change. Right now, exposure suggestions are the only
kind of suggestions that use provider constraints.
Depends on D220359
Differential Revision: https://phabricator.services.mozilla.com/D220359
Please see the bug for details.
This makes some changes to search strings and URLs in the tests that might look
unnecessary. They're due to switching to the QuickSuggestTestUtils helpers in
the head files instead of hardcoding the suggestions data. It's possible to
force the helpers to use the existing search strings and URLs that are used in
these tests, but it's good to be consistent with other tests that use these
helpers because it makes everything easier to understand.
Depends on D220352
Differential Revision: https://phabricator.services.mozilla.com/D219943
Please see the bug for details.
This makes some changes to search strings and URLs in the tests that might look
unnecessary. They're due to switching to the QuickSuggestTestUtils helpers in
the head files instead of hardcoding the suggestions data. It's possible to
force the helpers to use the existing search strings and URLs that are used in
these tests, but it's good to be consistent with other tests that use these
helpers because it makes everything easier to understand.
Depends on D219942
Differential Revision: https://phabricator.services.mozilla.com/D219943
As of 11amPT December 7, Contextual Services is now looking exclusively at
Glean-sent data for Firefox versions 116+. (See DSRE-1489)
Differential Revision: https://phabricator.services.mozilla.com/D195910
This makes sure `result.payload.originalUrl` is set correctly depending on Rust
vs. JS, and AMP vs. Wikipedia. The logic is getting a little messy due to all
the various combinations, but there's no way around it until we drop the JS
backend.
I tried setting `raw_url` (or `rawUrl`) on JS suggestions to match Rust
suggestions, and while that made the `AdmWikipedia` logic slightly nicer, it
either made the `UrlbarProviderQuickSuggest._canAddSuggestion()` logic a little
uglier because we need to also check `raw_url` for JS suggestions (snake_case),
or instead we could set `rawUrl` (camelCase) on JS suggestions just like Rust
suggestions, but every other property on JS suggestions uses snake_case.
This is *not* based on bug 1861540 because we need to uplift this, but I don't
want to uplift bug 1861540.
Differential Revision: https://phabricator.services.mozilla.com/D192124
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
The new Rust implementation of Suggest stores icons for some suggestion types in
its Sqlite database, and it returns these icons to consumers as byte arrays. To
show these icons in the view quickly and without any overhead, we can create
`Blob` URLs from the arrays. Blob URLs need to be revoked when we're done with
them to avoid leaking the backing data.
This patch implements some simple lifetime management of blob URLs in the view.
The first time the view shows a result, it creates a blob URL for the result's
icon. The view caches the URL while it remains open, so as the user continues to
type and possibly match the same result many times, the view will use the same
blob URL each time. When the view closes, it revokes the URL. This seems like a
reasonable, natural lifetime for these URLs, and the implementation is simple.
Depends on D189452
Differential Revision: https://phabricator.services.mozilla.com/D189615
This updates the engagement Glean test so it checks Rust results.
It also adds a getter to `SuggestBackendRust` that returns a promise that's
resolved once the backend's initialization is fully complete. Setting
`quicksuggest.rustEnabled` will either trigger the Rust backend to perform
initialization and ingestion, or it will trigger the JS backend to re-sync all
features. Either way, tests need to wait until all of that is done.
Differential Revision: https://phabricator.services.mozilla.com/D189452
This builds on D188681 and adds a new `BaseFeature` called `SuggestBackendRust`.
When `quickSuggestRustEnabled` is true, `UrlbarProviderQuickSuggest` will use
`SuggestBackendRust` to fetch remote settings suggestions; otherwise it will use
`SuggestBackendJs`.
The Rust component is already integrated into desktop Firefox (bug 1851256, bug
1851845), and it's exposed to JS via `RustSuggest.sys.mjs`. Currently it only
supports AMP (sponsored, a.k.a. adM) and Wikipedia (non-sponsored) suggestions.
It's possible to configure the path of the Sqlite file created by the Rust
component. This patch uses `suggest.sqlite` in the user's local profile (cache)
directory.
This is only the initial integration. I can think of a few follow-ups:
* Handle icons. In this patch, results from `SuggestBackendRust` don't have
icons at all. I have a WIP.
* Handle ingestion better. "Ingest" here means Firefox must tell the Rust
component to re-fetch suggestions from remote settings and rebuild its Sqlite
database. Unfortunately the Rust component doesn't keep the data updated by
itself, so we'll need to periodically tell the component to ingest. This patch
performs ingestion every time `SuggestBackendRust` is (re)enabled, which is a
good enough start.
* Maybe handle tests better. For now I modified the main quick suggest unit
test, test_quicksuggest.js, so it tests both backends. Other tests should
maybe be updated too, I'm not sure yet.
Depends on D188681
Differential Revision: https://phabricator.services.mozilla.com/D188684
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 adds two new fields to the Suggest PingCentre and Glean pings:
* `suggested_index` -
An integer value that is the intended index of the suggestion
being interacted with. If `suggested_index_relative_to_group` is true, the
index is relative to the "Firefox Suggest" group; otherwise the index is
relative to the entire list of suggestions. Non-negative values (starting
at 0) are relative to the start/top of the group/list; negative values are
relative to the end/bottom of the group/list.
* `suggested_index_relative_to_group` -
Whether `suggested_index` is relative to the "Firefox Suggest" group. If
false, it is relative to the entire list of suggestions.
In the Glean ping, `suggested_index` is stringified because there's no integer
metric type that can take negative numbers.
These two values reflect how the implementation works and will let us record any
indexes we want to experiment with. Suggestions are usually shown at the bottom
of the Firefox Suggest group, and in that case these values will be:
```
suggested_index = -1
suggested_index_relative_to_group = true
```
If we want to show suggestions at the top of the Firefox Suggest group instead
of the bottom, the values will be:
```
suggested_index = 0
suggested_index_relative_to_group = true
```
Differential Revision: https://phabricator.services.mozilla.com/D189035
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. (The actual bottom text per
result is added in D182632 since I didn't want to make this patch bigger than
necessary)
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", and the view
also uses it to create the "Top pick" row/group label. 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). IMO this is better than adding
rules for each type of suggestion to the CSS. It's cleaner and also indicates
what the "standard" icon sizes are.
Depends on D182580
Differential Revision: https://phabricator.services.mozilla.com/D182537
Covers topsites and quicksuggest impressions, clicks, and blocks.
Removes unused and not-to-be-used topsites-in-urlbar pingcentre instrumentation
rather than reinstrumenting it.
Differential Revision: https://phabricator.services.mozilla.com/D179856
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 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
Please see bug 1832927 for background. This fixes the problem on 115 by moving
the sponsored/nonsponsored logic back to the provider.
I added a task to test_quicksuggest_topPicks.js, and I created xpcshell tests
for addons and dynamic Wikipedia with similar tasks. It would be nice to unify
this check for all quick suggest types somehow -- maybe a task in
test_quicksuggest.js, but that's not quite as simple as it seems because each
suggestion type has its own suggestion object and expected result payload. We
might also want to add more tasks to these new files. We can think about that
later because there are other opportunities for test consolidation too.
Differential Revision: https://phabricator.services.mozilla.com/D177952
This keeps the current behavior where Firefox shows a suggestion as a top pick
when `is_top_pick` is true, but in addition the two best-match prefs must also
be true.
I cp'ed test_quicksuggest_bestMatch.js to test_quicksuggest_topPicks.js so that
we have a test specifically for top picks, and I added tasks for all preference
combinations. The terms "best match" and "top picks" are overloaded and I tried
to explain what they mean in the code comments I added.
Depends on D176114
Differential Revision: https://phabricator.services.mozilla.com/D177712
This refactors quick suggest remote settings management so it will be easier to
support new types of remote settings suggestions. It builds on D177191.
Currently, RemoteSettingsClient only handles adM and Wikipedia suggestions. It
would be possible for it to support more suggestions types by adding a series of
`if` statements, one per suggestion type (to check if the suggestion type is
enabled), with each statement fetching a specific suggestion type's data.
However, that doesn't scale elegantly.
It would also be possible for RemoteSettingsClient to be agnostic about the
suggestions in remote settings. IOW it doesn't need to care that different types
of suggestions are stored in RS. It could treat them all as one big map from
keywords to suggestions. However, that would require uniformity and stability in
how suggestions are stored in RS, and since the Suggest project is not mature,
that's not the case right now, and it won't be the case any time soon.
Instead, this revision moves suggestion-specific logic from RemoteSettingsClient
to BaseFeature classes specific to each suggestion type. Each BaseFeature
registers itself with remote settings using the new QuickSuggestRemoteSettings
module, fetches its specific RS data, and builds whatever data structure is
appropriate for serving its suggestions. I expect most features to use a simple
map from keywords to suggestions, like adM/Wikipedia does, so I factored out the
map-related code into a new SuggestionsMap class. The weather feature is an
exception because only its keywords are stored in RS, not the suggestion itself.
UrlbarProviderQuickSuggest accesses remote settings suggestions by going through
QuickSuggestRemoteSettings. It only needs to make one call.
This design should work well when the cross-platform Rust component is ready.
We should only need to modify UrlbarProviderQuickSuggest so it fetches
suggestions from the Rust component instead of QuickSuggestRemoteSettings.
Depends on D177191
Differential Revision: https://phabricator.services.mozilla.com/D176779
This makes some changes to UrlbarProviderQuickSuggest that are much more modest
than my previously proposed refactorings in D176111 and D175998. After thinking
about it more and discussing it with @wstuckey and @daisuke, I'm not sure it's a
good idea to split UrlbarProviderQuickSuggest into multiple providers. In the
future, we will replace a lot of our desktop JS with a single cross-platform
Rust component that serves remote settings suggestions and possibly Merino
suggestions too. We should work with that in mind, and I don't think it makes a
lot of sense to have multiple urlbar providers all fetching from this one
component, even though it would make some things nicer, like being able to
isolate suggestion-specific UI code to each provider.
The main goal of this revision is to better prepare UrlbarProviderQuickSuggest
for many new types of suggestions:
* Add `#makeResult()`, which returns a new `UrlbarResult` appropriate for a
given suggestion
* Add `#makeDefaultResult()`, which returns a result for suggestions that either
don't need special handling or are unrecognized
* Remove the `RESULT_SUBTYPE` consts. The idea is that the client doesn't need
to care too much about the types of results Merino returns, except when
special handling is required (special UI, special telemetry, etc.)
* Add `telemetryType` to result payloads so that the result types recorded in
Glean are clear and well defined. `telemetryType` is also used to tell the
type of a result in general. For results that are served by Merino,
`telemetryType` is the Merino provider name
* Streamline legacy telemetry handling a little, although hopefully we won't be
doing too much legacy telemetry in the future
There are still open questions that this revision does not resolve, especially
the ability isolate suggestion-specific UI code in a nice way (dynamic result
types). I don't think this revision paints us into any corners.
Other changes:
* Properly document the `${source}_${type}` result types in metrics.yaml (added
in D174209)
* For Glean result types, replace "suggest_sponsor" with "merino_adm_sponsored"
and "rs_adm_sponsored", and replace "suggest_non_sponsor" with
"merino_adm_nonsponsored" and "rs_adm_nonsponsored". This is more consistent
with the `${source}_${providerName}` convention I'd like to establish.
* Remove code related to Nimbus exposures and a test
(browser_quicksuggest_bestMatch.js). We aren't using Nimbus exposures anymore.
We can always add it back if necessary.
* Don't record the custom contextual services pings for dynamic Wikipedia
results. These pings are only necessary for adM suggestions.
Differential Revision: https://phabricator.services.mozilla.com/D177191
