The previous approach was autofilling partial subdomains from search engines.
Having a search engine pointing to "developer.mozilla.org, when "moz" was typed
we ended up autofilling "moz[illa.org]" but pointing to "developer.mozilla.org".
Unfortunately that made impossible to actually go to "mozilla.org" by typing it.
The new approach instead allows tab-to-search results to appear even if there's
no autofill, because the provider checks autonomously the autofill threshold.
There's a downside though, with the old approach we could return more partial
matches, and the Muxer would pick one, depending on the autofilled host. For
example we could return "rover.ebay.com" and match it to "eb[ay.it]".
With the new approach we don't have an autofilled host, and fetching it would
be expensive, we can only compare the search string with the engine's host.
For this reason the patch also tries to partially match against the searchForm
host, that often points to a page the user visits more often.
Differential Revision: https://phabricator.services.mozilla.com/D96942
This patch adds tab-to-search results, partially reusing the existing code in `UrlbarProviderAutofill._matchSearchEngineDomain`. Tests are included. Telemetry is included in a child revision.
This patch doesn't add the "Search <engine name> directly from the address bar." action text. Our current action text code l10n is in a .properties file and I thought adding branching translation logic in UrlbarView would be a mess. I think Marco might be converting that .properties file to Fluent in bug 1658629. If he is, I'll rebase my patch on his and add the new action text it in this patch. If he isn't, I'll file a follow-up for converting that .properties file and adding the tab-to-search action text, which we can address before preffing `update2.tabToComplete` on. Either way, I think this is ready for a first-round review.
This patch is based on D91076 and D91077. It doesn't have any functional dependency, but there are some conflicts, especially in the added telemetry.
Differential Revision: https://phabricator.services.mozilla.com/D91468
Summary of major changes:
* Bookmarks, history, and tabs restriction chars now enter search mode. I added
a method to UrlbarProviderHeuristicFallback to return a result with a keyword
when one of these is used.
* This fixes other bugs like recognizing aliases that are entered at the
beginning of non-empty search strings, and not quasi-re-entering search mode
when search mode is already entered and you type another alias.
* The heuristic now determines whether we enter search mode, similar to how it
also determines whether we autofill. When the heuristic has a keyword but no
keyword offer, and the keyword is one of the recognized search mode keywords,
then we enter search mode, cancel the current query, and start a new query
with the remainder of the search string after the keyword.
* I slightly changed how we detect an alias, but only when update2 is
enabled. Now, an alias must be followed by a space; otherwise, the alias is
not recognized and instead just remains part of the seach string. Because if
we don't do that, then you end up in a strange situation after typing an alias
but before pressing space: The heuristic says "Search with <engine with the
alias>", but we haven't entered search mode yet because you haven't typed a
space yet. This is true for both @aliaes and non-@aliases.
* A consequence of the previous point is that we can still autofill @aliases
with a trailing space, which IMO is important. Then, once the user types any
char (space or not), we immediately enter search mode with the query being
whatever char they typed. This is less important after bug 1658605 landed, but
it's still good to have.
* Previously, `UrlbarView.onQueryResults` called UrlbarInput in order to
autofill after the first result is received. This is circuitous becaue the
input already has an `onFirstResult` method, which I now use to enter search
mode when appropriate. So I moved the autofill call from UrlbarView to
`UrlbarInput.onFirstResult`.
* As I mentioned, I improved some test framework and simplified some related
product (non-test) code. For example:
* I removed `UrlbarUtils.KEYWORD_OFFER.NONE` in favor of just leaving
`keywordOffer` as `undefined`.
* `tailOffsetIndex` can now be `undefined` if it's not relevant.
* I removed empty-string `icon` properties from payloads in favor of
`undefined`.
* In tests, I ignore `undefined` but present properties in payloads so they
don't count when comparing payloads with `deepEqual`.
* We weren't previously comparing `result.source` and `result.type` in
xpcshell tests, and that's important IMO, so I added checks for those and
updated tests.
* `isSearchHistory` is redundant, so I removed it. For form history, we
should be checking `result.source == HISTORY` and `result.type == SEARCH`.
* A bunch of tests needed to be updated for this new behavior.
Differential Revision: https://phabricator.services.mozilla.com/D87944
This excludes the heuristic for empty searches when in search mode. I haven't
heard back from Verdi yet about excluding it for all searches in search mode. We
can add that in a follow-up if necessary.
Since we're now excluding the heuristic but we want the view to remain open,
it's possible for the view to be empty while it's open. I had to make some
changes to allow that to happen. There are three cases I want to call out:
1. When the search string is empty, the view shows top sites. If you then enter
search mode and the resulting search doesn't return any results, we
previously closed the view. This patch keeps it open. An example of this
scenario is when you don't have any bookmarks and you click the bookmarks
one-off.
2. When the urlbar isn't focused, it's in search mode, the input is empty, and
the search didn't return any results, then focusing the urlbar didn't do
anything previously. This patch auto-opens the view.
3. When the input contains a single char and it's in search mode, deleting the
char previously closed the view if the empty search didn't return any
results. This patch keeps the view open.
When the view is empty, we also need to hide the one-offs' top border that
usually separates them from the results. Otherwise there are two separator lines
right next to each other, the one above the one-offs and the one at the bottom
edge of the input. I don't think there's any CSS selector that will let us
easily do this due to the DOM structure, so I added a new `noresults` attribute
on the view for this.
I had to call `context.searchString.trim()` to tell whether the search string is
empty. Since we do that in a bunch of places, I added
`context.trimmedSearchString`, and a lot of this patch is replacing those `trim`
calls.
Differential Revision: https://phabricator.services.mozilla.com/D86908
The old query was picking a random bookmarked status out of the pages in an origin,
depending on the physical position of the origin rows in the table.
The new query uses window functions to partition the data by origin, so it can
query origins just once and sum www and non-www origin's frecency scores before
comparing to the autofill threshold.
Differential Revision: https://phabricator.services.mozilla.com/D84657
* Replace PlacesSearchAutocompleteProvider with UrlbarSearchUtils.
* Move the module from toolkit to browser. The only consumers of
PlacesSearchAutocompleteProvider are urlbar and UnifiedComplete.
* I'd like to add functions to UrlbarUtils instead, but
PlacesSearchAutocompleteProvider adds itself as an observer for search
engine changes, and it keeps some state so that alias lookups are O(1). It
has an init function to set that up. That's not quite as easy to do if I
just added some functions to UrlbarUtils, and I think that O(1) lookups of
aliases are worth keeping (vs. O(number of installed engines)), so I kept a
separate module, now called UrlbarSearchUtils.
* Init the search service (via UrlbarSearchUtils) from
UrlbarProvidersManager.startQuery so that every module involved in querying
doesn't need to do it.
* Remove PlacesSearchAutocompleteProvider.currentEngine. Previous consumers can
simply use Services.search directly now that the service is initialized early
(see previous point).
* Remove PlacesSearchAutocompleteProvider.parseSubmissionURL. Here again
consumers can use Services.search directly.
Differential Revision: https://phabricator.services.mozilla.com/D79244
By returning false (or not adding results to `context.results`), the muxer
signals that it needs more results in order to decide how to sort them and to
sort them without flickering. After the providers manager calls each provider's
`startQuery`, it removes the provider from `context.activeProviders` and calls
`_notifyResultsFromProvider` one more time so that the muxer can resort if
necessary given the new `context.activeProviders`.
We can also be a little faster and more efficient by making only two passes over
the results, and by keeping a `context.heuristicResult` property that's updated
by the providers manager.
This also fixes a bug with the search suggestions provider, where it didn't
include a suggestion that matched the user's search string even if the heuristic
result was not a search result (e.g., if it's an autofill). That deduping logic
should be done in the muxer.
Finally this removes browser_restyleSearches.js since the muxer changes removed
part of the restyling functionality. I'll file a follow-up to remove other parts
of restyling.
Differential Revision: https://phabricator.services.mozilla.com/D74192
Replace `isRestricting` with `getPriority` -- i.e., replace the binary restricting system with a general priority system. Instead of choosing restricting providers, the providers manager chooses the highest-priority providers.
Differential Revision: https://phabricator.services.mozilla.com/D60093
Replace `isRestricting` with `getPriority` -- i.e., replace the binary restricting system with a general priority system. Instead of choosing restricting providers, the providers manager chooses the highest-priority providers.
Differential Revision: https://phabricator.services.mozilla.com/D60093
Adds a `browser.urlbar.onEngagement` event. Listeners are passed the current engagement state: start, engagement, abandonment, or discard. The extension could use this to record its own parallel telemetry (scalars, event telemetry, etc.) per engagement.
Differential Revision: https://phabricator.services.mozilla.com/D53897
Adds a new event listener to `browser.urlbar` called `onResultPicked`. This event is fired for tip results when they don't specify a URL. Hypothetically it could be fired for any type of result that didn't specify a URL, but that's only tips for now.
The listener is passed two arguments: the payload of the result that was picked, and a "details" object whose properties depend on the type of result. For tips, details is `{ helpPicked }`, where `helpPicked` is true if the help button was picked and false if the main button was picked.
Differential Revision: https://phabricator.services.mozilla.com/D46254
* In nsAutoCompleteController, the logic that determines whether the new search is a prefix of the old search is only done in HandleText, i.e., on input, not when the value is set programmatically.
* That logic is a lot more complex in nsAutoCompleteController.
* nsAutoCompleteController autofills in one case where quantumbar doesn't: when completing the "placeholder" string before starting a new search and waiting for the async results (thereby preventing flicker).
* Some nsAutoCompleteController state gets reset each time the awesomebar is focused (see calls to attachController() in the autocomplete binding, which sets the controller's input, which calls ResetInternalState()). That state is important in regard to autofill and the placeholder string. If it's not reset, then the autofill of one search will incorrectly affect the autofill of a later search.
Differential Revision: https://phabricator.services.mozilla.com/D22306
This is a rollup of all the patches that have landed on the cedar project branch:
891252fdd0
Bug 1492475 - Part 1: Migrate most, if not all nsSearchService consumers to use async APIs. r=florian
79b2eb2367
Bug 1492475 - Part 2: Move nsIBrowserSearchService.idl to toolkit/components/search/nsISearchService.idl and update references. r=florian
a947d3cdf0
Bug 1492475 - Part 3: The search service init() method should simply return a Promise. r=florian
c1e172dfac
Bug 1492475 - Part 4: Remove the synchronous initialization flow. r=florian
cd41189eac
Bug 1492475 - Part 5: Since async initialization of the search service now is implicit behavior, remove the distinctive verbiage used internally. r=florian
2ae7189dfa
Bug 1492475 - Part 6: Update the cache build task to work with an actual Promise and re-initialize only once at the same time - all to fix race conditions here. r=florian
c8ee92973f
Bug 1492475 - Part 7: Make the region fetch not block the init flow, to ensure it's as fast as possible. r=florian
c44e674e16
Bug 1492475 - Part 8: Introduce an init flag, which can only be used privately, that allows to explicitly skip waiting for the region check process to complete. r=florian
6c79eaf1d3
Bug 1492475 - Part 9: Update unit tests to stop using 'currentEngine', in favor of 'defaultEngine'. r=Standard8
21b3aa17ee
Bug 1492475 - Part 10: Update unit tests to be fully aware of the new, async signatures of the search service API and remove sync init flow tests. r=mkaply,florian
ce5ba69019
Bug 1492475 - Part 11: Repair incorrect usage of the `identifier` property of nsISearchEngine instances. r=florian
fd177a7994
Bug 1518543 - Fix up the Android (Fennec) nsISearchService shim to work with the new asynchronous API. r=florian
3653d8ee22
Bug 1523708 - Change the search service interaction in the show-heartbeat action to use the new async API. r=florian
Differential Revision: https://phabricator.services.mozilla.com/D18355
***
Bug 1514594: Part 3a - Change ChromeUtils.import to return an exports object; not pollute global. r=mccr8
This changes the behavior of ChromeUtils.import() to return an exports object,
rather than a module global, in all cases except when `null` is passed as a
second argument, and changes the default behavior not to pollute the global
scope with the module's exports. Thus, the following code written for the old
model:
ChromeUtils.import("resource://gre/modules/Services.jsm");
is approximately the same as the following, in the new model:
var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
Since the two behaviors are mutually incompatible, this patch will land with a
scripted rewrite to update all existing callers to use the new model rather
than the old.
***
Bug 1514594: Part 3b - Mass rewrite all JS code to use the new ChromeUtils.import API. rs=Gijs
This was done using the followng script:
https://bitbucket.org/kmaglione/m-c-rewrites/src/tip/processors/cu-import-exports.jsm
***
Bug 1514594: Part 3c - Update ESLint plugin for ChromeUtils.import API changes. r=Standard8
Differential Revision: https://phabricator.services.mozilla.com/D16747
***
Bug 1514594: Part 3d - Remove/fix hundreds of duplicate imports from sync tests. r=Gijs
Differential Revision: https://phabricator.services.mozilla.com/D16748
***
Bug 1514594: Part 3e - Remove no-op ChromeUtils.import() calls. r=Gijs
Differential Revision: https://phabricator.services.mozilla.com/D16749
***
Bug 1514594: Part 3f.1 - Cleanup various test corner cases after mass rewrite. r=Gijs
***
Bug 1514594: Part 3f.2 - Cleanup various non-test corner cases after mass rewrite. r=Gijs
Differential Revision: https://phabricator.services.mozilla.com/D16750
