1722662 - Form autofill not displaying preview on hover

Status: VERIFIED FIXED

(Toolkit :: Form Autofill, defect)

Description

[Alexandru Trif, QA [:atrif]]

Attachment: Screen Recording 2021-07-28 at 10.05.44.mov

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

1. 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.

Comment 1

[Simona Badau]

Narrowed regression window to the following commit:

Bug 1698315 - Manage placeholder and autofill preview visibility using CSS rather than custom code. r=masayuki

pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9797872fe7aa7a2f9d137fcc4b1cb03d8ab353ae&tochange=d989f71f0a415c9efe4fce7b01de4868289cacf7

Comment 2

[Dimi Lee [:dimi][:dlee]]

Hi emilio, it seems that Bug 1698315 causes the regression, could you help take a look at this? thanks!

Comment 3

[Emilio Cobos Álvarez (:emilio)]

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...

Comment 4

[Alexandru Trif, QA [:atrif]]

(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.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Thank you.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1722662 - Make Element::{Add,Remove}States handle change notifications correctly. r=smaug

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.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1722662 - Make :-moz-autofill-preview imply :autofill. r=#layout-reviewers

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

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1722662 - Load SpecialPowers in reftests. r=jgraham,jmaher

Depends on D122014

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1722662 - Add tests for autofill rendering. r=#layout-reviewers

They're not the prettiest, but that's all that needs to happen right now for
autofill to work.

Depends on D122015

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

[Cosmin Sabou [:CosminS]]

Backed out for mochitest plain failures on test_formautofill_preview_highlight.html.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=abd8ccf71212bc069c92afa6304453acd691cc94&selectedTaskRun=XJJ7QHEGSYquQugvrAh4zg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=347863658&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/28ec24b2ab1d0c4740251d9626d3284f4c6db4d0

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

[Narcis Beleuzu [:NarcisB]]

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

Comment 14

[Alexandru Trif, QA [:atrif]]

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

[[Back August 16] Ryan VanderMeulen]

Please nominate this for Beta & ESR91 approval when you get a chance.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

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

Comment 17

[[Back August 16] Ryan VanderMeulen]

Comment on attachment 9235140 [details]
Bug 1722662 - Make Element::{Add,Remove}States handle change notifications correctly. r=smaug

Approved for 92.0b5.

Comment 18

[[Back August 16] Ryan VanderMeulen]

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

Comment 19

[Alexandru Trif, QA [:atrif]]

Verified fixed with 92.0b5 (20210817185605) on Windows 10x64, macOS 10.15 and Ubuntu 21.04.

Comment 20

[[Back August 16] Ryan VanderMeulen]

Comment on attachment 9235140 [details]
Bug 1722662 - Make Element::{Add,Remove}States handle change notifications correctly. r=smaug

Also approved for 91.1esr.

Comment 21

[[Back August 16] Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-esr91/rev/ae0d7cd33780
https://hg.mozilla.org/releases/mozilla-esr91/rev/9f615f29203b
https://hg.mozilla.org/releases/mozilla-esr91/rev/e2d127e96624
https://hg.mozilla.org/releases/mozilla-esr91/rev/bb7bcecb1f93

Comment 22

[Alexandru Trif, QA [:atrif]]

Verified fixed with 91.1.0esr (20210830184617) on Windows 10x64, macOS 10.15 and Ubuntu 21.04.