mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1539804 - Quantumbar: Re-enable browser_urlbarStopSearchOnSelection.js and fix a couple of related problems. r=mak 

This test uncovered a couple of problems:

(1) UrlbarController.handleKeyNavigation relies on event.defaultPrevented to tell whether the one-offs handled the key event. That's a problem when combined with deferring the down arrow key.

handleKeyNavigation is called twice in that case. The first time, the event is deferred (so executeAction = false), and handleKeyNavigation calls event.preventDefault. The second time, the event is being replayed, but defaultPrevented is true from the previous call regardless of whether the one-offs actually handled the event.

So handleKeyNavigation always returns early because it thinks the one-offs always handled the event, so it never properly replays down arrow keys.

(2) UrlbarProviderUnifiedComplete's query promise is never resolved when the query is canceled. That's a problem in general of course but I tripped over it in this test because I need to check results after the query is canceled, and the test ended up hanging since UrlbarTestUtils waits for the query to finish in order to get its results.

It's not a problem in UnifiedComplete itself per se because of course awesomebar uses UnifiedComplete too, and it doesn't have this problem. The difference is that nsAutoCompleteController::StopSearch calls input->OnSearchComplete() (via PostSearchCleanup): https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/toolkit/components/autocomplete/nsAutoCompleteController.cpp#1433

Quantumbar's UnifiedComplete provider is missing that behavior, so this patch adds it by resolving its query promise when the query is canceled.

Differential Revision: https://phabricator.services.mozilla.com/D29300

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Drew Willcoxon 2019-05-01 17:42:19 +00:00
Родитель caa48376ad
Коммит 70e0b1acf5
6 изменённых файлов: 83 добавлений и 45 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

8
browser/components/search/content/search-one-offs.js
Просмотреть файл

