Commit graph

209 commits

Author SHA1 Message Date
Chris H-C
6bf60bf55d Bug 1868580 - Remove PingCentre from Contextual Services r=nanj,perry.mcmanis
As of 11amPT December 7, Contextual Services is now looking exclusively at
Glean-sent data for Firefox versions 116+. (See DSRE-1489)

Differential Revision: https://phabricator.services.mozilla.com/D195910
2023-12-11 19:21:37 +00:00
Drew Willcoxon
d0b6a673f5 Bug 1858553 - Remove the merinoEnabled Nimbus variable and fallback pref. r=daisuke
Depends on D190745

Differential Revision: https://phabricator.services.mozilla.com/D190746
2023-10-12 15:50:50 +00:00
Drew Willcoxon
1dd0c84b18 Bug 1858549 - Remove the quickSuggestRemoteSettingsEnabled Nimbus variable and fallback pref. r=daisuke
Depends on D190742

Differential Revision: https://phabricator.services.mozilla.com/D190745
2023-10-12 15:50:50 +00:00
Drew Willcoxon
da6386c22c Bug 1857391 - Remove Firefox Suggest "best match" as its own separate feature. r=daisuke,settings-reviewers
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
2023-10-11 17:17:06 +00:00
Karandeep
5b311b33e2 Bug 1853910 - Remove Legacy Event Telemetry Dependencies. r=mak,extension-reviewers,robwu
Depends on D188721

Differential Revision: https://phabricator.services.mozilla.com/D189558
2023-10-11 07:45:13 +00:00
Karandeep
64d72a4b26 Bug 1853910 - Remove Legacy Event Telemetry. r=mak
Differential Revision: https://phabricator.services.mozilla.com/D188721
2023-10-11 07:45:13 +00:00
Stanca Serban
270068fd8d Backed out 2 changesets (bug 1853910) for causing mochitests failures in browser_ext_urlbar_attributionURL.js. CLOSED TREE
Backed out changeset eea288462cbb (bug 1853910)
Backed out changeset 75543d7d4250 (bug 1853910)
2023-10-07 00:17:27 +03:00
Karandeep
76eb0e17eb Bug 1853910 - Remove Legacy Event Telemetry Dependencies. r=mak,extension-reviewers,robwu
Differential Revision: https://phabricator.services.mozilla.com/D189558
2023-10-06 12:33:14 +00:00
Karandeep
b5b7981be6 Bug 1853910 - Remove Legacy Event Telemetry. r=mak
Differential Revision: https://phabricator.services.mozilla.com/D188721
2023-10-06 12:33:14 +00:00
Drew Willcoxon
b551362822 Bug 1854755 - Add suggested_index and suggested_index_relative_to_group to Suggest pings. r=nanj
This adds two new fields to the Suggest PingCentre and Glean pings:

* `suggested_index` -
  An integer value that is the intended index of the suggestion
  being interacted with. If `suggested_index_relative_to_group` is true, the
  index is relative to the "Firefox Suggest" group; otherwise the index is
  relative to the entire list of suggestions. Non-negative values (starting
  at 0) are relative to the start/top of the group/list; negative values are
  relative to the end/bottom of the group/list.
* `suggested_index_relative_to_group` -
  Whether `suggested_index` is relative to the "Firefox Suggest" group. If
  false, it is relative to the entire list of suggestions.

In the Glean ping, `suggested_index` is stringified because there's no integer
metric type that can take negative numbers.

These two values reflect how the implementation works and will let us record any
indexes we want to experiment with. Suggestions are usually shown at the bottom
of the Firefox Suggest group, and in that case these values will be:

```
suggested_index = -1
suggested_index_relative_to_group = true
```

If we want to show suggestions at the top of the Firefox Suggest group instead
of the bottom, the values will be:

```
suggested_index = 0
suggested_index_relative_to_group = true
```

Differential Revision: https://phabricator.services.mozilla.com/D189035
2023-09-26 02:48:30 +00:00
Karandeep
7c27382e27 Bug 1842247 - Integrate clipboard result feature with existing telemetry for success measurement. r=mak
Differential Revision: https://phabricator.services.mozilla.com/D187046
2023-09-15 14:16:16 +00:00
mcheang
6df14d2939 Bug 1840558 - Remove autofill search engines code. r=mak
Differential Revision: https://phabricator.services.mozilla.com/D182163
2023-06-29 12:36:08 +00:00
James Teow
3c95fa07c8 Bug 1840376 - Remove outstanding references to Preloaded Top URLs - r=adw
Differential Revision: https://phabricator.services.mozilla.com/D182195
2023-06-29 08:18:27 +00:00
James Teow
93e78407ca Bug 1466103 - Add telemetry for adaptive history results in the address bar - r=mak
Differential Revision: https://phabricator.services.mozilla.com/D180168
2023-06-22 23:36:23 +00:00
James Teow
6093cc7f6f Bug 1837239 - Remove the Preloaded Top URLs feature - r=adw
Differential Revision: https://phabricator.services.mozilla.com/D180246
2023-06-13 18:27:12 +00:00
Drew Willcoxon
cd732d535b Bug 1836903 - Implement Pocket suggestions. r=daisuke,desktop-theme-reviewers,dao
This implements most parts of Pocket suggestions. They don't need any special UI
or a dynamic result type because they're only shown as the usual best match rows
or non-best match rows.

Still to do:

* Implement the "Show less frequently" behavior once we decide what the keywords
  will be and how that will work.
* Implement the bottom "Mozilla Pocket" text inside the suggestion row once it's
  finalized. We can use the same technique we use to show the "Sponsored" bottom
  text for adM suggestions.

Other changes this makes:

* Replace the `type=bestmatch` attribute with an `bestmatch` attribute. That
  lets best-match rows have a `type` too, in this case `"pocket"` (actually
  either `"rs_pocket"` or `"merino_pocket"`, since I'm using the telemetry
  result type).
* Improve how UrlbarProviderQuickSuggest delegates to individual features when
  getting result commands, view updates, handling commands, etc., so that we
  don't need to add new `case` statements for each new type of suggestion.

Differential Revision: https://phabricator.services.mozilla.com/D180059
2023-06-07 20:19:44 +00:00
Drew Willcoxon
3545495040 Bug 1831971 - Remove zero-prefix functionality from the weather suggestion and don't start fetches until a config is set. r=daisuke
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
2023-05-09 06:37:34 +00:00
Norisz Fay
5cf0725f32 Backed out changeset a39f9cd81a42 (bug 1831971) for causing bc failures on browser_glean_telemetry_engagement_selected_result.js CLOSED TREE 2023-05-09 09:14:25 +03:00
Drew Willcoxon
1ab4707494 Bug 1831971 - Remove zero-prefix functionality from the weather suggestion and don't start fetches until a config is set. r=daisuke
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
2023-05-09 04:28:47 +00:00
Sylvestre Ledru
a5ecda1e47 Bug 1827596 - Fix UrlbarTestUtils reference link in 'Adress Bar/Testing'
Differential Revision: https://phabricator.services.mozilla.com/D176784
2023-04-29 07:59:55 +00:00
Marc Seibert
0c6ac66e1f Bug 1736939 - Re-add "Search history" item to history (sub) views from history and library buttons. r=dao,fluent-reviewers,flod
Differential Revision: https://phabricator.services.mozilla.com/D175653
2023-04-27 14:32:46 +00:00
mcheang
2fff7f53fc Bug 1829880 - Fix broken links in search lifecycle docs and add the most up to date perma links. r=scunnane
Differential Revision: https://phabricator.services.mozilla.com/D176425
2023-04-26 22:52:30 +00:00
Drew Willcoxon
6829a78aa2 Bug 1827943 - Implement the weather suggestion result menu UI. r=dao,fluent-reviewers,flod
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
2023-04-18 16:22:56 +00:00
Drew Willcoxon
a3b0a1fd4c Bug 1827762 - Replace UrlbarProvider.pickResult() and blockResult() with onEngagement() r=mak
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
2023-04-13 06:03:33 +00:00
Dale Harvey
f5338ca629 Bug 1823543 - Add telemetry to distinguish trending results. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D173093
2023-03-28 13:28:44 +00:00
James Teow
34cbd31c4c Bug 1815971 - Count number of times persisted search is viewed and reverted due to Popups - r=adw
Differential Revision: https://phabricator.services.mozilla.com/D172690
2023-03-22 13:59:06 +00:00
Noah
3e5c4be1ba Bug 1821392 - Change Search and Address Bar telemetry docs to use titles for the probe names. r=Standard8
Differential Revision: https://phabricator.services.mozilla.com/D172203
2023-03-10 12:31:59 +00:00
Drew Willcoxon
34b853d0d1 Bug 1819797 - Update telemetry for navigational suggestions and dynamic Wikipedia and add subtypes to quick suggest results. r=daleharvey
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
2023-03-08 01:11:02 +00:00
mcheang
c85c0172e0 Bug 1815699 - Modify existing Glean telemetry and add new telemetry to account for weather as a suggestion. r=adw
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
2023-03-01 19:06:49 +00:00
Marco Bonardo
c9738d3a71 Bug 1818881 - Avoid polling for frecency recalcs. r=florian,daisuke
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
2023-02-28 12:57:19 +00:00
Sandor Molnar
52b16d91f5 Backed out changeset 6596733ddc41 (bug 1818881) for causing xpc failures in toolkit/components/search/tests/xpcshell/test_searchSuggest_cookies.js CLOSED TREE 2023-02-28 13:31:40 +02:00
Marco Bonardo
11246a7aca Bug 1818881 - Avoid polling for frecency recalcs. r=florian,daisuke
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
2023-02-28 10:11:43 +00:00
Marco Bonardo
dbd7c88d1f Bug 1818549 - Update frecency recalculation documentation. r=mcheang
Reformat special values and fields with double backticks.
Add additional details about the recalculation.

