whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.16k stars 2.69k forks source link

Specify events to fire when a browser autocompletion mechanism fills a field #3016

Open mnoorenberghe opened 7 years ago

mnoorenberghe commented 7 years ago

https://html.spec.whatwg.org/#autofill-processing-model:control's-data-4

The autocompletion mechanism must be implemented by the user agent acting as if the user had modified the control's data, and must be done at a time where the element is mutable (e.g. just after the element has been inserted into the document, or when the user agent stops parsing).

In Firefox bug 1395928 and older bugs for Firefox's password manager, the following line is unclear:

as if the user had modified the control's data

This can be interpreted in various ways and it would be great to have compatibility between browsers for autofill. One problem is that sites don't think about browser/extension autofill and assume they only need to update custom validation after a blur event (ideally they should listen for change instead) and so some browsers not synthesize focus and blur.

Possible events that could be fired:

CC @annevk

annevk commented 7 years ago

Seems https://bugzilla.mozilla.org/show_bug.cgi?id=1395928#c7 has some analysis about Chrome's behavior, but only about event types, not about the classes used. Then later on some decision is made to use CustomEvent. Was that based on testing?

@rbyers could you help out here?

mnoorenberghe commented 7 years ago

Seems https://bugzilla.mozilla.org/show_bug.cgi?id=1395928#c7 has some analysis about Chrome's behavior, but only about event types, not about the classes used.

Here is a bit more data in tabular form but I didn't realize you wanted classes until after I compiled this: https://docs.google.com/spreadsheets/d/1z6pcu55PHU0l9UraKhZSuh0kjVsamlopVMP-amAMtUs/edit#gid=0

Then later on some decision is made to use CustomEvent. Was that based on testing?

Ignore that… it was a proof-of-concept. We want to know what we should do for interop before implementing something.

CC: @smaug----

RByers commented 7 years ago

Sorry I missed this. I'm not sure who owns this autofill logic but it's related to input to some extent. @dtapuska can you help find someone who can help explain what Chrome is doing with input events on autofill and discuss any potential changes folks might want to make?

dtapuska commented 7 years ago

The relevant Chrome code is here (don't shoot the messenger)

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/exported/WebFormControlElement.cpp?l=95

Reading the code I get: 1) No focusout/focusin 2) Firing focus/blur events do not change the active focused node. 3) Checkboxes - no focus/blur, just input, change (not sure if this fires) 4) Input/Textarea - focus (if not current focused node), keydown, input, change, keyup, blur (if not current focused node) 5) Select - focus (if not current focused node), input, change, blur (if not current focused node)

smaug---- commented 7 years ago

Why is Chrome behaving so oddly? Why focus/blur doesn't change active focused node? Why no focusout/in? This all feels very much ad-hoc "let's do something which happens to work for some particular use case".

Would be better to actually design how it should work and then spec it properly. Does anyone have time to do that? ;)

dtapuska commented 7 years ago

Chrome is doing this because it is dispatching a focus event directly as opposed to calling the focus method on the field. If it called focus() then yes the correct events would fire. Albeit that chrome fires the focusin/out & focus/blur in non-standard order already.

I wonder if it was done this way because focusing a field causes it to scroll into view I believe which would be an under-desirable property of autofilling but I'd have to do more research.

smaug---- commented 7 years ago

That doesn't really tell why Chrome is doing that. But yeah, if would be good to know some historical reasons why this rather unexpected behavior was added.

dtapuska commented 7 years ago

@sebsg Can you comment why added these events in https://chromium.googlesource.com/chromium/src/+/4463586e3152ae8dec90f949a401b4695acf882c and didn't actually focus the field?

Are you able to work on spec'ing them?

sebsg commented 7 years ago

Hi!

Initially chrome only used SetValue(...) on the field.

However, a bunch of sites were listening to focus, keydown/up, or blur events to do some validation or other changes in JS. That completely broke autofill in those cases and lead to inconsistent and confusing behavior.

We decided to fire these events without modifying the how the actual filling works (like dtapuska mentioned, we don't want to focus on all the fields as it could scroll down and be confusing for the users).

I hope this helps!

dtapuska commented 7 years ago

Was scrolling down the only thing? We can certainly fix that (via https://github.com/whatwg/html/issues/834#issuecomment-305089508)

zkoch commented 7 years ago

Sorry for my ignorance, but does focusing on the fields also set the cursor inside the field?

For background, our general idea was that Autofill should function like an IME, but with the challenge that IMEs are typically limited to the field that is currently focused. So from a dev perspective, they should just see a bunch of fields getting filled out really quickly (all the standard events should fire).

That said, this also shouldn't be jarring for a user. This means that they shouldn't see a cursor hopping around from field to field, and the page shouldn't scroll as well.

We're very open to solving or addressing this in different ways. :)