This fixes the bug by not starting fetches until a config is set from either
remote settings or Nimbus. By "config" I mean keywords basically, but we sync
more than keywords -- the min keyword length and min keyword length cap -- so
that's why I use different term. So, before remote settings is synced, no config
will be set, so no fetches will happen, so the suggestion will be null. When the
urlbar provider starts a query, it will see the suggestion is null and not add a
result. Once a config is set from RS or Nimbus, we'll start fetching.
Currently we allow zero prefix when keywords or the min keyword length aren't
set. This patch removes that functionality because on second thought, there's
not a safe and obvious way to support zero prefix using these keyword-related
config properties/variables by themselves, and zero prefix isn't a feature
requirement anymore anyway. If we wanted to keep supporting it with these
properties/variables, there are a few options, and I don't like any of them:
* If `keywords` is undefined or null, use zero prefix. This is dangerous because
we may make a mistake in RS or Nimbus and forget to set it. Also, we use null
as the default value for the Nimbus var, and since we use UrlbarPrefs to
access Nimbus vars, there's no good way to tell whether null was set
intentionally or not.
* If `keywords` is an empty array, use zero prefix. This is awkward because the
user can now increment the min keyword length, which means the keywords array
kept by `Weather` can become empty when the min keyword length is incremented
a lot. In that case, no suggestion should be shown at all.
* If `min_keyword_length` is zero/falsey, use zero prefix. This has the same
problems as the first point above.
If we do need to support zero prefix again in the future, I think we should add
an RS property/Nimbus variable that makes it clear what's happening, e.g., a
`useZeroPrefix` boolean.
I removed the exposure scalar because it's entirely based on zero prefix. We can
use Glean for that now anyway.
I also noticed weather suggestions are case insenstive, so I fixed that and
added a test task.
Differential Revision: https://phabricator.services.mozilla.com/D177448
This fixes the bug by not starting fetches until a config is set from either
remote settings or Nimbus. By "config" I mean keywords basically, but we sync
more than keywords -- the min keyword length and min keyword length cap -- so
that's why I use different term. So, before remote settings is synced, no config
will be set, so no fetches will happen, so the suggestion will be null. When the
urlbar provider starts a query, it will see the suggestion is null and not add a
result. Once a config is set from RS or Nimbus, we'll start fetching.
Currently we allow zero prefix when keywords or the min keyword length aren't
set. This patch removes that functionality because on second thought, there's
not a safe and obvious way to support zero prefix using these keyword-related
config properties/variables by themselves, and zero prefix isn't a feature
requirement anymore anyway. If we wanted to keep supporting it with these
properties/variables, there are a few options, and I don't like any of them:
* If `keywords` is undefined or null, use zero prefix. This is dangerous because
we may make a mistake in RS or Nimbus and forget to set it. Also, we use null
as the default value for the Nimbus var, and since we use UrlbarPrefs to
access Nimbus vars, there's no good way to tell whether null was set
intentionally or not.
* If `keywords` is an empty array, use zero prefix. This is awkward because the
user can now increment the min keyword length, which means the keywords array
kept by `Weather` can become empty when the min keyword length is incremented
a lot. In that case, no suggestion should be shown at all.
* If `min_keyword_length` is zero/falsey, use zero prefix. This has the same
problems as the first point above.
If we do need to support zero prefix again in the future, I think we should add
an RS property/Nimbus variable that makes it clear what's happening, e.g., a
`useZeroPrefix` boolean.
I removed the exposure scalar because it's entirely based on zero prefix. We can
use Glean for that now anyway.
I also noticed weather suggestions are case insenstive, so I fixed that and
added a test task.
Differential Revision: https://phabricator.services.mozilla.com/D177448
This implements the weather suggestion result menu UI and builds on D174941.
References:
* [Spec]( https://www.figma.com/file/Hdi0oHB7trRcncyVAKZypO/accuweather-explorations?node-id=2421%3A62540&t=29w6wH3UYchqBxqX-1) (See "A11y review" in the sidebar)
* [Clickable prototype](https://www.figma.com/proto/Hdi0oHB7trRcncyVAKZypO/accuweather-explorations?page-id=2192%3A42825&node-id=2394-52468&viewport=246%2C526%2C0.12&scaling=min-zoom&starting-point-node-id=2394%3A52468&show-proto-sidebar=1) (See "Revised 4/3" in the sidebar)
There are a couple important points about the menu. First, one of the commands,
"Report inaccurate location", is specific to weather suggestions, or at least
location-based suggestions. I don't think it's a good idea to centralize all
commands in UrlbarView, and in general I'd like to stop centralizing handling of
different result types in the view and input, so I added a new provider method
called `getResultCommands()`.
Second, the spec calls for a menu separator and a submenu so the user can select
a reason they don't want to see the result, so the return value of
`getResultCommands()` is flexible enough to support those two things, and I
modified `#populateResultMenu()` too.
These new commands will be recorded in Glean engagement telemetry as new
`engagement_type` values, same as "dismiss" and "help" currently are.
This patch doesn't implement handling of two of the commands, "Report inaccurate
location" and "Show less frequently", because I wanted to keep it focused on the
fundamentals described above.
Depends on D174941
Differential Revision: https://phabricator.services.mozilla.com/D174994
This removes `UrlbarProvider.pickResult()` and `blockResult()` in favor of
handling picks and dismissals through `onEngagement()`. A number of providers
use those two methods, so this revision touches a lot of files.
Handling dismissals through `onEngagement()` means `UrlbarInput.pickResult()`
can no longer tell whether a result is successfully dismissed, so it can't
remove the result anymore. (Maybe `onEngagement()` could return some value
indicating it dismissed the result, but I don't want to go down that road.)
Instead, I split `UrlbarController.handleDeleteEntry()` into two methods: a
public one that removes the result and notifies listeners, and a private one
that handles dismissing the selected result internally in
UrlbarController. Providers that have dismissable results should now implement
`onEngagement()` and call `controller.removeResult()`.
I made some other improvements to engagement handling. There's still room for
more but this patch is big enough already.
Other notable changes:
Include the engaged result in engagement notifications so providers have easy
access to it and can respond to clicks and dismissals more easily. That also
lets us stop passing `selIndex` and `provider` to `engagementEvent.record()`
since now it can compute those from the passed-in result.
Add the concept of `isSessionOngoing` to engagement notifications so providers
can tell whether an engagement ended the search session. Right now, providers
like quick suggest that record a bunch of provider-specific legacy telemetry
assume that `onEngagement()` ends the session, but that's no longer true.
Unify result buttons and result menu commands by setting
`element.dataset.command` on buttons (hopefully we can remove buttons soon, at
least the ones that aren't tip buttons)
Make sure we always notify providers on engagement even on dismissals or
when skipping legacy telemetry
Move dismissal of restyled search suggestions and history results from
`UrlbarController.handleDeleteEntry()` to the Places provider
Move dismissal of form history results from
`UrlbarController.handleDeleteEntry()` to the search suggestions provider
In the Places provider, remove the unused `_addSearchEngineMatch()` method. Also
remove the code in the "searchengine" case that creates a non-search-history
result. This code is unreached because the only time the provider creates a
"searchengine" match it also sets `isSearchHistory` to true.
In `UrlbarTestUtils.promiseAutocompleteResultPopup()`, change the default value
of the `fireInputEvent` param from false to true. This is necessary because
without a starting input event, the start event info in `engagementEvent` will
be null, so when `engagementEvent.record()` is called at the end of the
engagement, it will bail, and providers will not be notified of the engagement.
IMO true is a better default value anyway because input events will typically be
fired when the user performs a search.
Differential Revision: https://phabricator.services.mozilla.com/D174941
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 patch modifies existing Glean urlbar abandonment, engagement, and
impression events by including a weather suggestion.
This patch also adds a new telemetry scalar url.picked.weather to the legacy
telemetry system.
Differential Revision: https://phabricator.services.mozilla.com/D169225
Rather than checking every minute for work to do, use .exchange() to check
if sShouldStartFrecencyRecalculation switches to true and if so dispatch a
runnable to notify PlacesFrecencyRecalculator. The latter sets back
sShouldStartFrecencyRecalculation to false once the recalculation is complete.
Differential Revision: https://phabricator.services.mozilla.com/D171093
Rather than checking every minute for work to do, use .exchange() to check
if sShouldStartFrecencyRecalculation switches to true and if so dispatch a
runnable to notify PlacesFrecencyRecalculator. The latter sets back
sShouldStartFrecencyRecalculation to false once the recalculation is complete.
Differential Revision: https://phabricator.services.mozilla.com/D171093
Add an histogram to measure time to recalculate a chunk, and a scalar to measure
the number of pages that must be recalculated.
Differential Revision: https://phabricator.services.mozilla.com/D169203
This adds telemetry to UrlbarView that records the following things related to
the zero-prefix view (i.e., the topsites view):
* Exposures: How many times the ZP view was shown
* Engagements: How many times a result was picked in the ZP view
* Abandonments: How many times the user abandoned the ZP view
* Dwell time: How long the user was shown the ZP view
I considered adding telemetry specifically for topsites instead of the ZP view
as a whole, but since we have plans to start showing other types of results in
the ZP view, I don't think it's a good idea to rely on one specific type of
result as a proxy for the view itself. What DS and Product want to know about is
the view itself: how many times it was shown, for how long, etc.
This also adds one related scalar related to weather suggestions that counts the
number of times it's shown. This is the same scalar I added in D164778 using a
different, more complex approach.
Depends on D164615
Differential Revision: https://phabricator.services.mozilla.com/D165253
This adds the following scalars:
* `impression_weather`
* `click_weather`
* `help_weather`
* `block_weather`
And these histograms:
* `FX_URLBAR_MERINO_LATENCY_WEATHER_MS`
* `FX_URLBAR_MERINO_RESPONSE_WEATHER`
The histograms are updated in addition to the existing general Merino latency
and response histograms. I also modified the existing response histogram by
adding a new `no_suggestion` category so we can tell the difference between a
successful fetch with suggestions and a successful fetch without suggestions.
There's other telemetry in https://mozilla-hub.atlassian.net/browse/SNT-333 that
this doesn't add. I didn't want to do it all here since some of it is very
different. I'll file new bugs as necessary.
Other changes this makes:
* Factor out weather initialization from test_weather.js into MerinoTestUtils so
it can also be used in the new browser_telemetry_weather.js
* Copy `updateTopSites()` from the main urlbar head.js to quicksuggest's head.js
* Add some more `info()` logging to the telemetry helpers in head.js
Differential Revision: https://phabricator.services.mozilla.com/D164615
This makes a couple of large changes:
(1) "Generic" buttons (the ones added by `UrlbarView.#addRowButton()`) are now
supported in all row types. The help button that's currently included in some
types of rows when `result.payload.helpUrl` is defined is now supported for all
row types, and two additional button types are now supported too: block buttons
and labeled buttons. A row will get a block button if its
`result.payload.isBlockable` is defined. It will get a labeled button if
`result.payload.buttons` is defined and non-empty. A button can include a `url`
property that is then added as an attribute on the button's element, and
`UrlbarInput.pickResult()` will use this attribute to load the URL when the
button is picked.
(2) The reason I added labeled buttons is because it lets us support tip buttons
without much more effort, which then lets us get rid of the special row type
used for tips. With this patch, tips are now standard rows that use generic
buttons.
This approach should be compatible with the result menu, when we switch over to
it, because we can include the help and block commands in the menu when
`helpUrl` and `isBlockable` are defined, instead of creating buttons for them.
Labeled buttons -- the ones used in tips -- would still be created. The result
menu button itself can continue to be a generic button.
It should also be compatible with including the result menu button inside the
row selection. We'll still add buttons to `.urlbarView-row`, separate from
`.urlbarView-row-inner`, so that the buttons can continue to be on the right
side of the row. We can color the background of the row instead of the
row-inner.
As with D163630, my motivation for this change is to support generic buttons in
dynamic result rows so that help and block buttons can be easily added to
weather suggestions. Here too the larger changes of supporting generic labeled
buttons and removing special rows for tips aren't strictly necessary, but I took
the opportunity to rework things.
Finally, this makes a few other changes:
* It includes some of the more minor improvements to selection that I made in
D163630.
* It removes the help URL code from quick actions since it was decided not to
show a help button. Currently, the button is hidden in CSS, but now that a
generic help button is added for dynamic result rows when
`result.payload.helpUrl` is defined, `helpUrl` needs to be removed from the
payload to prevent a button from being added.
* I removed the special tip wrapping behavior, where the tip button and help
button would wrap below the tip's text. Instead, now the text wraps inside
row-inner and the buttons always remain on the same horizontal as the text. I
don't think it's worth the extra complication.
Differential Revision: https://phabricator.services.mozilla.com/D163766
This makes a couple of large changes:
(1) "Generic" buttons (the ones added by `UrlbarView.#addRowButton()`) are now
supported in all row types. The help button that's currently included in some
types of rows when `result.payload.helpUrl` is defined is now supported for all
row types, and two additional button types are now supported too: block buttons
and labeled buttons. A row will get a block button if its
`result.payload.isBlockable` is defined. It will get a labeled button if
`result.payload.buttons` is defined and non-empty. A button can include a `url`
property that is then added as an attribute on the button's element, and
`UrlbarInput.pickResult()` will use this attribute to load the URL when the
button is picked.
(2) The reason I added labeled buttons is because it lets us support tip buttons
without much more effort, which then lets us get rid of the special row type
used for tips. With this patch, tips are now standard rows that use generic
buttons.
This approach should be compatible with the result menu, when we switch over to
it, because we can include the help and block commands in the menu when
`helpUrl` and `isBlockable` are defined, instead of creating buttons for them.
Labeled buttons -- the ones used in tips -- would still be created. The result
menu button itself can continue to be a generic button.
It should also be compatible with including the result menu button inside the
row selection. We'll still add buttons to `.urlbarView-row`, separate from
`.urlbarView-row-inner`, so that the buttons can continue to be on the right
side of the row. We can color the background of the row instead of the
row-inner.
As with D163630, my motivation for this change is to support generic buttons in
dynamic result rows so that help and block buttons can be easily added to
weather suggestions. Here too the larger changes of supporting generic labeled
buttons and removing special rows for tips aren't strictly necessary, but I took
the opportunity to rework things.
Finally, this makes a few other changes:
* It includes some of the more minor improvements to selection that I made in
D163630.
* It removes the help URL code from quick actions since it was decided not to
show a help button. Currently, the button is hidden in CSS, but now that a
generic help button is added for dynamic result rows when
`result.payload.helpUrl` is defined, `helpUrl` needs to be removed from the
payload to prevent a button from being added.
* I removed the special tip wrapping behavior, where the tip button and help
button would wrap below the tip's text. Instead, now the text wraps inside
row-inner and the buttons always remain on the same horizontal as the text. I
don't think it's worth the extra complication.
Differential Revision: https://phabricator.services.mozilla.com/D163766
This makes a couple of large changes:
(1) "Generic" buttons (the ones added by `UrlbarView.#addRowButton()`) are now
supported in all row types. The help button that's currently included in some
types of rows when `result.payload.helpUrl` is defined is now supported for all
row types, and two additional button types are now supported too: block buttons
and labeled buttons. A row will get a block button if its
`result.payload.isBlockable` is defined. It will get a labeled button if
`result.payload.buttons` is defined and non-empty. A button can include a `url`
property that is then added as an attribute on the button's element, and
`UrlbarInput.pickResult()` will use this attribute to load the URL when the
button is picked.
(2) The reason I added labeled buttons is because it lets us support tip buttons
without much more effort, which then lets us get rid of the special row type
used for tips. With this patch, tips are now standard rows that use generic
buttons.
This approach should be compatible with the result menu, when we switch over to
it, because we can include the help and block commands in the menu when
`helpUrl` and `isBlockable` are defined, instead of creating buttons for them.
Labeled buttons -- the ones used in tips -- would still be created. The result
menu button itself can continue to be a generic button.
It should also be compatible with including the result menu button inside the
row selection. We'll still add buttons to `.urlbarView-row`, separate from
`.urlbarView-row-inner`, so that the buttons can continue to be on the right
side of the row. We can color the background of the row instead of the
row-inner.
As with D163630, my motivation for this change is to support generic buttons in
dynamic result rows so that help and block buttons can be easily added to
weather suggestions. Here too the larger changes of supporting generic labeled
buttons and removing special rows for tips aren't strictly necessary, but I took
the opportunity to rework things.
Finally, this makes a few other changes:
* It includes some of the more minor improvements to selection that I made in
D163630.
* It removes the help URL code from quick actions since it was decided not to
show a help button. Currently, the button is hidden in CSS, but now that a
generic help button is added for dynamic result rows when
`result.payload.helpUrl` is defined, `helpUrl` needs to be removed from the
payload to prevent a button from being added.
* I removed the special tip wrapping behavior, where the tip button and help
button would wrap below the tip's text. Instead, now the text wraps inside
row-inner and the buttons always remain on the same horizontal as the text. I
don't think it's worth the extra complication.
Differential Revision: https://phabricator.services.mozilla.com/D163766
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 adds a Nimbus variable corresponding to `browser.urlbar.merino.providers`.
I noticed that when we added a Nimbus variable for client variants (bug
1743685), we didn't actually use it in the provider. Instead we use its fallback
pref. I fixed that too.
Differential Revision: https://phabricator.services.mozilla.com/D159557
Please see the bug for info. This revision changes the URL of the adaptive
history doc to the new published doc. Ultimately this doc should live in-tree
but until then this is a nice, simple improvement.
Differential Revision: https://phabricator.services.mozilla.com/D150516
The documentation says this change was made in 103 because I'll request uplift
to Beta 103.
This also adds documentation for the new `sid` and `seq` Merino params from
D150289. I should have update the doc there, and since I'm modifying it now
anyway let's do it here.
Differential Revision: https://phabricator.services.mozilla.com/D150490
