Bug 1283329 - Perceptible pause when quickly typing and hitting enter in the urlbar. r=adw
MozReview-Commit-ID: 2NdTvY9H25Z
--- a/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
+++ b/browser/base/content/test/urlbar/browser_autocomplete_enter_race.js
@@ -64,27 +64,59 @@ add_task(function* test_disabled_ac() {
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* () {
+ function* cleanup() {
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);
- });
+ if (engine) {
+ Services.search.removeEngine(engine);
+ }
+ }
+ registerCleanupFunction(cleanup);
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");
+ yield cleanup();
});
+
+add_task(function* test_delay() {
+ const TIMEOUT = 10000;
+ // Set a large delay.
+ let delay = Preferences.get("browser.urlbar.delay");
+ Preferences.set("browser.urlbar.delay", TIMEOUT);
+
+ registerCleanupFunction(function* () {
+ Preferences.set("browser.urlbar.delay", delay);
+ });
+
+ // This is needed to clear the current value, otherwise autocomplete may think
+ // the user removed text from the end.
+ let start = Date.now();
+ yield promiseAutocompleteResultPopup("");
+ Assert.ok((Date.now() - start) < TIMEOUT);
+
+ start = Date.now();
+ gURLBar.closePopup();
+ 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/");
+ Assert.ok((Date.now() - start) < TIMEOUT);
+});
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1499,17 +1499,17 @@ file, You can obtain one at http://mozil
<body><![CDATA[
if (this.mPopupOpen) {
return;
}
this.mInput = aInput;
this.selectedIndex = this._isFirstResultHeuristic ? 0 : -1;
this.view = aInput.controller.QueryInterface(Components.interfaces.nsITreeView);
- this.invalidate();
+ this._invalidate();
var rect = window.document.documentElement.getBoundingClientRect();
var width = rect.right - rect.left;
this.setAttribute("width", width);
// Adjust the direction of the autocomplete popup list based on the textbox direction, bug 649840
var popupDirection = aElement.ownerDocument.defaultView.getComputedStyle(aElement).direction;
this.style.direction = popupDirection;
@@ -1769,25 +1769,26 @@ file, You can obtain one at http://mozil
this.selectedIndex = 0;
this.richlistbox.suppressMenuItemEvent = false;
}
this.input.gotResultForCurrentQuery = true;
// Check if we should perform a delayed handleEnter.
if (this.input.handleEnterInstance) {
- try {
+ let instance = this.input.handleEnterInstance;
+ this.input.handleEnterInstance = null;
+ // Don't handle this immediately or we could cause a recursive
+ // loop where the controller sets popupOpen and re-enters here.
+ setTimeout(() => {
// Safety check: handle only if the search string didn't change.
- let instance = this.input.handleEnterInstance;
if (this.input.mController.searchString == instance.searchString) {
this.input.mController.handleEnter(false, instance.event);
}
- } finally {
- this.input.handleEnterInstance = null;
- }
+ }, 0);
}
]]>
</body>
</method>
<method name="_onSearchBegin">
<body><![CDATA[
// Set the selected index to 0 (heuristic) until a result comes back
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -891,25 +891,31 @@ Search.prototype = {
queries.push(this._switchToTabQuery);
}
queries.push(this._searchQuery);
// Add the first heuristic result, if any. Set _addingHeuristicFirstMatch
// to true so that when the result is added, "heuristic" can be included in
// its style.
this._addingHeuristicFirstMatch = true;
- yield this._matchFirstHeuristicResult(conn);
+ let hasHeuristic = yield this._matchFirstHeuristicResult(conn);
this._addingHeuristicFirstMatch = false;
+ if (!this.pending)
+ return;
// We sleep a little between adding the heuristicFirstMatch and matching
// any other searches so we aren't kicking off potentially expensive
// searches on every keystroke.
- yield this._sleep(Prefs.delay);
- if (!this.pending)
- return;
+ // Though, if there's no heuristic result, we start searching immediately,
+ // since autocomplete may be waiting for us.
+ if (hasHeuristic) {
+ yield this._sleep(Prefs.delay);
+ if (!this.pending)
+ return;
+ }
if (this._enableActions) {
yield this._matchSearchSuggestions();
if (!this.pending)
return;
}
for (let [query, params] of queries) {
@@ -945,68 +951,70 @@ Search.prototype = {
*_matchFirstHeuristicResult(conn) {
// We always try to make the first result a special "heuristic" result. The
// heuristics below determine what type of result it will be, if any.
if (this._searchTokens.length > 0) {
// This may be a Places keyword.
let matched = yield this._matchPlacesKeyword();
if (matched) {
- return;
+ return true;
}
}
if (this.pending && this._enableActions) {
// If it's not a Places keyword, then it may be a search engine
// with an alias - which works like a keyword.
let matched = yield this._matchSearchEngineAlias();
if (matched) {
- return;
+ return true;
}
}
let shouldAutofill = this._shouldAutofill;
if (this.pending && shouldAutofill) {
// It may also look like a URL we know from the database.
let matched = yield this._matchKnownUrl(conn);
if (matched) {
- return;
+ return true;
}
}
if (this.pending && shouldAutofill) {
// Or it may look like a URL we know about from search engines.
let matched = yield this._matchSearchEngineUrl();
if (matched) {
- return;
+ return true;
}
}
if (this.pending && this._enableActions) {
// If we don't have a result that matches what we know about, then
// we use a fallback for things we don't know about.
// We may not have auto-filled, but this may still look like a URL.
// However, even if the input is a valid URL, we may not want to use
// it as such. This can happen if the host would require whitelisting,
// but isn't in the whitelist.
let matched = yield this._matchUnknownUrl();
if (matched) {
- return;
+ return true;
}
}
if (this.pending && this._enableActions && this._originalSearchString) {
// When all else fails, and the search string is non-empty, we search
// using the current search engine.
let matched = yield this._matchCurrentSearchEngine();
if (matched) {
- return;
+ return true;
}
}
+
+ return false;
},
*_matchSearchSuggestions() {
// Limit the string sent for search suggestions to a maximum length.
let searchString = this._searchTokens.join(" ")
.substr(0, Prefs.maxCharsForSearchSuggestions);
// Avoid fetching suggestions if they are not required, private browsing
// mode is enabled, or the search string may expose sensitive information.
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1113,17 +1113,23 @@ extends="chrome://global/content/binding
// Update the richlistbox height.
if (this._adjustHeightTimeout) {
clearTimeout(this._adjustHeightTimeout);
}
if (this._shrinkTimeout) {
clearTimeout(this._shrinkTimeout);
}
- this._adjustHeightTimeout = setTimeout(() => this.adjustHeight(), 0);
+
+ if (this.mPopupOpen) {
+ delete this._adjustHeightOnPopupShown;
+ this._adjustHeightTimeout = setTimeout(() => this.adjustHeight(), 0);
+ } else {
+ this._adjustHeightOnPopupShown = true;
+ }
this._currentIndex = 0;
if (this._appendResultTimeout) {
clearTimeout(this._appendResultTimeout);
}
this._appendCurrentResult(reason);
]]>
</body>
@@ -1301,20 +1307,23 @@ extends="chrome://global/content/binding
}
else {
// set the class at the end so we can use the attributes
// in the xbl constructor
item.className = "autocomplete-richlistitem";
this.richlistbox.appendChild(item);
}
- let changed = item.adjustSiteIconStart(this._siteIconStart);
- if (changed) {
- item.handleOverUnderflow();
- }
+ // The binding may have not been applied yet.
+ setTimeout(() => {
+ let changed = item.adjustSiteIconStart(this._siteIconStart);
+ if (changed) {
+ item.handleOverUnderflow();
+ }
+ }, 0);
this._currentIndex++;
}
if (typeof this.onResultsAdded == "function")
this.onResultsAdded();
if (this._currentIndex < matchCount) {
@@ -1372,16 +1381,26 @@ extends="chrome://global/content/binding
document.getAnonymousElementByAttribute(this, "anonid", "richlistbox");
</field>
<property name="view"
onget="return this.mInput.controller;"
onset="return val;"/>
</implementation>
+ <handlers>
+ <handler event="popupshown">
+ <![CDATA[
+ if (this._adjustHeightOnPopupShown) {
+ delete this._adjustHeightOnPopupShown;
+ this.adjustHeight();
+ }
+ ]]>
+ </handler>
+ </handlers>
</binding>
<binding id="autocomplete-richlistitem" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
<content align="center"
onoverflow="this._onOverflow();"
onunderflow="this._onUnderflow();">
<xul:image anonid="type-icon"