1557555 - Autofill happens on backspace

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

STR

1. Type a few letters so that a domain is autofilled -- don't type the whole domain
2. Press the right arrow key to move the caret to the end of the input
3. Hit backspace

The trailing slash is immediately autofilled again.

I thought we fixed this and added tests for it. :-(

Comment 1

[Drew Willcoxon :adw]

It's a problem in the Nightly before bug 1522278, too.

Comment 2

[Drew Willcoxon :adw]

Also a problem in the Nightly before bug 1529931.

Comment 3

[Drew Willcoxon :adw]

browser_autoFill_backspaced.js checks quite a few things, but not the STR in comment 0. :-( If you type out the whole domain so that only the trailing slash autofills and then hit backspace, that works correctly, and the test does check that.

Comment 4

[Marco Bonardo [:mak]]

This was already happening without QB, IIRC, unless it was for a different reason.

Comment 5

[Drew Willcoxon :adw]

This isn't bug 1545731, which requires first selecting a non-heuristic result. It's a qb regression that happens on the heuristic result, i.e., the STR in comment 0 exactly.

Comment 6

[Drew Willcoxon :adw]

Ah, this looks like the reason it doesn't happen in awesomebar but does in qb: https://searchfox.org/mozilla-central/rev/fe7dbedf223c0fc4b37d5bd72293438dfbca6cec/toolkit/components/autocomplete/nsAutoCompleteController.cpp#633

We don't do anything like that in qb. When I set this._lastSearchString = this.textValue in UrlbarInput._on_keydown, the problem goes away.

Comment 7

[Drew Willcoxon :adw]

... and because that nsAutoCompleteController check is only for DOM_VK_LEFT, DOM_VK_RIGHT, and DOM_VK_HOME, you can trick it by using for example the readline shortcut to move the caret to the end (Ctrl-E). When you do that, you can reproduce this bug on awesomebar, too. You can also trick it by using the mouse, e.g. by clicking to move the caret to the end, and even by selecting multiple characters at the end, wow.

Comment 8

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d287dfaed6b8201999547a80ac3ca3fd41812ac

Comment 9

[Drew Willcoxon :adw]

Attachment: Bug 1557555 - Quantumbar: Autofill only when text is entered, never when it's removed (e.g., on backspace).

Comment 10

[Drew Willcoxon :adw]

Attachment: Bug 1557555 - Quantumbar: Autofill only when text is entered, never when it's removed (e.g., on backspace).

Autofill only when the user enters text -- i.e., when event.data in _on_input is non-empty. That allows us to simplify autofill quite a bit. We can get rid of the edit action listener that we previously used to detect selection deletion, and we also don't need the prefix check (lastSearchStartsWithNewSearch).

Therefore this fixes both this bug (bug 1557555) and bug 1545731/719888.

This makes one other change: We can check event.inputType in _on_input to tell whether the user is pasting, rather than keeping a _valueIsPasted property that we set in _on_paste.

Comment 11

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d6f78623ae2e37d71e40cfe7a7c879b7ef90ba8

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f75bb1e4de0
Quantumbar: Autofill only when text is entered, never when it's removed (e.g., on backspace). r=mak

Comment 13

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/2f75bb1e4de0

Comment 14

[Drew Willcoxon :adw]

Attachment: Beta patch

Beta/Release Uplift Approval Request

• User impact if declined: 68 is the release where quantumbar is shipping, so if this request is declined, autofill will be slightly broken for the release where quantumbar is debuting, and it'll also be a regression from 67.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Smallish patch that only affects autofill on quantumbar, still time to bake on Nightly and Beta
• String changes made/needed: None

Comment 15

[Alin Ilea, Desktop Test Engineering]

Verified - Fixed on latest Nightly 69.0a1 (2019-06-12) (64-bit) on Windows 10 x64, Mac OS 10.12 and Ubuntu 18.04.
Updating flag and waiting for fix on Beta.

Comment 16

[Julien Cristau [:jcristau]]

Comment on attachment 9071738 [details] [diff] [review]
Beta patch

quantumbar fix for 68.0b10

Comment 17

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/4c434d19d03d

Comment 18

[Alin Ilea, Desktop Test Engineering]

Verified - Fixed on latest Beta 68.0b10 (64-bit) on Windows 10 x64, Mac OS 10.12 and Ubuntu 18.04.