Differential Revision: https://phabricator.services.mozilla.com/D170785
2023-02-24 13:50:23 +00:00
Dão Gottwald
78dcb2f4d0 Bug 1818245 - Consolidate dynamic attribute setting code in UrlbarView. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D170612
2023-02-24 07:16:56 +00:00
Marco Bonardo
3ab87cc3c1 Bug 1765552 - Add ranking documentation to Address Bar source docs. r=mcheang
Differential Revision: https://phabricator.services.mozilla.com/D157934
2023-02-22 18:08:47 +00:00
ogiorgis
690fadc913 Bug 1810702 - remove circular reference warnings r=firefox-source-docs-reviewers,sylvestre DONTBUILD
Moreover it fixes the behavior of the right menu.

Depends on D170178

Differential Revision: https://phabricator.services.mozilla.com/D170179
2023-02-17 12:38:03 +00:00
Daisuke Akatsuka
41a47644eb Bug 1815342: Take telemetry of the user action that deletes the autofilled string. r=mak
Differential Revision: https://phabricator.services.mozilla.com/D169172
2023-02-09 21:55:22 +00:00
Marco Bonardo
dd273b0ca7 Bug 1811209 - Add telemetry to measure frecency recalculation. r=daisuke
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
2023-02-09 14:38:22 +00:00
Dale Harvey
c3782e1e63 Bug 1806024 - Add telemetry for QuickAction inpressions. r=adw
Differential Revision: https://phabricator.services.mozilla.com/D166611
2023-02-02 10:12:13 +00:00
Drew Willcoxon
774cdbe2fc 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
ae86a3014b 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
Daisuke Akatsuka
963abc662a Bug 1806524: Add links of telemetry doc built from metrics.yaml to telemetry.rst r=adw,mak
Differential Revision: https://phabricator.services.mozilla.com/D165378
2022-12-26 01:24:21 +00:00
Dale Harvey
3c8fccf233 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
Sandor Molnar
0492225328 Backed out changeset ba7612a763a0 (bug 1800993) for needing data-review. CLOSED TREE 2022-12-08 00:55:42 +02:00
Dale Harvey
6a3c3db00e 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
43b648c36b 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
42fd111277 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
b080ee4b3a 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
1942c132b0 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
a2e3d559b4 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