Bug 1301093 - Part 3: fix delayed handleEnter regression. r=adw
MozReview-Commit-ID: 1SZeMUtLjNg
--- a/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
+++ b/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
@@ -36,8 +36,55 @@ add_task(function* test_sametext() {
event.initEvent("input", true, true);
gURLBar.dispatchEvent(event);
EventUtils.synthesizeKey("VK_RETURN", {});
info("wait for the page to load");
yield BrowserTestUtils.browserLoaded(gBrowser.selectedTab.linkedBrowser,
false, "http://example.com/");
});
+
+add_task(function* test_after_empty_search() {
+ yield promiseAutocompleteResultPopup("");
+ gURLBar.focus();
+ gURLBar.value = "e";
+ EventUtils.synthesizeKey("x", {});
+ EventUtils.synthesizeKey("VK_RETURN", {});
+
+ info("wait for the page to load");
+ yield BrowserTestUtils.browserLoaded(gBrowser.selectedTab.linkedBrowser,
+ false, "http://example.com/");
+});
+
+add_task(function* test_disabled_ac() {
+ // Disable autocomplete.
+ let suggestHistory = Preferences.get("browser.urlbar.suggest.history");
+ Preferences.set("browser.urlbar.suggest.history", false);
+ let suggestBookmarks = Preferences.get("browser.urlbar.suggest.bookmark");
+ Preferences.set("browser.urlbar.suggest.bookmark", false);
+ let suggestOpenPages = Preferences.get("browser.urlbar.suggest.openpage");
+ Preferences.set("browser.urlbar.suggest.openpages", false);
+
+ Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
+ "http://example.com/?q={searchTerms}");
+ let engine = Services.search.getEngineByName("MozSearch");
+ let originalEngine = Services.search.currentEngine;
+ Services.search.currentEngine = engine;
+
+ registerCleanupFunction(function* () {
+ Preferences.set("browser.urlbar.suggest.history", suggestHistory);
+ Preferences.set("browser.urlbar.suggest.bookmark", suggestBookmarks);
+ Preferences.set("browser.urlbar.suggest.openpage", suggestOpenPages);
+
+ Services.search.currentEngine = originalEngine;
+ let engine = Services.search.getEngineByName("MozSearch");
+ Services.search.removeEngine(engine);
+ });
+
+ gURLBar.focus();
+ gURLBar.value = "e";
+ EventUtils.synthesizeKey("x", {});
+ EventUtils.synthesizeKey("VK_RETURN", {});
+
+ info("wait for the page to load");
+ yield BrowserTestUtils.browserLoaded(gBrowser.selectedTab.linkedBrowser,
+ false, "http://example.com/?q=ex");
+});
--- a/browser/base/content/test/urlbar/browser_canonizeURL.js
+++ b/browser/base/content/test/urlbar/browser_canonizeURL.js
@@ -11,16 +11,23 @@ var pairs = [
["example.foo/bar ", "http://example.foo/bar"],
["1.1.1.1", "http://1.1.1.1/"],
["ftp://example", "ftp://example/"],
["ftp.example.bar", "http://ftp.example.bar/"],
["ex ample", Services.search.defaultEngine.getSubmission("ex ample", null, "keyword").uri.spec],
];
add_task(function*() {
+ // Disable autoFill for this test, since it could mess up the results.
+ let autoFill = Preferences.get("browser.urlbar.autoFill");
+ Preferences.set("browser.urlbar.autoFill", false);
+ registerCleanupFunction(() => {
+ Preferences.set("browser.urlbar.autoFill", autoFill);
+ });
+
for (let [inputValue, expectedURL] of pairs) {
let focusEventPromise = BrowserTestUtils.waitForEvent(gURLBar, "focus");
let messagePromise = BrowserTestUtils.waitForMessage(gBrowser.selectedBrowser.messageManager,
"browser_canonizeURL:start");
let stoppedLoadPromise = ContentTask.spawn(gBrowser.selectedBrowser, [inputValue, expectedURL],
function([inputValue, expectedURL]) {
return new Promise(resolve => {
--- a/browser/base/content/test/urlbar/head.js
+++ b/browser/base/content/test/urlbar/head.js
@@ -5,16 +5,18 @@ XPCOMUtils.defineLazyModuleGetter(this,
XPCOMUtils.defineLazyModuleGetter(this, "Task",
"resource://gre/modules/Task.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesTestUtils",
"resource://testing-common/PlacesTestUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TabCrashHandler",
"resource:///modules/ContentCrashHandlers.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
+ "resource://gre/modules/Preferences.jsm");
function waitForCondition(condition, nextTest, errorMsg, retryTimes) {
retryTimes = typeof retryTimes !== 'undefined' ? retryTimes : 30;
var tries = 0;
var interval = setInterval(function() {
if (tries >= retryTimes) {
ok(false, errorMsg);
moveOn();
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -127,17 +127,17 @@ file, You can obtain one at http://mozil
<field name="_value">""</field>
<field name="gotResultForCurrentQuery">false</field>
<!--
This is set around HandleHenter so it can be used in handleCommand.
It is also used to track whether we must handle a delayed handleEnter,
by checking if it has been cleared.
-->
- <field name="searchStringOnHandleEnter">""</field>
+ <field name="handleEnterInstance">null</field>
<!--
For performance reasons we want to limit the size of the text runs we
build and show to the user.
-->
<field name="textRunsMaxLen">255</field>
<!--
@@ -388,16 +388,20 @@ file, You can obtain one at http://mozil
event &&
event.altKey &&
!isTabEmpty(gBrowser.selectedTab);
where = altEnter ? "tab" : "current";
}
}
let url = this.value;
+ if (!url) {
+ return;
+ }
+
let mayInheritPrincipal = false;
let postData = null;
let lastLocationChange = gBrowser.selectedBrowser.lastLocationChange;
let matchLastLocationChange = true;
let action = this._parseActionUrl(url);
if (action) {
switch (action.type) {
@@ -453,35 +457,34 @@ file, You can obtain one at http://mozil
this._recordSearchEngineLoad(selectedOneOff.engine, value,
event, where, openUILinkParams);
this._loadURL(url, postData, where, openUILinkParams,
matchLastLocationChange, mayInheritPrincipal);
return;
}
}
- this._canonizeURL(event, response => {
- [url, postData, mayInheritPrincipal] = response;
+ url = this._canonizeURL(event, url);
+ getShortcutOrURIAndPostData(url).then(({url, postData, mayInheritPrincipal}) => {
if (url) {
matchLastLocationChange =
- lastLocationChange ==
- gBrowser.selectedBrowser.lastLocationChange;
+ lastLocationChange == gBrowser.selectedBrowser.lastLocationChange;
this._loadURL(url, postData, where, openUILinkParams,
matchLastLocationChange, mayInheritPrincipal);
}
});
]]></body>
</method>
<property name="oneOffSearchQuery">
<getter><![CDATA[
// this.textValue may be an autofilled string. Search only with the
// portion that the user typed, if any, by preferring the autocomplete
- // controller's searchString (including searchStringOnHandleEnter).
- return this.searchStringOnHandleEnter ||
+ // controller's searchString (including handleEnterInstance.searchString).
+ return (this.handleEnterInstance && this.handleEnterInstance.searchString) ||
this.mController.searchString ||
this.textValue;
]]></getter>
</property>
<method name="_loadURL">
<parameter name="url"/>
<parameter name="postData"/>
@@ -568,23 +571,19 @@ file, You can obtain one at http://mozil
.maybeRecordTelemetry(event, openUILinkWhere, openUILinkParams);
let submission = engine.getSubmission(query, null, "keyword");
return [submission.uri.spec, submission.postData];
]]></body>
</method>
<method name="_canonizeURL">
<parameter name="aTriggeringEvent"/>
- <parameter name="aCallback"/>
+ <parameter name="aUrl"/>
<body><![CDATA[
- var url = this.value;
- if (!url) {
- aCallback(["", null, false]);
- return;
- }
+ let url = aUrl;
// Only add the suffix when the URL bar value isn't already "URL-like",
// and only if we get a keyboard event, to match user expectations.
if (/^\s*[^.:\/\s]+(?:\/.*|\s*)$/i.test(url) &&
(aTriggeringEvent instanceof KeyEvent)) {
let accel = this.AppConstants.platform == "macosx" ?
aTriggeringEvent.metaKey :
aTriggeringEvent.ctrlKey;
@@ -625,19 +624,17 @@ file, You can obtain one at http://mozil
} else {
url = url + suffix;
}
url = "http://www." + url;
}
}
- getShortcutOrURIAndPostData(url).then(data => {
- aCallback([data.url, data.postData, data.mayInheritPrincipal]);
- });
+ return url;
]]></body>
</method>
<field name="_contentIsCropped">false</field>
<method name="_initURLTooltip">
<body><![CDATA[
if (this.focused || !this._contentIsCropped)
@@ -1039,21 +1036,24 @@ file, You can obtain one at http://mozil
// However, if the default result is automatically selected, we
// ensure that it corresponds to the current input.
// Store the current search string so it can be used in
// handleCommand, which will be called as a result of
// mController.handleEnter().
// Note this is also used to detect if we should perform a delayed
// handleEnter, in such a case it won't have been cleared.
- this.searchStringOnHandleEnter = this.mController.searchString;
+ this.handleEnterInstance = {
+ searchString: this.mController.searchString,
+ event: event
+ };
if (this.popup.selectedIndex != 0 || this.gotResultForCurrentQuery) {
let rv = this.mController.handleEnter(false, event);
- this.searchStringOnHandleEnter = "";
+ this.handleEnterInstance = null;
return rv;
}
return true;
]]></body>
</method>
<method name="handleDelete">
@@ -1758,45 +1758,58 @@ file, You can obtain one at http://mozil
</method>
<method name="onResultsAdded">
<body>
<![CDATA[
// If nothing is selected yet, select the first result if it is a
// pre-selected "heuristic" result. (See UnifiedComplete.js.)
if (this.selectedIndex == -1 && this._isFirstResultHeuristic) {
- // Don't handle this as a user-initiated action.
- this._ignoreNextSelect = true;
-
// Don't fire DOMMenuItemActive so that screen readers still see
// the input as being focused.
this.richlistbox.suppressMenuItemEvent = true;
-
this.selectedIndex = 0;
this.richlistbox.suppressMenuItemEvent = false;
- this._ignoreNextSelect = false;
}
this.input.gotResultForCurrentQuery = true;
// Check if we should perform a delayed handleEnter.
- if (this.input.searchStringOnHandleEnter) {
+ if (this.input.handleEnterInstance) {
try {
// Safety check: handle only if the search string didn't change.
- if (this.input.mController.searchString == this.input.searchStringOnHandleEnter) {
- this.input.mController.handleEnter(false);
+ let instance = this.input.handleEnterInstance;
+ if (this.input.mController.searchString == instance.searchString) {
+ this.input.mController.handleEnter(false, instance.event);
}
} finally {
- this.input.searchStringOnHandleEnter = "";
+ this.input.handleEnterInstance = null;
}
}
]]>
</body>
</method>
+ <method name="_onSearchBegin">
+ <body><![CDATA[
+ // Set the selected index to 0 (heuristic) until a result comes back
+ // and we can evaluate it better.
+ //
+ // This is required to properly manage delayed handleEnter:
+ // 1. if a search starts we set selectedIndex to 0 here, and it will
+ // be updated by onResultsAdded. Since selectedIndex is 0,
+ // handleEnter will delay the action if a result didn't arrive yet.
+ // 2. if a search doesn't start (for example if autocomplete is
+ // disabled), this won't be called, and the selectedIndex will be
+ // the default -1 value. Then handleEnter will know it should not
+ // delay the action, cause a result wont't ever arrive.
+ this.selectedIndex = 0;
+ ]]></body>
+ </method>
+
</implementation>
<handlers>
<handler event="SelectedOneOffButtonChanged"><![CDATA[
this._selectedOneOffChanged();
]]></handler>
<handler event="mousedown"><![CDATA[
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -1626,16 +1626,20 @@ nsAutoCompleteController::ProcessResult(
uint32_t delta = totalMatchCount - oldRowCount;
mRowCount += delta;
if (mTree) {
mTree->RowCountChanged(oldRowCount, delta);
}
}
+ // Try to autocomplete the default index for this search.
+ // Do this before invalidating so the binding knows about it.
+ CompleteDefaultIndex(aSearchIndex);
+
// Refresh the popup view to display the new search results
nsCOMPtr<nsIAutoCompletePopup> popup;
input->GetPopup(getter_AddRefs(popup));
NS_ENSURE_TRUE(popup != nullptr, NS_ERROR_FAILURE);
popup->Invalidate(nsIAutoCompletePopup::INVALIDATE_REASON_NEW_RESULT);
uint32_t minResults;
input->GetMinResultsForPopup(&minResults);
@@ -1643,20 +1647,18 @@ nsAutoCompleteController::ProcessResult(
// Make sure the popup is open, if necessary, since we now have at least one
// search result ready to display. Don't force the popup closed if we might
// get results in the future to avoid unnecessarily canceling searches.
if (mRowCount || !minResults) {
OpenPopup();
} else if (mSearchesOngoing == 0) {
ClosePopup();
}
- }
-
- if (searchResult == nsIAutoCompleteResult::RESULT_SUCCESS ||
- searchResult == nsIAutoCompleteResult::RESULT_SUCCESS_ONGOING) {
+ } else if (searchResult == nsIAutoCompleteResult::RESULT_SUCCESS ||
+ searchResult == nsIAutoCompleteResult::RESULT_SUCCESS_ONGOING) {
// Try to autocomplete the default index for this search.
CompleteDefaultIndex(aSearchIndex);
}
return NS_OK;
}
nsresult
@@ -1735,20 +1737,21 @@ nsAutoCompleteController::CompleteDefaul
return NS_OK;
bool shouldComplete;
input->GetCompleteDefaultIndex(&shouldComplete);
if (!shouldComplete)
return NS_OK;
nsAutoString resultValue;
- if (NS_SUCCEEDED(GetDefaultCompleteValue(aResultIndex, true, resultValue)))
+ if (NS_SUCCEEDED(GetDefaultCompleteValue(aResultIndex, true, resultValue))) {
CompleteValue(resultValue);
- mDefaultIndexCompleted = true;
+ mDefaultIndexCompleted = true;
+ }
return NS_OK;
}
nsresult
nsAutoCompleteController::GetDefaultCompleteResult(int32_t aResultIndex,
nsIAutoCompleteResult** _result,
int32_t* _defaultIndex)
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1045,16 +1045,20 @@ extends="chrome://global/content/binding
return val;
]]>
</setter>
</property>
<method name="onSearchBegin">
<body><![CDATA[
this.richlistbox.mouseSelectedIndex = -1;
+
+ if (typeof this._onSearchBegin == "function") {
+ this._onSearchBegin();
+ }
]]></body>
</method>
<method name="openAutocompletePopup">
<parameter name="aInput"/>
<parameter name="aElement"/>
<body>
<![CDATA[