This allows us to move away from using IsNavigating field in parent-controlled
paths. Use a new distinct error code in cases when we cancel loads in
Canonical BC due to another load starting. This way, we know to not reset the
urlbar if we are doing another load.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1721217#c10 for longer
explanation of what is going on here.
Differential Revision: https://phabricator.services.mozilla.com/D126845
We only need to default `browser.urlbar.merino.enabled` to true. If the user has
opted in (either through the modal or by toggling on the data collection pref in
the prefs UI), then `quicksuggest.dataCollection.enabled` will also be true and
we'll fetch Merino suggestions. Otherwise it will be false and we won't fetch
Merino suggestions. That logic is implemented here:
https://searchfox.org/mozilla-central/rev/9a5f36b0ddb9cb8ae556fc5b45f8ccea0f0da6f8/browser/components/urlbar/UrlbarProviderQuickSuggest.jsm#144
Note this defaults the pref to true for everyone, even users in offline. It make
senses now that we have a separate toggle for data collection in the preferences
UI. Even offline users can opt in to Merino and data collection.
I also updated the various sets of prefs for test suites so that the Merino
endpoint URL is empty when running tests so they don't hit the network. I could
have forced `merino.enabled` to false instead, but setting the endpoint URL has
a couple of benefits, although admittedly they're very small:
* It runs a little more of the Merino code path (i.e., calls
`_fetchMerinoSuggestions`)
* It lets Merino tests set only one pref, the endpoint URL, instead of two, both
the endpoint pref and enabled pref
Differential Revision: https://phabricator.services.mozilla.com/D131988
This allows us to move away from using IsNavigating field in parent-controlled
paths. Use a new distinct error code in cases when we cancel loads in
Canonical BC due to another load starting. This way, we know to not reset the
urlbar if we are doing another load.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1721217#c10 for longer
explanation of what is going on here.
Differential Revision: https://phabricator.services.mozilla.com/D126845
This code comment may end up being moot depending on changes we make to the
online scenario once we start rolling that out again. Now that we have a
separate user-facing pref for data collection, it probably doesn't make sense
anymore to disable suggestions by default for online. Instead they should
probably be enabled by default, just like they are for offline. The only thing
online should even be anymore is just an opt-in modal for data collection.
But anyway let's fix it for now so there's not wrong info in the meantime.
Differential Revision: https://phabricator.services.mozilla.com/D131271
This adds `UrlbarProviderQuickSuggest.isURLEquivalentToResultURL()` to tell
whether a URL is equivalent to a quick suggest URL. URLs are equivalent if they
are the same except for their timestamp substrings. The muxer uses the new
method to discard history results whose URLs are equivalent to the quick suggest
URL.
Depends on D130943
Differential Revision: https://phabricator.services.mozilla.com/D131181
This moves the template substitution logic from UrlbarQuickSuggest to
UrlbarProviderQuickSuggest, where it can be applied to all suggestions
regardless of source.
Differential Revision: https://phabricator.services.mozilla.com/D130943
Otherwise, the warning is displayed:
```
WARNING: Could not lex literal_block as "json". Highlighting skipped.
```
Depends on D131092
Differential Revision: https://phabricator.services.mozilla.com/D131093
This updates the non-localized strings that appear in the Firefox Suggest
preferences UI with the final approved strings.
Differential Revision: https://phabricator.services.mozilla.com/D131031
The problem here is a race: Even though the user is enrolled in the online
scenario via Nimbus, the Nimbus urlbar variables may not be initialized by the
time we migrate Suggest preferences on startup. The data-collection pref is set
when we [migrate prefs](https://searchfox.org/mozilla-central/rev/df6434d2ebfdf2b5f89f205fc81d60d64a774fe1/browser/components/urlbar/UrlbarPrefs.jsm#872) -- and only there --
so we're not hitting that path like we should be. That's because the scenario
can still be the default value of "offline".
Fortunately the Nimbus client API gives us a ready promise we can await that is
resolved when the feature is loaded and ready, so all we need to do is await it
before trying to update the scenario and do migrations.
Differential Revision: https://phabricator.services.mozilla.com/D131065
Behind a pref for now. Given these selectors do nothing on non-chrome
documents (they just don't match) it seems worth trying.
A cursory search seems to indicate they're not used for UA detection or
something like that (or at least I haven't found such an usage).
Differential Revision: https://phabricator.services.mozilla.com/D130736
Behind a pref for now. Given these selectors do nothing on non-chrome
documents (they just don't match) it seems worth trying.
A cursory search seems to indicate they're not used for UA detection or
something like that (or at least I haven't found such an usage).
Differential Revision: https://phabricator.services.mozilla.com/D130736
Now that we're likely not targeting a 94 dot release anymore, I'd like to go
ahead and rename the `suggest.quicksuggest` pref to
`suggest.quicksuggest.nonsponsored` as we considered doing before. There won't
be a better time to do it.
Depends on D128665
Differential Revision: https://phabricator.services.mozilla.com/D130565
This adds telemetry for the new data-collection pref:
* Adds a `data_collect_toggled` object to the `contextservices.quicksuggest`
telemetry event
* Adds the new pref to telemetry environment
Depends on D128661
Differential Revision: https://phabricator.services.mozilla.com/D128665
This updates the Privacy pane for the new Firefox Suggest preferences spec.
Please see the spec here: https://mozilla-hub.atlassian.net/browse/SNT-37
The Jira description is a little out of date, and the wording of these new
strings (which are not exposed to localizers) isn't finalized, but I'm told the
structure of the UI is final more or less.
There is also a Figma here: https://www.figma.com/file/seJ2ZA4v3FgoV7jCxUR74B/Firefox-Suggest-exploration?node-id=3197%3A55695
We're replacing the current two Firefox Suggest checkboxes with three toggle
buttons. The first two toggle buttons correspond to the existing
`browser.urlbar.suggest.quicksuggest` and
`browser.urlbar.suggest.quicksuggest.sponsored` prefs. However, the second pref
is no longer dependent on the first, and it can be toggled regardless of whether
the first is enabled. The third toggle corresponds to a new pref,
`browser.urlbar.quicksuggest.dataCollection.enabled`. It can also be toggled
independently of the others.
In addition, we're adding an info bar/box below the toggles to explain to the
user the effect of their toggle selection. The text in the box depends on the
state of the toggles. The box itself is hidden when all three toggles are off.
Depends on D129224
Differential Revision: https://phabricator.services.mozilla.com/D128661
This is a big revision that makes a lot of changes. The core change is adding a
new pref to control data collection for Firefox Suggest and making the existing
non-sponsored and sponsored prefs independent of each other. The data-collection
pref defaults to false in every scenario; in online, the user is opted in to it
when they opt in to the modal, and in both online and offline the user will be
able to enable it in the preferences UI (part 2 of this bug, see below).
A new requirement for this bug that came up after I wrote my original patch in
D128579 is that we want the ability to enable Merino via an experiment, which
requires the data-collection pref to be true. That's tricky because data
collection will be exposed in the preferences UI. If the user disables it, we
don't want to override that. There's a distinction to draw between its being
disabled/enabled by default and its being disabled/enabled by the user. It's OK
to override the default but not the user's choice, and that's true not only for
this new data-collection pref but for all prefs exposed in the UI.
We can handle that by taking care to set prefs on the default and user branches
as appropriate. If a pref exposed in the UI has a value on the user branch, then
we can assume the user modified it, and ideally that value should stick around
indefinitely. On the other hand we can set default values as configured by
Nimbus on the default branch. If the user modified a pref, then it'll take
precedence since it's on the user branch.
To make user-modified preferences stick around indefinitely, I made them sticky
in firefox.js. That way even when user values are the same as default values,
the user-branch values will be preserved, which is not true of non-sticky prefs.
I added a new Nimbus variable for data collection, so we can use that to enable
it by default. It was trivial to add similar variables for the other UI-exposed
prefs, the two suggestions types, so I did that too.
This is all tested pretty thoroughly in browser_quicksuggest_configuration.js.
This is only part 1 of 4. Part 2 updates the prefs UI, part 3 adds telemetry for
the new data-collection pref (event telemetry when it's toggled, similar to the
existing prefs, and telemetry environment), and part 4 changes the name of
`browser.urlbar.suggest.quicksuggest` to
`browser.urlbar.suggest.quicksuggest.nonsponsored`.
For review, I'd suggest looking at parts in the following order:
1. UrlbarPrefs.jsm, browser_quicksuggest_configuration.js, and
test_quicksuggest_migrate.js.
The core changes are in UrlbarPrefs.jsm. The two test files document the
expected behavior of different scenarios/enrollments and prefs migrations.
browser_quicksuggest_configuration.js tests different enrollments and the
prefs stuff I mentioned above.
test_quicksuggest_migrate.js tests prefs migrations. I added a migration step
that's run on startup when the scenario is updated. There are two things we
need to do: decouple the existing `suggest.quicksuggest` prefs and initialize
the new data-collection pref. (For brevity I'll omit the `browser.urlbar`
prefix to all these pref names in this commit message.)
The `suggest.quicksuggest` pref now controls non-sponsored suggestions, and
the `suggest.quicksuggest.sponsored` now controls sponsored suggestions
independent of the other pref. The new data pref is
`quicksuggest.dataCollection.enabled`.
For the current online rollout on 92.0.1, we don't record the user's choice
to the opt-in modal, so the initial value of the data pref for existing users
is debatable. I Slacked with Natalie about it, and we think it's OK to
initialize it to allow data collection as long as the main
`suggest.quicksuggest` pref is true. The user may not have opted in to the
modal in that case, but if not, they enabled suggestions in
about:preferences, where we show a description under the main Firefox Suggest
checkbox about data sharing.
For new profiles (i.e., profiles that are created in versions of Firefox
where this revision has landed), the migration step runs but is effectively a
no-op.
2. UrlbarProviderQuickSuggest.jsm and UrlbarProviderQuickSuggest.jsm. The
changes here are again related to decoupling the `suggest.quicksuggest` prefs
and checking the data pref before recording data in telemetry and fetching
from Merino.
3. All the other tests. I ended up adding some checks to a lot of them and
simplifying some, especially browser_urlbar_telemetry_quicksuggest.js.
Depends on D130531
Differential Revision: https://phabricator.services.mozilla.com/D129224
This adds a new histogram called
`FX_URLBAR_QUICK_SUGGEST_REMOTE_SETTINGS_LATENCY_MS`. That's a verbose name, but
urlbar-related histograms are prefixed with `FX_URLBAR`, and if the
`QUICK_SUGGEST` part wasn't there it wouldn't be clear which urlbar use of
remote settings the probe was referring to.
The histogram has a range of 30 seconds, which is too big for something that
happens entirely on the client, but Corey says it's more important that it has
the same buckets and period as the similar Merino latency histogram.
Depends on D130530
Differential Revision: https://phabricator.services.mozilla.com/D130531
This adds a new categorical histogram called `FX_URLBAR_MERINO_RESPONSE`. There
are four categories per the discussion in the bug:
0: success
1: timeout
2: network_error
3: http_error
Only one value is recorded per fetch, so for example if Merino times out but
then later finishes successfully, we only record the timeout.
Depends on D129772
Differential Revision: https://phabricator.services.mozilla.com/D130530
This adds a 200ms timeout using `setTimeout` and the current `AbortController`.
The timeout value can be set in prefs and Nimbus.
It also modifies SkippableTimer in UrlbarUtils.jsm so canceling and firing are
idempotent.
Depends on D130429
Differential Revision: https://phabricator.services.mozilla.com/D129772
I ran into problems while working on tests for bug 1737923 and bug 1737928,
where pref changes were causing UrlbarQuickSuggest to recreate its results map
and keyword tree and they both ended up becoming empty at random points during
the tests. With the way UrlbarQuickSuggest is currently written, there's no way
for tests to prevent that from happening, so I came back to this bug and finally
finished it.
This revision makes these changes:
* Serialize all access to RS, so for example we're not trying to handle a sync
listener callback from RS at the same time we're initializing the
UrlbarQuickSuggest instance, or we're not trying to do two concurrent syncs
from RS (`onEnabledUpdate`) when quick suggest prefs change back to back. This
also allows tests to safely access/test the UrlbarQuickSuggest instance
(combined with `readyPromise`) without the possibility of the data changing at
the same time
* Add a `readyPromise` that consumers (tests) can use so they can be certain
that no updates are currently ongoing
* Simplify the methods related to syncing from RS. Now there's only one method,
`_queueSettingsSync`
* Don't unnecessarily recreate the RS client (`_rs`) and sync from it every time
a quick suggest pref changes or something else causes `onEnabledUpdate` to be
called
* Tear down the RS client (`_rs`) when quick suggest is disabled so that it's
not taking up memory (hopefully?) and it's not listening for syncs that are
unnecessary when the user turns off suggestions
* Move `SUGGESTION_SCORE` from KeywordTree to UrlbarQuickSuggest. It was
supposed to be there all along but I added it to the wrong class in D124132!
:-/
* Make a few other style/cosmetic tweaks to UrlbarQuickSuggest
* Make some unrelated trivial improvements to test_quicksuggest_merino.js
Depends on D128970
Differential Revision: https://phabricator.services.mozilla.com/D130429
I've added the most minimal changes I could, but I also removed some code bloat,
by moving the assignment of this.inputField out of the if-control block's branches.
Also changed the deprecated use of events created with createEvent("UIEvents"),
so that they use "new UIEvent()" constructor.
Differential Revision: https://phabricator.services.mozilla.com/D128013
I've added the most minimal changes I could, but I also removed some code bloat,
by moving the assignment of this.inputField out of the if-control block's branches.
Also changed the deprecated use of initUIEvent at the bottom of method to adhere
to Mozilla's own advice of using new UIEvent instead.
this._revertOnBlurValue was assigned the wrong value due to this.value being a getter after my patch.
This is now fixed.
Fixed code to follow code style standard.
Changed other input event that should be constructed using new UIEvent().
Left event called "ValueChange" alone.
Differential Revision: https://phabricator.services.mozilla.com/D128013
