From b52e93223b8ee8d8e3b60f757fbc7b7ebde403a4 Mon Sep 17 00:00:00 2001 From: Drew Willcoxon Date: Wed, 23 Jan 2019 19:37:20 +0000 Subject: [PATCH] Bug 1521236 - Add UrlbarInput.autofill(), call it from the controller, and disable autofill when the new search string is a prefix of the old string. r=mak There are two parts to this: When the controller receives the first result and the context has an autofill value, then call a new input.autofill() method, which does the actual autofill in the input. We shouldn't autofill when the user back spaces. Actually back spacing is just one example of deleting text at the end of the input; the user could select text and press delete, or use the delete-previous-word key shortcut, etc. So it's not correct to simply check whether the last keypress was a backspace and skip autofill then. We should copy the logic of the legacy autocomplete controller and check whether the new search string is a prefix of the old string. If so, then don't autofill. So I added that logic to UrlbarInput. I could have added similar logic to the controller or the context instead, but it makes the most sense being in the input since it's the input where the user is interacting with the text value. Differential Revision: https://phabricator.services.mozilla.com/D17040 --HG-- extra : moz-landing-system : lando --- .../components/urlbar/UrlbarController.jsm | 16 +++++---- browser/components/urlbar/UrlbarInput.jsm | 36 +++++++++++++++++-- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/browser/components/urlbar/UrlbarController.jsm b/browser/components/urlbar/UrlbarController.jsm index e8c6f9efe8ef..5825192c9ce1 100644 --- a/browser/components/urlbar/UrlbarController.jsm +++ b/browser/components/urlbar/UrlbarController.jsm @@ -93,7 +93,7 @@ class UrlbarController { } this._lastQueryContext = queryContext; - queryContext.lastTelemetryResultCount = 0; + queryContext.lastResultCount = 0; TelemetryStopwatch.start(TELEMETRY_1ST_RESULT, queryContext); TelemetryStopwatch.start(TELEMETRY_6_FIRST_RESULTS, queryContext); @@ -126,15 +126,18 @@ class UrlbarController { * @param {UrlbarQueryContext} queryContext The query details. */ receiveResults(queryContext) { - if (queryContext.lastTelemetryResultCount < 1 && - queryContext.results.length >= 1) { + if (queryContext.lastResultCount < 1 && queryContext.results.length >= 1) { TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT, queryContext); } - if (queryContext.lastTelemetryResultCount < 6 && - queryContext.results.length >= 6) { + if (queryContext.lastResultCount < 6 && queryContext.results.length >= 6) { TelemetryStopwatch.finish(TELEMETRY_6_FIRST_RESULTS, queryContext); } - queryContext.lastTelemetryResultCount = queryContext.results.length; + + if (queryContext.lastResultCount == 0 && queryContext.autofillValue) { + this.input.autofill(queryContext.autofillValue); + } + + queryContext.lastResultCount = queryContext.results.length; this._notify("onQueryResults", queryContext); } @@ -199,7 +202,6 @@ class UrlbarController { // Prevent beep on Mac. event.preventDefault(); } - // TODO: We may have an autofill entry, so we should use that instead. // TODO: We should have an input bufferrer so that we can use search results // if appropriate. this.input.handleCommand(event); diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm index 92bfe91fdae9..e790dac32ff5 100644 --- a/browser/components/urlbar/UrlbarInput.jsm +++ b/browser/components/urlbar/UrlbarInput.jsm @@ -390,15 +390,26 @@ class UrlbarInput { return; } + let searchString = this.textValue; + + // If the user has deleted text at the end of the input since the last + // query, then we don't want to autofill because doing so would autofill the + // very text the user just deleted. + let enableAutofill = + UrlbarPrefs.get("autoFill") && + (!this._lastSearchString || + !this._lastSearchString.startsWith(searchString)); + this.controller.startQuery(new UrlbarQueryContext({ - enableAutofill: UrlbarPrefs.get("autoFill"), + enableAutofill, isPrivate: this.isPrivate, lastKey, maxResults: UrlbarPrefs.get("maxRichResults"), muxer: "UnifiedComplete", providers: ["UnifiedComplete"], - searchString: this.textValue, + searchString, })); + this._lastSearchString = searchString; } typeRestrictToken(char) { @@ -428,6 +439,27 @@ class UrlbarInput { this.textbox.classList.remove("hidden-focus"); } + /** + * Autofills the given value into the input. That is, sets the input's value + * to the given value and selects the portion of the new value that comes + * after the current value. The given value should therefore start with the + * input's current value. If it doesn't, then this method doesn't do + * anything. + * + * @param {string} value + * The value to autofill. + */ + autofill(value) { + if (!value.toLocaleLowerCase() + .startsWith(this.textValue.toLocaleLowerCase())) { + return; + } + let len = this.textValue.length; + this.value = this.textValue + value.substring(len); + this.selectionStart = len; + this.selectionEnd = value.length; + } + // Getters and Setters below. get focused() {