The problem is the row remains selected after its content is replaced with the
dismissal acknowledgment tip. This patch clears the selection and selects the
tip's "Got it" button if the row was selected.
I didn't add a general browser test for feedback and dismissal acknowledgments
earlier, so I added one now. I cp'ed the weather test and used it as a starting
point since it does test the acknowledgments but is specific to the weather
suggestion. Even though some of it now duplicates the new test, I left it intact
because we still need a test for the weather-specific commands, and the amount
of duplicated code isn't very much.
I modified the test helper function that opens the result menu so that the row
is not selected when `byMouse` is true. That way I can make sure the selection
doesn't change when the dismissed row is not selected. I used
`EventUtils.promiseElementReadyForUserInput()` to synthesize a mousemove over
the row to make the menu button visible. That function has a bug where the
interval is added after `onHit` is called and the promise is resolved, leaving
the interval ticking forever. Moving the `synthesizeMouseAtCenter()` after the
timeout is created fixes the problem.
Differential Revision: https://phabricator.services.mozilla.com/D178264
This does two things:
* Modify `nsXULPopupManager::ShowPopup()` so it calls `ShowPopupAsNativeMenu()`
as long as an anchor wasn't passed in, and only on Mac.
* Modify `-[MOZMenuOpeningCoordinator _openMenu:atScreenPosition:forView:withAppearance:]`
so it also takes a `aIsContextMenu` param. If the param is true, we synthesize
a right-click event and pop up a context menu as usual. If it's false, we use
`-[NSMenu popUpMenuPositioningItem:atLocation:inView:]` instead.
The reason this works is because `-[NSMenu popUpMenuPositioningItem:atLocation:inView:]`
opens the menu in a sensible place when the x-y coords are near the right edge
of the screen. In contrast, `+[NSMenu popUpContextMenu:withEvent:forView:]` will
anchor the menu's top-right corner to the mouse cursor when near the right edge.
Differential Revision: https://phabricator.services.mozilla.com/D177355
This increments the minimum keyword length when the user clicks the "Show less
frequently" command for the weather suggestion. It adds a pref to keep track of
the current min length. If the pref is zero, we use the min length in Nimbus or
remote settings.
There is a limit to the number of times "Show less frequently" can be clicked.
This patch calls it the cap. Once the cap is reached, the min length can't be
incremented any more and the command is not shown in the menu again. The cap can
be set in Nimbus and remote settings.
This patch also modifies UrlbarPrefs by making it possible to remove observers.
`Weather` needs to listen for changes to the `weather.minKeywordLength` pref,
and when the weather feature is disabled, it needs to stop listening.
Depends on D175827
Differential Revision: https://phabricator.services.mozilla.com/D177218
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 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
Some random thoughts:
* This is a minimal implementation - there were plans to expand this with autocomplete support that we never got around to
* I built this just from reading docs + poking around in the urlbar code, so there might be a more desireable way to do all of this
* We've gone back and forth between calling this "contextual search" and "site specific search," we should probably pick a name and stick to it
* There were never any design specs for this so the UI/UX was based off of existing urlbar results as they appeared in Flowstate
* There were some open questions around how we treat installing opensearch engines, since how they are handled here seems to differ from how we use them elsewhere
* I don't remember the reasoning for showing this result last in the urlbar, we may want to move it elsewhere
Differential Revision: https://phabricator.services.mozilla.com/D159052
