This modifies tests to use the server. There are a few important points to call
out:
This means tests are now using the real Rust component, and we need to make sure
the test RS data is valid and matches what Rust expects. For example, I had to
add `icon` properties to suggestions and set the `advertiser` to "Wikipedia" for
non-sponsored suggestions. Otherwise Rust hits an error on ingest. I also
removed some test cases because they tested behaviors that are impossible with
Rust, for example Pocket keywords that are duplicated in the high- and
low-confidence arrays.
We need to be careful to wait until Suggest is done syncing from remote settings
regardless of whether it's using the JS or Rust backend. I added a way to force
the backends to sync. That way, tests can force a sync, wait for it to finish,
and be sure that all sync activity is done.
A common pattern in tests is to call `ensureQuickSuggestInit()` and then set
Suggest-related prefs (or vice versa). This is a little problematic because both
`ensureQuickSuggestInit()` and setting prefs can cause Suggest to start a sync.
It's more problematic now that we're not mocking remote settings or Rust. So I
combined the two by adding a `prefs` param to `ensureQuickSuggestInit()`. That
way, tests can be sure that all syncing is done once that function returns.
Depends on D192037
Differential Revision: https://phabricator.services.mozilla.com/D192038
This builds on the recent vendor that was done in bug 1859069 and D191004 in
particular.
The parameter to the Rust `SuggestStore.query()` function now takes an object
with a `providers` property, instead of separate boolean sponsored and
nonsponsored properties. `providers` is an array whose items are integers that
identify Rust providers (defined in [the RustSuggest bindings here](https://searchfox.org/mozilla-central/rev/c3c92500ef8ffab4fa53d4b2d5c5e2b49025e89b/toolkit/components/uniffi-bindgen-gecko-js/components/generated/RustSuggest.sys.mjs#953-958)).
`MockRustSuggest` in `QuickSuggestTestUtils.sys.mjs` is becoming kind of a pain
to keep updated. More tasks in `test_quicksuggest_pocket.js` should use Rust
than the ones I've updated here, but they don't because I would need to
(re)implement the prefix-matching behavior in `MockRustSuggest`. In a follow-up
I'd like to explore using a mock remote settings server instead so that the real
Rust component can be tested.
Differential Revision: https://phabricator.services.mozilla.com/D191351
This builds on the recent vendor that was done in bug 1859069 and D191004 in
particular.
The parameter to the Rust `SuggestStore.query()` function now takes an object
with a `providers` property, instead of separate boolean sponsored and
nonsponsored properties. `providers` is an array whose items are integers that
identify Rust providers (defined in [the RustSuggest bindings here](https://searchfox.org/mozilla-central/rev/c3c92500ef8ffab4fa53d4b2d5c5e2b49025e89b/toolkit/components/uniffi-bindgen-gecko-js/components/generated/RustSuggest.sys.mjs#953-958)).
`MockRustSuggest` in `QuickSuggestTestUtils.sys.mjs` is becoming kind of a pain
to keep updated. More tasks in `test_quicksuggest_pocket.js` should use Rust
than the ones I've updated here, but they don't because I would need to
(re)implement the prefix-matching behavior in `MockRustSuggest`. In a follow-up
I'd like to explore using a mock remote settings server instead so that the real
Rust component can be tested.
Differential Revision: https://phabricator.services.mozilla.com/D191351
As the TODO's say, I'd like to clean this up and add support for AMO and Pocket,
but right now let's just get the status quo working.
Depends on D191003
Differential Revision: https://phabricator.services.mozilla.com/D191004
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
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 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
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 adds a bunch of scalars to record navigational suggestions telemetry as
discussed with data science and described in the spec. These scalars are
different from the other Suggest ones because we want to record how nav
suggestions interact with the heuristic result. Unlike the existing scalars, the
keys of these new scalars are the types of heuristics that were shown when a nav
suggestion was or wasn't shown. One of the scalars is updated every time a nav
suggestion is *not* shown, and of course for most users that will be the vast
majority of the time or all the time, so I put all these scalars behind a Nimbus
variable. We'll set the variable to true in the control and treatment branches
of the nav suggestions experiment.
This patch also makes sure nav suggestions are recorded properly in Glean, as
`navigational`. I noticed that dynamic Wikipedia results are currently recorded
as `suggest_non_sponsor`, so I also added a new `dynamic_wikipedia` Glean type
for them. They're also recorded as `urlbar.picked.quicksuggest` in the legacy
telemetry, so I also changed it so they're recorded as
`urlbar.picked.dynamic_wikipedia`.
Currently for dynamic Wikipedia, the non-sponsored scalars are also incremented,
and I discussed with data science whether they and the sponsored scalars should
be incremented for all the new Suggest suggestion types we now have. We agreed
that they should be reserved for the usual partner sponsored and expanded
Wikipedia suggestions, and they should not be used for these new Suggest types,
so this patch also makes that change, and it does not update the non-sponsored
scalars for nav suggestions either.
The other major change this makes is to add a new `subtype` property to quick
suggest result payloads. I think we need a clear, simple way to distinguish
between all these various Suggest suggestion types that doesn't depend on
examining different payload properties depending on the type.
Differential Revision: https://phabricator.services.mozilla.com/D171525
This adds a new pref, `browser.urlbar.weather.zeroPrefix`. When true, weather
suggestions are shown on "zero prefix" like they are now, which means they're
shown when the user focuses the urlbar and before they type anything. When the
pref is false, the weather provider will show suggestions only if the search
string matches a keyword in a set of keywords managed by `QuickSuggest.weather`.
My plan is to store the keywords in the quick suggest config object in remote
settings. Nan suggested this too. Currently the config does not contain any
keywords, but this patch can be tested by setting the pref to false, running the
following in the browser console, and then typing "weather" in the urlbar:
```lang=js
ChromeUtils.importESModule("resource:///modules/QuickSuggest.sys.mjs")
.QuickSuggest.remoteSettings._test_setConfig({
weather_keywords: ["weather"],
});
```
Other changes:
* It's possible for a keyword to match both the weather suggestion and a quick
suggest suggestion. Only the weather suggestion should be shown. This patch
modifies to the muxer for that.
* This modifies `UrlbarView.#rowLabel()` to show the "Top pick" (a.k.a. best
match) label for keyword-based weather suggestions, in addition to zero-prefix
suggestions like we currently do.
* This modifies the remote settings client so it becomes enabled when keyword-
based weather suggestions are enabled, so that the config can be accessed.
Depends on D168738
Differential Revision: https://phabricator.services.mozilla.com/D168757
`QuickSuggestTestUtils.ensureQuickSuggestInit()` was written before Merino, so
it assumes the suggestion objects passed in are remote settings suggestions.
This revision modifies it so Merino and remote settings suggestions can both be
passed in. That makes it a little nicer for tests that need to test Merino
suggestions in particular, like navigational suggestions, dynamic Wikipedia,
etc.
Another motivation for this change is that it makes it clear which type of
suggestion is being passed to `ensureQuickSuggestInit()`. Unfortunately Merino
suggestion objects are slightly different from remote settings result objects
(`block_id` vs. `id` for example), which are both different from UrlbarResult
objects, and it can be confusing when reading tests. Since "result" is the name
of remote settings objects used internally in the remote settings client, I've
used that term here, and I've updated all callers to use it instead of
"suggestion".
This also makes `MerinoTestUtils` and `QuickSuggestTestUtils` singletons.
Otherwise the new `MerinoTestUtils` instance used inside `QuickSuggestTestUtils`
isn't the same as the one used in the test that calls into
`QuickSuggestTestUtils`, which is very confusing. This made me realize it's a
good idea for these test utils objects to be singletons.
Finally I removed `is_top_pick` handling from the remote settings client and
remote settings suggestions, since the related test is now using Merino. I also
removed `_test_is_best_match` since only one test was using it and it's not
necessary.
Depends on D166019
Differential Revision: https://phabricator.services.mozilla.com/D166050
This refactors quick suggest telemetry tests. My goal is to make it really
simple to add tests for new types of suggestions, like dynamic Wikipedia and
weather.
I've split browser_quicksuggest_telemetry.js into different files, one per
suggestion type: sponsored, nonsponsored, and dynamic Wikipedia. I'm working on
a new file for weather in bug 1804536. There are two other files for other
telemetry that isn't primary or core telemetry. (In the patch, I cp'ed
browser_quicksuggest_telemetry.js to these two new files,
browser_telemetry_other.js and browser_telemetry_impressionEdgeCases.js.)
I named the new files browser_telemetry_foo.js. I don't think it's important to
keep the "quicksuggest" in their names because they're already in the
quicksuggest directory and these file names were starting to get long.
I added some automated telemetry helpers to head.js. They can be used to
declaratively define the primary telemetry that's expected for a particular
suggestion. The helpers automatically check all types of primary telemetry for
the suggestion: impression, click, help, and block. When we add a new suggestion
type, we can add a new file that uses these helpers, and hopefully it should be
pretty simple.
I also removed some tasks that in hindsight aren't important and/or are covered
by other tasks/tests.
All the changes from `http` to `https` in this revision are due to lint.
Depends on D162253
Differential Revision: https://phabricator.services.mozilla.com/D164307
This makes Merino opt out on Beta. There are two different parts:
1. Decide what "Beta" means. After discussing this with RyanVM, I'm using the
same definition [used by telemetry](https://searchfox.org/mozilla-central/rev/2f47e3dacf0d773e9c7f363cecf10cfbea490679/toolkit/components/telemetry/app/TelemetryControllerBase.jsm#103,112-114): (a) if `MOZ_UPDATE_CHANNEL` is "beta"
or (b) if it's "release" and `app.update.channel` is "beta". The latter case
is true for RC builds delivered to the Beta channel. I moved the relevant
telemetry logic to `AppConstants` so I can easily use it too. Bug 1435753 has
context on this logic w/r/t telemetry.
2. Change the Merino default so it's enabled on Beta in the offline scenario. I
kept the online scenario's disabled default since the only point of
delivering the online scenario to the user now is to prompt them to opt in to
Merino. This change is in UrlbarPrefs.sys.mjs.
The rest of the patch updates tests.
The effect of this patch will be to use a new default value of true for the
`quicksuggest.dataCollection.enabled` pref on Beta in the offline scenario. If
the user didn't touch the pref at all, then its previous default value was
false, and the new default will now be true.
If the user enabled the pref, it will remain enabled. If the user enabled and
then disabled it, it will remain disabled. That's because the pref is sticky, so
once it has a value on the user branch, it will keep that value. This patch only
changes the value of the pref on the default branch, so it won't override the
user's previous choice since that is set on the user branch.
Differential Revision: https://phabricator.services.mozilla.com/D162353
This adds a weather feature to quick suggest. It periodically fetches a weather
suggestion from Merino. UrlbarProviderQuickSuggest shows the suggestion when the
search string is empty ("zero prefix").
The implementation of the UrlbarResult returned by UrlbarProviderQuickSuggest is
only temporary. Mandy is working on the final UI in
[SNT-323](https://mozilla-hub.atlassian.net/browse/SNT-323). Landing a temporary
implementation allows Mandy to base her patch on it and trigger real weather
suggestions from Merino to test her implementation against. It also lets people
on the team test weather suggestions without having to wait for the final UI to
land.
I added the following prefs and Nimbus variable to control the feature. It will
initially be rolled out in an experiment, so we need a Nimbus variable. In the
initial experiment, users will be able to dismiss the suggestion but not toggle
a checkbox in about:preferences.
* `weatherFeatureGate` - Nimbus variable that controls the whole feature
* `browser.urlbar.weather.featureGate` - Fallback pref for the Nimbus variable
* `browser.urlbar.suggest.weather` - When the feature gate pref is true, this
determines whether the suggestion should be shown. In a future patch, we'll
flip this to false when users dismiss the suggestion.
I set the fetch interval to 30 minutes. That seems reasonable considering that
the suggestion contains the current temperature and weather. Merino will set
caching headers appropriately so that Firefox won't actually make a new network
request if the previously fetched suggestion is new enough.
Depends on D161368
Differential Revision: https://phabricator.services.mozilla.com/D161410
This adds a weather feature to quick suggest. It periodically fetches a weather
suggestion from Merino. UrlbarProviderQuickSuggest shows the suggestion when the
search string is empty ("zero prefix").
The implementation of the UrlbarResult returned by UrlbarProviderQuickSuggest is
only temporary. Mandy is working on the final UI in
[SNT-323](https://mozilla-hub.atlassian.net/browse/SNT-323). Landing a temporary
implementation allows Mandy to base her patch on it and trigger real weather
suggestions from Merino to test her implementation against. It also lets people
on the team test weather suggestions without having to wait for the final UI to
land.
I added the following prefs and Nimbus variable to control the feature. It will
initially be rolled out in an experiment, so we need a Nimbus variable. In the
initial experiment, users will be able to dismiss the suggestion but not toggle
a checkbox in about:preferences.
* `weatherFeatureGate` - Nimbus variable that controls the whole feature
* `browser.urlbar.weather.featureGate` - Fallback pref for the Nimbus variable
* `browser.urlbar.suggest.weather` - When the feature gate pref is true, this
determines whether the suggestion should be shown. In a future patch, we'll
flip this to false when users dismiss the suggestion.
I set the fetch interval to 30 minutes. That seems reasonable considering that
the suggestion contains the current temperature and weather. Merino will set
caching headers appropriately so that Firefox won't actually make a new network
request if the previously fetched suggestion is new enough.
Depends on D161368
Differential Revision: https://phabricator.services.mozilla.com/D161410
This ports the remote settings client to `BaseFeature`.
The remote settings client is different from the other two features I ported in
this patch stack because it depends on preferences, not Nimbus variables:
`suggest.quicksuggest.sponsored` and `suggest.quicksuggest.nonsponsored`. To
support that, I modified `BaseFeature` so it can list the prefs it depends
on. When QuickSuggest observes a pref change, it checks whether it's a pref for
a feature, and if so, it updates it.
Please see bug 1799264 for details.
Depends on D161367
Differential Revision: https://phabricator.services.mozilla.com/D161368
This does the following:
* Get rid of `QUICK_SUGGEST_SOURCE` since it's only used in a couple of
places. Its simpler to use the string literals directly.
* Set `source: "merino"` on Merino suggesions in the Merino client instead of
doing it in UrlbarProviderQuickSuggest, similar to how the remote settings
client sets `source: "remote-settings"`
* Export `ONBOARDING_CHOICE` and `ONBOARDING_URI` on the QuickSuggest object for
consistency with other consts
* Remove unnecessary consts from QuickSuggestTestUtils that are already defined
on QuickSuggest
Please see bug 1798595 for details.
Depends on D160986
Differential Revision: https://phabricator.services.mozilla.com/D160987
This does the following:
* Moves quick suggest initialization from UrlbarQuickSuggest to QuickSuggest
* Renames UrlbarQuickSuggest.sys.mjs to QuickSuggestRemoteSettingsClient.sys.mjs, so now this file is focused only on remote settings
* Makes QuickSuggest create an instance of QuickSuggestRemoteSettingsClient and keep it as `QuickSuggest.remoteSettings`
* Moves latency telemetry from UrlbarProviderQuickSuggest into QuickSuggestRemoteSettingsClient
* Changes the ad hoc logger used in QuickSuggestRemoteSettingsClient to a proper urlbar-style logger
* Updates consumers to use `QuickSuggest.remoteSettings` instead of UrlbarQuickSuggest
Please see bug 1798595 for details.
Depends on D160985
Differential Revision: https://phabricator.services.mozilla.com/D160986
This moves the help URL const out of UrlbarProviderQuickSuggest in order to make
UrlbarProviderQuickSuggest focused on being a urlbar provider.
It also changes how QuickSuggestTestUtils is initialized to be more similar to
MerinoTestUtils. There's no need for a separate `init()` method when the
constructor will do. I'd like to make this change in a different revision but I
did it here because I want to include all changes to about:preferences in one
revision for easier review.
Please see bug 1798595 for details.
Depends on D160984
Differential Revision: https://phabricator.services.mozilla.com/D160985
