Add UrlbarInput.autofill(), call it from the controller, and disable autofill when the new search string is a prefix of the old string

RESOLVED FIXED in Firefox 66

Status

()

P1
normal
RESOLVED FIXED
a month ago
29 days ago

People

(Reporter: adw, Assigned: adw)

Tracking

unspecified
Firefox 66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a month ago

Add UrlbarInput.autofill(), call it from the controller, and disable autofill when the new search string is a prefix of the old string.

(Assignee)

Comment 1

a month ago

Created attachment 9037713 [details]
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.

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.

AFAICT the lastKey option passed around in the input and other places was added in anticipation of autofill, e.g., from AddressBar.rst:

lastKey; // {string} The last key pressed by the user. This can affect the
         // behavior, for example by not autofilling again when the user
         // hit backspace.

So I think we can remove it? I'll file another bug for that.

Comment 2

29 days ago
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d6c453110b21
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
(Assignee)

Comment 3

29 days ago

I filed bug 1522280 for adding tests. I marked it blocking the main autofill bug instead of this one.

Comment 4

29 days ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 29 days ago
status-firefox66: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
You need to log in before you can comment on or make changes to this bug.