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

User interacted flag gets set when change events are set, but tests disagree #10013

Open
emilio opened this issue Dec 20, 2023 · 5 comments
Open

Comments

@emilio
Copy link
Contributor

emilio commented Dec 20, 2023

What is the issue with the HTML Standard?

This test: https://github.com/web-platform-tests/wpt/blob/a41c93e155/css/selectors/user-invalid.html#L120-L152

Claims:

A required input or textarea should match :user-invalid if a user types into it and then clears it before blurring.

But I don't think there's spec text backing that out.

I think the tested behavior is reasonable, probably https://html.spec.whatwg.org/#focus-update-steps should set the user-interacted flag even if the value hasn't changed in the end, even though right now it's inside:

the user has changed the element's value or its list of selected files while the control was focused without committing that change (such that it is different to what it was when the control was first focused).

Thoughts @nt1m @josepharhar

@emilio
Copy link
Contributor Author

emilio commented Dec 20, 2023

I just realized that's a bit tricky if you want https://github.com/web-platform-tests/wpt/blob/a41c93e155a26abe762bf93b9267b8ff4a7c1c5e/css/selectors/user-invalid.html#L45-L49 to keep working... Maybe should be tied to the value dirty flag, instead of something more complex?

@emilio emilio added the agenda+ To be discussed at a triage meeting label Dec 20, 2023
@whatwg whatwg deleted a comment from mitunshikder123 Jan 5, 2024
@josepharhar
Copy link
Contributor

Yeah I wrote that test and implemented the change in chromium here: https://chromium-review.googlesource.com/c/chromium/src/+/4966771

The gist of it is that as soon as the user types anything into any of the supported elements we set a flag saying that the user has interacted with it, and when the element is blurred, we start matching :user-valid/:user-invalid.

This is kind of an alternative to using the change event as a signal to start matching :user-valid/:user-invalid, because if the user types something in and then erases it and then blurs we don't fire a change event but start matching :user-valid/:user-invalid.

I think the current spec just sets user interacted to true when the change event is fired, right? Perhaps it would need to be set somewhere else that gets run any time the user types anything...?

@smaug----
Copy link

Yeah, the "user interacted" flag in the spec is just broken atm. It doesn't seem to have anything to do with user interaction.

@josepharhar
Copy link
Contributor

We just discussed looking at trusted input events before blurring rather than looking for a change event. In chromium, this is how we currently determine whether the input is dirty for :user-valid/:user-invalid: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/text_control_element.cc;l=131-137;drc=67d90538f11c6b232dbfd716075db52aeb34fd15

It gets set when a webkit-editable-content-changed event is fired except in the scope of a call to document.execCommand(). I presume it would be the same to look for trusted input events.

And of course, we don't start matching :user-valid/:user-invalid until the input is "dirty" according to this definition and the input is then blurred. This is tracked in chromium via this state machine: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/html_form_control_element_with_state.h;l=80-101;drc=8036f183da3dfc0fe8e873e61f40a147f4373edd

@josepharhar
Copy link
Contributor

Also I wrote a test for script setting value but I may have forgotten to blur the element after assigning from script so the test may need fixing: web-platform-tests/wpt#43058

@past past removed the agenda+ To be discussed at a triage meeting label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants