Commit Graph

71 Commits

Author SHA1 Message Date
Drew Willcoxon
9f4378e671 Bug 1814732 - Move weather suggestions to their own provider. r=daisuke
This moves weather suggestions from the quick suggest provider to their own
provider. This will make it easier to implement weather suggestions that are
triggered by keyword instead of being shown on zero-prefix.

It does the following:

* Copies UrlbarProviderQuickSuggest.sys.mjs to UrlbarProviderWeather.sys.mjs
* Removes everything weather-related from UrlbarProviderQuickSuggest.sys.mjs
* Removes everything not weather-related from UrlbarProviderWeather.sys.mjs
* Makes some simplifications to the new provider since it doesn't need to
  support quick suggest suggestions
* Removes `result.payload.isWeather` since now we can use `result.providerName`
* This does *not* change any telemetry

Differential Revision: https://phabricator.services.mozilla.com/D168738
2023-02-07 02:26:54 +00:00
Dão Gottwald
a7c5be04f9 Bug 1814689 - Consolidate code handling UrlbarView overflowable elements. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D168699
2023-02-03 10:25:39 +00:00
mcheang
f26d2a59ce Bug 1811556 - Fix text fading in weather suggestion result and hide middot when high and low temperatures are wrapped. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D167416
2023-02-01 13:52:57 +00:00
Drew Willcoxon
b2d3b1dc40 Bug 1813155 - Quick suggest provider should not be active for search strings that contain only spaces. r=mak
I broke this in D167218. We didn't have a test for it.

