Form autofill not displaying preview on hover
Categories
(Toolkit :: Form Autofill, defect)
Tracking
()
People
(Reporter: atrif, Assigned: emilio)
References
(Regression)
Details
(Keywords: regression)
Attachments
(5 files)
2.13 MB,
video/quicktime
|
Details | |
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr91+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr91+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr91+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr91+
|
Details | Review |
Affected versions
- 92.0a1 (20210727094543)
- 91.0b8 (20210727214000)
- 90.0.2 (20210721174149)
Affected platforms
- Windows 10x64
- Windows 7x64
- macOS 11
- Ubuntu 18.04
Preconditions
- have at least one saved address
Steps to reproduce
- Open https://luke-chang.github.io/autofill-demo/basic.html and hover over the saved addresses.
Expected result
- Previews are displayed inside fields on hover.
Actual result
- Only a yellow bar is shown.
Regression range
- Nightly from 2021-02-02 is not affected. I will search for one ASAP.
Notes
- Attached a screen recording showing an older version and 91.0b8.
Reporter | ||
Updated•2 years ago
|
Updated•2 years ago
|
Comment 1•2 years ago
|
||
Narrowed regression window to the following commit:
Bug 1698315 - Manage placeholder and autofill preview visibility using CSS rather than custom code. r=masayuki
Updated•2 years ago
|
Comment 2•2 years ago
|
||
Hi emilio, it seems that Bug 1698315 causes the regression, could you help take a look at this? thanks!
Assignee | ||
Comment 3•2 years ago
|
||
Sure. How do I get the address autofill? At least when I try to autofill the form here I don't get any suggestions, even on a clean build...
Reporter | ||
Comment 4•2 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)
Sure. How do I get the address autofill? At least when I try to autofill the form here I don't get any suggestions, even on a clean build...
Hello! For autofill to work you will need to set extensions.formautofill.available:on
and browser.search.region:US
. Restart the browser and add some addresses by pressing Saved Addresses under Forms and Autofill inside about:preferences#privacy. You can use https://www.fakepersongenerator.com/ site or add random ones. Note that Country or Region needs to be on United States.
Assignee | ||
Comment 6•2 years ago
|
||
The style system uses the changed bits to compute the old state, so if
it's inaccurate it might cause styles to be incorrectly invalidated.
This causes issues because with the next patch the autofill jsm
calls removeManuallyManagedStates(AUTOFILL), then
addManuallyManagedStates(AUTOFILL | AUTOFILL_PREVIEW), and if the input
didn't have AUTOFILL before we'd incorrectly detect it as not changing
with the next patch.
Also make them not virtual anymore since nobody overrides them. An
alternative to this would be to assert that we don't yet have the state
we're adding (or that we have the state we're removing), and handle it
in the callers. But this is a bit more convenient.
Assignee | ||
Comment 7•2 years ago
|
||
This is useful so that author rules for :autofill also work for the
autofill preview.
It also makes the UA sheet in forms.css simpler (otherwise we'd need to
tweak the selectors to put :-moz-autofill-preview everywhere we put
:autofill).
Depends on D122013
Assignee | ||
Comment 8•2 years ago
|
||
Depends on D122014
Assignee | ||
Comment 9•2 years ago
|
||
They're not the prettiest, but that's all that needs to happen right now for
autofill to work.
Depends on D122015
Comment 10•2 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/72b71bfe6d2c Make Element::{Add,Remove}States handle change notifications correctly. r=smaug https://hg.mozilla.org/integration/autoland/rev/db41a8a3a901 Make :-moz-autofill-preview imply :autofill. r=hiro https://hg.mozilla.org/integration/autoland/rev/1bcfaa7b4b3f Load SpecialPowers in reftests. r=jgraham,jmaher https://hg.mozilla.org/integration/autoland/rev/abd8ccf71212 Add tests for autofill rendering. r=hiro
Comment 11•2 years ago
|
||
Backed out for mochitest plain failures on test_formautofill_preview_highlight.html.
Failure log: https://treeherder.mozilla.org/logviewer?job_id=347863658&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/28ec24b2ab1d0c4740251d9626d3284f4c6db4d0
Assignee | ||
Updated•2 years ago
|
Comment 12•2 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/69da4af50d23 Make Element::{Add,Remove}States handle change notifications correctly. r=smaug https://hg.mozilla.org/integration/autoland/rev/3e04c6be2e0c Make :-moz-autofill-preview imply :autofill. r=hiro https://hg.mozilla.org/integration/autoland/rev/b3c361c3fc38 Load SpecialPowers in reftests. r=jgraham,jmaher https://hg.mozilla.org/integration/autoland/rev/429bfcda963d Add tests for autofill rendering. r=hiro
Comment 13•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/69da4af50d23
https://hg.mozilla.org/mozilla-central/rev/3e04c6be2e0c
https://hg.mozilla.org/mozilla-central/rev/b3c361c3fc38
https://hg.mozilla.org/mozilla-central/rev/429bfcda963d
Reporter | ||
Comment 14•2 years ago
|
||
Verified fixed with 93.0a1 (20210815094823) on Windows 10x64, macOS 10.15 and Ubuntu 21.04. Autofill preview is displayed as expected when hovered. Thank you!
Comment 15•2 years ago
|
||
Please nominate this for Beta & ESR91 approval when you get a chance.
Assignee | ||
Comment 16•2 years ago
|
||
Comment on attachment 9235140 [details]
Bug 1722662 - Make Element::{Add,Remove}States handle change notifications correctly. r=smaug
Beta/Release Uplift Approval Request
- User impact if declined: comment 0
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: 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): Relatively straight-forward patch (the harder bits are actually tests)
- String changes made/needed: none
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Comment 17•2 years ago
|
||
Comment on attachment 9235140 [details]
Bug 1722662 - Make Element::{Add,Remove}States handle change notifications correctly. r=smaug
Approved for 92.0b5.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 18•2 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/d97a93b6b25f
https://hg.mozilla.org/releases/mozilla-beta/rev/374f794c5641
https://hg.mozilla.org/releases/mozilla-beta/rev/fb8d1d76a8bb
https://hg.mozilla.org/releases/mozilla-beta/rev/e2fca5cea74b
Reporter | ||
Comment 19•2 years ago
|
||
Verified fixed with 92.0b5 (20210817185605) on Windows 10x64, macOS 10.15 and Ubuntu 21.04.
Comment 20•2 years ago
|
||
Comment on attachment 9235140 [details]
Bug 1722662 - Make Element::{Add,Remove}States handle change notifications correctly. r=smaug
Also approved for 91.1esr.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 21•2 years ago
|
||
bugherderuplift |
Reporter | ||
Comment 22•2 years ago
|
||
Verified fixed with 91.1.0esr (20210830184617) on Windows 10x64, macOS 10.15 and Ubuntu 21.04.
Description
•