Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mnoorenberghe opened this issue Sep 7, 2017 · 11 comments
Labels
interop Implementations are not interoperable with each other topic: events topic: forms

Comments

@mnoorenberghe
Copy link

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:

  • mousedown, click, mouseup and any other mouse events as if a user clicked in the field to focus each field being filled
  • focus/focusin/focusout + blur events for each field
    • in which order should they be fired?
    • what is Document.activeElement during each event?
  • keydown, keypress, keyup, input and any other keyboard/composition-related events
  • change event

CC @annevk

@annevk annevk added topic: events topic: forms interop Implementations are not interoperable with each other labels Sep 7, 2017
@annevk
Copy link
Member

annevk commented Sep 7, 2017

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
Copy link
Author

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
Copy link

RByers commented Sep 27, 2017

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
Copy link
Contributor

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----
Copy link
Collaborator

smaug---- commented Oct 5, 2017

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
Copy link
Contributor

dtapuska commented Oct 5, 2017

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----
Copy link
Collaborator

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
Copy link
Contributor

dtapuska commented Oct 5, 2017

@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
Copy link

sebsg commented Oct 5, 2017

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
Copy link
Contributor

dtapuska commented Oct 5, 2017

Was scrolling down the only thing? We can certainly fix that (via #834 (comment))

@zkoch
Copy link

zkoch commented Oct 6, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other topic: events topic: forms
Development

No branches or pull requests

7 participants