Differential Revision: https://phabricator.services.mozilla.com/D168139
2023-01-30 16:45:32 +00:00
Dão Gottwald
d7994183f4 Bug 1811507 - Fix highlighting of dynamic type rows. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D167795
2023-01-27 19:21:10 +00:00
Drew Willcoxon
c81b3fa8ae Bug 1811373 - Cache weather suggestion l10n strings and allow L10nCache to cache strings only by ID. r=dao
This caches weather suggestion l10n strings by adding `cacheable: true` to the
view update object returned by the provider. Doing so hooks into UrlbarView's
dynamic result type functionality [here](https://searchfox.org/mozilla-central/rev/738b761bb2847f609f9cacc550680071cdc53637/browser/components/urlbar/UrlbarView.sys.mjs#1737-1739).

w/r/t l10n strings and caching, weather suggestions are a bit of a new case
because most of these strings take arguments that can't be known in advance and
that will change over time. For a string with arguments, L10nCache creates a
cache key by concating the string's ID and the values of its arguments. That
makes sense for strings whose values are things like search engine names, where
the set of possible argument values is small and where we may need to show
different translated strings when for example the search engine changes. For
those strings, we want to cache the translated strings separately using
different keys.

Weather suggestion strings like "20°C" are a different story. The ideal UX for
these strings is: While the UI is waiting for the string to be re-localized with
new argument values, it should show the previous localized string with the old
argument values. If the argument values have changed, there will still be some
flicker as the old values are replaced with the new ones, but it's the best we
can do, and at least there won't be empty space in the UI.

This isn't possible with L10nCache right now due to how it creates cache keys,
as mentioned earlier. So I added a new option that tells it to cache strings by
ID only, excluding argument values. That way only one translated string is
cached regardless of whatever argument values it was cached with.

Differential Revision: https://phabricator.services.mozilla.com/D167318
2023-01-23 17:49:55 +00:00
Drew Willcoxon
215545afbe Bug 1811018 - Don't show a weather suggestion when the search string contains spaces. r=daisuke
The weather suggestion shouldn't be shown when the search string contains only
spaces. We should check `queryContext.searchString` directly instead of the
trimmed search string. I implemented this wrong in D161410.

Differential Revision: https://phabricator.services.mozilla.com/D167218
2023-01-19 16:31:58 +00:00
mcheang
3be5c87967 Bug 1810250 - Add weather icons for weather suggestions in the urlbar.r=adw,desktop-theme-reviewers,dao
This patch receives a weather icon id from our merino server. We then use that
icon id and map it to a specific weather icon svg file within
urlbar-dyanmic-result.css. The icons colors are specified for light, dark, and
high contrast mode.

Differential Revision: https://phabricator.services.mozilla.com/D166848
2023-01-17 15:11:23 +00:00
Sandor Molnar
2a3754ea58 Backed out changeset dfce1bce524c (bug 1810250) for causing xpc failures in browser/components/urlbar/tests/quicksuggest/unit/test_weather.js CLOSED TREE 2023-01-16 23:59:30 +02:00
mcheang
3fccc60ccb Bug 1810250 - Add weather icons for weather suggestions in the urlbar.r=adw,desktop-theme-reviewers,dao DONTBUILD
This patch receives a weather icon id from our merino server. We then use that
icon id and map it to a specific weather icon svg file within
urlbar-dyanmic-result.css. The icons colors are specified for light, dark, and
high contrast mode.

Differential Revision: https://phabricator.services.mozilla.com/D166848
2023-01-16 19:59:19 +00:00
Dão Gottwald
e7a8bffbe0 Bug 1809705 - Implement "Learn more about Firefox Suggest" menu item in the urlbar result menu. r=adw,fluent-reviewers,flod
Differential Revision: https://phabricator.services.mozilla.com/D166578
2023-01-13 16:03:06 +00:00
Cristian Tuns
e148ca6903 Backed out changeset 03202cdedb1e (bug 1809705) for causing bc failures CLOSED TREE 2023-01-13 10:18:41 -05:00
Dão Gottwald
047b8c03dc Bug 1809705 - Implement "Learn more about Firefox Suggest" menu item in the urlbar result menu. r=adw,fluent-reviewers,flod
Differential Revision: https://phabricator.services.mozilla.com/D166578
2023-01-13 10:50:16 +00:00
Drew Willcoxon
21d32b4e51 Bug 1809201 - Use regionalPrefsLocales for the weather suggestions temperature unit. r=nanj
This replaces `Services.locale.appLocaleAsBCP47` with `regionalPrefsLocales[0]`
when determining the temperature unit to use for weather suggestions.

In summary, that means two things:

* When the language of the OS locale is the same as the language of the app's
  locale, weather suggestions will use the OS locale. e.g., if your OS locale is
  en-CA but your Firefox is en-US, weather will prefer en-CA since both locales
  are English, and so temperatures will be shown in C. This is a change from the
  current behavior, where they would be shown in F.
* When the user checked the "Use your operating system settings..." checkbox in
  about:preferences for unit formatting, weather suggestions will always use the
  OS locale, regardless of the app locale.

This is due to how `regionalPrefsLocales` works [1].

This revision also makes a couple of changes to code added in D166216:

* Instead of storing both C and F temperatures in the UrlbarResult payload,
  store only the user's appropriate temperature. This allows the xcpshell test
  (test_weather.js) to test locale behavior instead of having to do it in a
  browser test, and there's no reason not to do it anyway.
* Replace the hardcoded expected suggestion properties in test_weather.js with
  the ones from `WEATHER_SUGGESTION`, as was the case before D166216.

[1] `regionalPrefsLocales` is implemented [here](https://searchfox.org/mozilla-central/rev/d62c4c4d5547064487006a1506287da394b64724/intl/locale/LocaleService.cpp#485). If
`intl.regional_prefs.use_os_locales` is true, `regionalPrefsLocales` returns the
user's OS locales. The checkbox for this pref [is visible](https://searchfox.org/mozilla-central/rev/893a8f062ec6144c84403fbfb0a57234418b89cf/browser/components/preferences/main.js#1485-1491) only when the user's
primary OS locale doesn't match the app's primary locale. The full label for the
checkbox is [here](https://searchfox.org/mozilla-central/rev/d62c4c4d5547064487006a1506287da394b64724/browser/locales/en-US/browser/preferences/preferences.ftl#324). The pref defaults to false.

If `intl.regional_prefs.use_os_locales` is false, `regionalPrefsLocales` returns
the OS locales only if the OS locale's language is the same as the app locale's
language. Otherwise it returns the app's locales.

In either case, if an error is encountered, the app's locales are returned.

Differential Revision: https://phabricator.services.mozilla.com/D166722
2023-01-12 22:04:19 +00:00
mcheang
cd7ed9d357 Bug 1808974 - Client UI for weather suggestions. r=adw,flod
This patch implements the following:

It gets a weather result by calling `_makeWeatherResult` which calls our backend
Merino server. Based on the data returned by Merino, it parses through the
results to display the city, url, provider, weather summary, current, high, and
low temperatues to the user. It checks for a 0-prefix result to display the
weather. Lastly, it includes a top pick label for weather.

Differential Revision: https://phabricator.services.mozilla.com/D166216
2023-01-11 23:07:45 +00:00
Drew Willcoxon
51937fb742 Bug 1808697 - Allow Merino and remote settings suggestions to be passed separately to ensureQuickSuggestInit(). r=daleharvey
`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
2023-01-09 20:46:00 +00:00
Drew Willcoxon
bab02048db Bug 1806950 - Don't send custom engagement pings for navigational suggestions. r=daleharvey
This stops trying to send custom engagement pings for navigational
suggestions. That fixes the JS error that's due to the fact that these
suggestions don't have an advertiser. The purpose of these pings is to report
aggregate engagements to partners, and that's not applicable to navigational
suggestions, so not sending pings for these is OK.

I added browser_telemetry_navigationalSuggestions.js to ensure we're not sending
pings. This test will probably need to be substantially modified when we
implement all the required telemetry for navigational suggestions. That's
tracked in [this Jira ticket](https://mozilla-hub.atlassian.net/browse/SNT-337) and documented in the spec.

Until we implement that new telemetry, navigational suggestions are treated as
non-sponsored suggestions w/r/t telemetry, so I copied this file from
browser_telemetry_nonsponsored.js.

Depends on D164615

Differential Revision: https://phabricator.services.mozilla.com/D166019
2023-01-09 20:45:59 +00:00
Drew Willcoxon
9cb9133500 Bug 1806765 - Implement zero-prefix telemetry and add weather suggestion exposure telemetry. r=dao
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
2023-01-06 23:44:19 +00:00
Drew Willcoxon
4f60a88fed Bug 1804536 - Implement telemetry for weather suggestions. r=daleharvey
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
2023-01-06 16:41:49 +00:00
Dale Harvey
31c13c0cd4 Bug 1800993 - Add telemetry for dynamic wikipedia results. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D162253
2022-12-09 21:29:38 +00:00
Drew Willcoxon
05d8fae55e Bug 1804525 - Make weather suggestions blockable. r=daisuke
This makes the weather suggestion blockable. It depends on the new `isBlockable`
payload property added in D163766. The suggestion's row in the view will
automatically get a block button by setting that property.

We don't have an existing browser-chrome test for the block button by itself to
make sure picking it removes the row, so I added a task to
browser_remove_match.js.

The change to test_weather.js makes sure calling `blockResult()` on the provider
correctly disables the `suggest.weather` pref.

Depends on D163766

Differential Revision: https://phabricator.services.mozilla.com/D164120
2022-12-08 16:22:32 +00:00
Sandor Molnar
b46f787337 Backed out changeset ba7612a763a0 (bug 1800993) for needing data-review. CLOSED TREE 2022-12-08 00:55:42 +02:00
Dale Harvey
05f278cf30 Bug 1800993 - Add telemetry for dynamic wikipedia results. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D162253
2022-12-07 22:40:53 +00:00
Drew Willcoxon
212d13a054 Bug 1803873 - Support row buttons in all row types and make changes to tip rows. r=dao
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
2022-12-06 18:43:49 -05:00
Noemi Erli
5ffe6e3698 Backed out changeset e0eac08ef8bc (bug 1803873) fo causing failures in browser_search_telemetry_sources_navigation CLOSED TREE 2022-12-07 01:24:44 +02:00
Drew Willcoxon
f7f081c409 Bug 1803873 - Support row buttons in all row types and make changes to tip rows. r=dao
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
2022-12-06 22:28:55 +00:00
Cristian Tuns
63266cca48 Backed out changeset 263fffe843be (bug 1803873) for causing mochitest failures on browser_test_focus_urlbar.js CLOSED TREE 2022-12-06 15:10:09 -05:00
Drew Willcoxon
e634925c17 Bug 1803873 - Support row buttons in all row types and make changes to tip rows. r=dao
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
2022-12-06 16:35:31 +00:00
Mark Banner
0cd6f3836f Bug 1804037 - Convert PartnerLinkAttribution.jsm to an ES module. r=adw,search-reviewers,daleharvey
Differential Revision: https://phabricator.services.mozilla.com/D163814
2022-12-06 11:34:18 +00:00
Drew Willcoxon
26a76fee34 Bug 1799363 - Add weather suggestions to quick suggest. r=daisuke
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
2022-11-17 05:04:07 +00:00
Drew Willcoxon
05db5ab826 Bug 1800810 - Always record visible quick suggest results in engagement telemetry. r=dao
This is a follow up to D161866 and effectively reverts and replaces it with a
different approach. Please see bug 1800810 for background. In short, engagement
telemetry should be based on the result that's visible in the view, not on the
result the provider added last.

D161866 fixed the case where the last-added result is in the view but hidden.
There's another case we need to handle, when the last-added result is not in the
view at all. That can happen when you type something and hit enter really
quickly, before the view can update. I was able to trigger it several times.
When that happens, there are two possibilities:

* No quick suggest result is visible in the view. `result.rowIndex` is its
  initial value of -1, so we record an engagement for a result that is not
  visible and that doesn't have a valid index.

* A quick suggest result from a previous query is visible in the view. Here
  again, `result.rowIndex` is -1 so we record an engagement for a result that is
  not visible and that doesn't have a valid index, and we miss recording an
  engagement for the result that's actually visible.

Right now it's not easy to fix this inside the provider because providers don't
know anything about the view(s). I could record this telemetry in the view but
that's not its role. I think it makes sense to include the view in the query
context, so that's what this does. I added `view.visibleResults` to make getting
the visible results easy.

Daisuke's new Glean telemetry in D160193 uses `context.results`, which has this
same problem of not being based on actually visible results. We'll need to use
`context.view.visibleResults` there too, and there may be other existing cases
as well.

Depends on D161866

Differential Revision: https://phabricator.services.mozilla.com/D162182
2022-11-16 21:47:55 +00:00
Cristian Tuns
0fdc1821ce Backed out changeset 053963c15ffe (bug 1799363) for causing xpcshell failures on test_weather.js CLOSED TREE 2022-11-16 14:00:59 -05:00
Drew Willcoxon
48330effac Bug 1799363 - Add weather suggestions to quick suggest. r=daisuke
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
2022-11-16 17:59:23 +00:00
Drew Willcoxon
843f50bea3 Bug 1800184 - Don't record quick suggest impression telemetry when the result is hidden. r=dao
This adds `result.isVisible`. Like `rowIndex`, it's updated by the view.
UrlbarProviderQuickSuggest will skip impression telemetry updates (and
impression stats updates too) when it's false.

Please see bug 1800184 for background.

Differential Revision: https://phabricator.services.mozilla.com/D161866
2022-11-15 17:05:22 +00:00
Dale Harvey
f3224cf163 Bug 1797328 - Handle 'is_top_pick' being defined in merino results. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D160240
2022-11-08 11:03:42 +00:00
Drew Willcoxon
9c36ab2fc3 Bug 1799264 - Refactor QuickSuggest features [Part 2]: Port blocked suggestions to BaseFeature. r=daisuke
This ports blocked suggestions to `BaseFeature`.

Please see bug 1799264 for details.

Depends on D161366

Differential Revision: https://phabricator.services.mozilla.com/D161367
2022-11-08 05:42:09 +00:00
Drew Willcoxon
3871ea4423 Bug 1799264 - Refactor QuickSuggest features [Part 1]: Add BaseFeature and port impression caps to it. r=daisuke
This adds a new `BaseFeature` class that quick suggest features can extend.
`BaseFeature` encapsulates the logic for deciding whether a feature should be
enabled. Subclasses override a few methods and getters to describe their own
logic and to do initialization and uninitialization.

Any time a Nimbus variable (or a variable fallback pref) changes, QuickSuggest
iterates over each feature and enables or disables it appropriately.

As a first step, I ported impression caps to `BaseFeature`. Later patches in
this stack will port other features.

Please see bug 1799264 for details.

Differential Revision: https://phabricator.services.mozilla.com/D161366
2022-11-08 05:42:09 +00:00
Drew Willcoxon
be65e71e30 Bug 1798595 - Refactor quick suggest [Part 6]: Minor improvements. r=daisuke
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
2022-11-02 06:50:15 +00:00
Drew Willcoxon
16aaeab517 Bug 1798595 - Refactor quick suggest [Part 5]: Move remote settings code into QuickSuggestRemoteSettingsClient and initialization into QuickSuggest. r=daisuke
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
2022-11-02 06:50:15 +00:00
Drew Willcoxon
209873a1d6 Bug 1798595 - Refactor quick suggest [Part 4]: Move help URL from UrlbarProviderQuickSuggest to QuickSuggest and change how the test utils are initialized. r=daisuke
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
2022-11-02 06:50:14 +00:00
Drew Willcoxon
df0f74d429 Bug 1798595 - Refactor quick suggest [Part 2]: Move Nimbus exposure event recording from UrlbarQuickSuggest to QuickSuggest. r=daisuke
This moves Nimbus exposure event recording out of UrlbarQuickSuggest in
preparation for making UrlbarQuickSuggest strictly related to remote settings.

Please see bug 1798595 for details.

Depends on D160982

Differential Revision: https://phabricator.services.mozilla.com/D160983
2022-11-02 06:50:14 +00:00
Drew Willcoxon
53f88e7241 Bug 1798595 - Refactor quick suggest [Part 1]: Move core code out of UrlbarProviderQuickSuggest into a new QuickSuggest module. r=daisuke
This does the following:

* Copies UrlbarProviderQuickSuggest.sys.mjs to a new QuickSuggest.sys.mjs file
* Removes everything related to UrlbarProvider from QuickSuggest
* Removes most everything not related to UrlbarProvider from
  UrlbarProviderQuickSuggest
* Updates consumers to use the new QuickSuggest module

Please see bug 1798595 for details.

Differential Revision: https://phabricator.services.mozilla.com/D160982
2022-11-02 06:50:13 +00:00
Drew Willcoxon
b5ead527af Bug 1797673 - Factor out Merino code into MerinoClient and add MerinoTestUtils. r=daisuke
This factors out the Merino client code from UrlbarProviderQuickSuggest into
MerinoClient. It also adds MerinoTestUtils.

There are a few reasons for this:

1. Weather suggestions will be prefetched from Merino, and it may make sense to
   do the prefetching outside of UrlbarProviderQuickSuggest, I'm not sure
   yet.
2. Weather suggestions will be fetched from a slightly different URL from other
   suggestions. They'll need to include `providers=weather` as a search param.
   In the future there may be other types of suggestions that also require
   different parameters or whole new endpoints. Supporting options like this is
   a little nicer with a dedicated Merino client class.
3. In general this code is substantial enough that it makes sense to encapsulate
   it in its own file and class, even if the provider is the only thing that
   will use it.

This is a large patch but a lot of it moves code around. Summary of changes:

* Factor out Merino client code into MerinoClient
* Also include some new useful helpers in MerinoClient
* Consolidate and factor out Merino test code into MerinoTestUtils
* Also include some new useful helpers in MerinoTestUtils and make other improvements
* Add a MockMerinoServer class (in MerinoTestUtils.sys.mjs) to help with setting up a mock Merino server
* Add test_merinoClient.js to test the client in isolation. This file started as a copy of test_quicksuggest_merino.js because it's so similar, and some of the tests in the latter are moved to the former.
* Add test_merinoClient_sessions.js to test Merino sessions in isolation. Similar to the previous point, this file started as a copy of test_quicksuggest_merinoSessions.js.
* Modify existing Merino tests so they use MerinoTestUtils

Differential Revision: https://phabricator.services.mozilla.com/D160449
2022-10-28 21:25:55 +00:00
trickypr
4b9c23b007 Bug 1510561 - Part 7: Apply plugin:mozilla/require-jsdoc to browser/components/urlbar. r=Standard8
Differential Revision: https://phabricator.services.mozilla.com/D159472
2022-10-24 13:32:38 +00:00
trickypr
f28fa5aa80 Bug 1510561 - Part 6: Apply plugin:mozilla/valid-jsdoc to browser/components/urlbar. r=Standard8
Differential Revision: https://phabricator.services.mozilla.com/D159471
2022-10-24 13:32:37 +00:00
Drew Willcoxon
d7aafbd006 Bug 1795803 - Expose the Merino provider pref to Nimbus. r=daisuke
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
2022-10-18 19:58:53 +00:00
Mark Banner
5407bdffa8 Bug 1792341 - Migrate more toolkit/modules consumers to use direct ES module import. r=Gijs,webdriver-reviewers,perftest-reviewers,necko-reviewers,geckoview-reviewers,preferences-reviewers,application-update-reviewers,pip-reviewers,credential-management-reviewers,sgalich,owlish,bytesized,AlexandruIonescu,whimboo,mconley,mixedpuppy
Mainly automated changes. Some manual ESLint fixes and whitespace cleanup.

Differential Revision: https://phabricator.services.mozilla.com/D158452
2022-10-18 11:21:26 +00:00
Mark Banner
79b9557a71 Bug 1792398 - Enable ESLint rule 'strict' on mjs files as the directive is not necessary for modules. r=arai,pip-reviewers
Differential Revision: https://phabricator.services.mozilla.com/D158115
2022-09-26 21:47:50 +00:00
Marian-Vasile Laza
b7f6194b8d Backed out changeset 0679274d6ed5 (bug 1792398) for causing bc failures on browser_sendQuery.js. CLOSED TREE 2022-09-26 22:53:00 +03:00
Mark Banner
5dcae1ed44 Bug 1792398 - Enable ESLint rule 'strict' on mjs files as the directive is not necessary for modules. r=arai,pip-reviewers
Differential Revision: https://phabricator.services.mozilla.com/D158115
2022-09-26 18:51:57 +00:00