@ -829,8 +829,8 @@ class SearchOneOffs {
* it also handles Up/Down keys that cross the boundaries between list
* items and the one-off buttons.
*
* If this method handles the key press, then event.defaultPrevented will
* be true when it returns.
* If this method handles the key press, then it will call
* event.preventDefault() and return true.
*
* @param {Event} event
* The key event.
@ -849,10 +849,11 @@ class SearchOneOffs {
* restored to the value that the user typed. Pass that value here.
* However, if you pass true for allowEmptySelection, you don't need
* to pass anything for this parameter. (Pass undefined or null.)
* @returns {boolean} True if the one-offs handled the key press.
*/
handleKeyPress(event, numListItems, allowEmptySelection, textboxUserValue) {
if (!this.popup) {
return;
return false;
}
let handled = this._handleKeyPress(event, numListItems,
allowEmptySelection,
@ -861,6 +862,7 @@ class SearchOneOffs {
event.preventDefault();
event.stopPropagation();
}
return handled;
}
_handleKeyPress(event, numListItems, allowEmptySelection, textboxUserValue) {

4
browser/components/urlbar/UrlbarController.jsm
Просмотреть файл

@ -258,12 +258,12 @@ class UrlbarController {
if (this.view.isOpen && executeAction) {
let queryContext = this._lastQueryContext;
if (queryContext) {
this.view.oneOffSearchButtons.handleKeyPress(
let handled = this.view.oneOffSearchButtons.handleKeyPress(
event,
queryContext.results.length,
this.view.allowEmptySelection,
queryContext.searchString);
if (event.defaultPrevented) {
if (handled) {
return;
}
}

5
browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
Просмотреть файл

@ -127,10 +127,12 @@ class ProviderUnifiedComplete extends UrlbarProvider {
addCallback(UrlbarProviderUnifiedComplete, match);
}
if (done) {
delete this._resolveSearch;
resolve();
}
},
};
this._resolveSearch = resolve;
unifiedComplete.startSearch(queryContext.searchString,
params.join(" "),
null, // previousResult
@ -150,6 +152,9 @@ class ProviderUnifiedComplete extends UrlbarProvider {
// This doesn't properly support being used concurrently by multiple fields.
this.queries.delete(queryContext);
unifiedComplete.stopSearch();
if (this._resolveSearch) {
this._resolveSearch();
}
}
}

1
browser/components/urlbar/tests/browser/browser.ini
Просмотреть файл

@ -166,7 +166,6 @@ support-files =
skip-if = (os == "linux" || os == "mac") && debug # bug 970052, bug 970053
[browser_urlbarStop.js]
[browser_urlbarStopSearchOnSelection.js]
skip-if = true # Bug 1524510 - Not implemented yet.
support-files =
searchSuggestionEngineSlow.xml
searchSuggestionEngine.sjs

108
browser/components/urlbar/tests/browser/browser_urlbarStopSearchOnSelection.js
Просмотреть файл

@ -13,7 +13,7 @@ const TEST_ENGINE_BASENAME = "searchSuggestionEngineSlow.xml";
// This should match the `timeout` query param used in the suggestions URL in
// the test engine.
const TEST_ENGINE_SUGGESTIONS_TIMEOUT = 1000;
const TEST_ENGINE_SUGGESTIONS_TIMEOUT = 3000;
// The number of suggestions returned by the test engine.
const TEST_ENGINE_NUM_EXPECTED_RESULTS = 2;
@ -36,48 +36,80 @@ add_task(async function init() {
});
add_task(async function mainTest() {
// Trigger an initial search. Restrict matches to search suggestions.
await promiseAutocompleteResultPopup(`${UrlbarTokenizer.RESTRICT.SEARCH} test`, window);
await promiseSuggestionsPresent("Waiting for initial suggestions");
// Open a tab that will match the search string below so that we're guaranteed
// to have more than one result (the heuristic result) so that we can change
// the selected result. We open a tab instead of adding a page in history
// because open tabs are kept in a memory SQLite table, so open-tab results
// are more likely than history results to be fetched before our slow search
// suggestions. This is important when the test runs on slow debug builds on
// slow machines.
await BrowserTestUtils.withNewTab("http://example.com/", async () => {
await BrowserTestUtils.withNewTab("about:blank", async () => {
// Do an initial search. There should be 4 results: heuristic, open tab,
// and the two suggestions.
await promiseAutocompleteResultPopup("amp");
await TestUtils.waitForCondition(() => {
return UrlbarTestUtils.getResultCount(window) ==
2 + TEST_ENGINE_NUM_EXPECTED_RESULTS;
});
// Now synthesize typing a character. promiseAutocompleteResultPopup doesn't
// work in this case because it causes the popup to close and re-open with the
// new input text.
await new Promise(r => EventUtils.synthesizeKey("x", {}, window, r));
// Type a character to start a new search. The new search should still
// match the open tab so that the open-tab result appears again.
EventUtils.synthesizeKey("l");
// At this point, a new search starts, the autocomplete's _invalidate is
// called, _appendCurrentResult is called, and matchCount remains 3 from the
// previous search (the heuristic result plus the two search suggestions).
// The suggestion results should not outwardly change, however, since the
// autocomplete controller should still be returning the initial suggestions.
// What has changed, though, is the search string, which is kept in the
// results' ac-text attributes. Call waitForAutocompleteResultAt to wait for
// the results' ac-text to be set to the new search string, "testx", to make
// sure the autocomplete has seen the new search.
await waitForAutocompleteResultAt(TEST_ENGINE_NUM_EXPECTED_RESULTS);
// There should be 2 results immediately: heuristic and open tab. (On
// quantumbar, there will be only 2, but on awesomebar, there will be 4
// since awesomebar will keep showing the excess old results for a bit.)
await TestUtils.waitForCondition(() => {
return UrlbarTestUtils.getResultCount(window) >= 2;
});
// Now press the Down arrow key to change the selection.
await new Promise(r => EventUtils.synthesizeKey("VK_DOWN", {}, window, r));
// Before the search completes, change the selected result. Pressing only
// the down arrow key ends up selecting the first one-off on Linux debug
// builds on the infrastructure for some reason, so arrow back up to
// select the heuristic result again. The important thing is to change
// the selection. It doesn't matter which result ends up selected.
EventUtils.synthesizeKey("KEY_ArrowDown");
EventUtils.synthesizeKey("KEY_ArrowUp");
// Changing the selection should have stopped the new search triggered by
// typing the "x" character. Wait a bit to make sure it really stopped.
await new Promise(r => setTimeout(r, 2 * TEST_ENGINE_SUGGESTIONS_TIMEOUT));
// Wait for the new search to complete. It should be canceled due to the
// selection change, but it should still complete.
await promiseSearchComplete();
// Both of the suggestion results should retain their initial values,
// "testfoo" and "testbar". They should *not* be "testxfoo" and "textxbar".
// To make absolutely sure the suggestions don't appear after the search
// completes, wait a bit.
await new Promise(r => setTimeout(r, 1 + TEST_ENGINE_SUGGESTIONS_TIMEOUT));
// + 1 for the heuristic result
let numExpectedResults = TEST_ENGINE_NUM_EXPECTED_RESULTS + 1;
Assert.equal(UrlbarTestUtils.getResultCount(window), numExpectedResults,
"Should have the correct number of results displayed");
// The heuristic result should reflect the new search, "ampl".
let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
Assert.equal(result.type, UrlbarUtils.RESULT_TYPE.SEARCH,
"Should have the correct result type");
Assert.equal(result.searchParams.query, "ampl",
"Should have the correct query");
let expectedSuggestions = ["testfoo", "testbar"];
for (let i = 0; i < TEST_ENGINE_NUM_EXPECTED_RESULTS; i++) {
// + 1 to skip the heuristic result
let result = await UrlbarTestUtils.getDetailsOfResultAt(window, i + 1);
Assert.equal(result.type, UrlbarUtils.RESULT_TYPE.SEARCH,
"Should have the correct result type");
Assert.equal(result.searchParams.suggestion, expectedSuggestions[i],
"Should have the correct suggestion");
}
// None of the other results should be "ampl" suggestions, i.e., amplfoo
// and amplbar should not be in the results.
let count = UrlbarTestUtils.getResultCount(window);
for (let i = 1; i < count; i++) {
if (!UrlbarPrefs.get("quantumbar")) {
// On awesomebar, UrlbarTestUtils.getDetailsOfResultAt() can throw due
// to its urlbar.controller.getFinalCompleteValueAt() call. That's
// because the results at this point may contain results from two
// different searches. This only seems to happen on --verify runs.
try {
gURLBar.controller.getFinalCompleteValueAt(i);
} catch (ex) {
info("getFinalCompleteValueAt exception: " + ex);
continue;
}
}
result = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
Assert.ok(
result.type != UrlbarUtils.RESULT_TYPE.SEARCH ||
!["amplfoo", "amplbar"].includes(result.searchParams.suggestion),
"Suggestions should not contain the typed l char"
);
}
});
});
});

2
browser/components/urlbar/tests/browser/searchSuggestionEngineSlow.xml
Просмотреть файл

@ -4,7 +4,7 @@
<SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
<ShortName>searchSuggestionEngineSlow.xml</ShortName>
<Url type="application/x-suggestions+json" method="GET" template="http://mochi.test:8888/browser/browser/components/urlbar/tests/browser/searchSuggestionEngine.sjs?query={searchTerms}&amp;timeout=1000"/>
<Url type="application/x-suggestions+json" method="GET" template="http://mochi.test:8888/browser/browser/components/urlbar/tests/browser/searchSuggestionEngine.sjs?query={searchTerms}&amp;timeout=3000"/>
<Url type="text/html" method="GET" template="http://mochi.test:8888/" rel="searchform">
<Param name="terms" value="{searchTerms}"/>
</Url>