Autofill happens on backspace
Categories
(Firefox :: Address Bar, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox67 | --- | unaffected |
firefox68 | --- | verified |
firefox69 | --- | verified |
People
(Reporter: adw, Assigned: adw)
References
Details
Attachments
(2 files, 1 obsolete file)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
17.47 KB,
patch
|
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
STR
- Type a few letters so that a domain is autofilled -- don't type the whole domain
- Press the right arrow key to move the caret to the end of the input
- Hit backspace
The trailing slash is immediately autofilled again.
I thought we fixed this and added tests for it. :-(
Assignee | ||
Comment 1•4 years ago
|
||
It's a problem in the Nightly before bug 1522278, too.
Assignee | ||
Comment 2•4 years ago
|
||
Also a problem in the Nightly before bug 1529931.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
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•4 years ago
|
||
This was already happening without QB, IIRC, unless it was for a different reason.
Assignee | ||
Comment 5•4 years ago
|
||
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.
Assignee | ||
Comment 6•4 years ago
|
||
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.
Assignee | ||
Comment 7•4 years ago
|
||
... 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.
Assignee | ||
Comment 8•4 years ago
|
||
Assignee | ||
Comment 9•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
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.
Assignee | ||
Comment 11•4 years ago
|
||
Comment 12•4 years ago
|
||
Pushed by dwillcoxon@mozilla.com: 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•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 14•4 years ago
|
||
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
Updated•4 years ago
|
Comment 15•4 years ago
|
||
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.
Updated•4 years ago
|
Comment 16•4 years ago
|
||
Comment on attachment 9071738 [details] [diff] [review] Beta patch quantumbar fix for 68.0b10
Comment 17•4 years ago
|
||
bugherderuplift |
Comment 18•4 years ago
|
||
Verified - Fixed on latest Beta 68.0b10 (64-bit) on Windows 10 x64, Mac OS 10.12 and Ubuntu 18.04.
Assignee | ||
Updated•4 years ago
|
Description
•