fix: constraint validation API in form controls doesn't work and prevents form submission

[claviska]

🐛 Bug Report

When using required, pattern, and other attributes associated with the constraint validation API, an error is thrown and no constraint is signaled when the form is submitted.

💻 Repro or Code Sample

https://jsfiddle.net/v7cmd0Lj/

🤔 Expected Behavior

The browser should show an alert stating that a value is required in the form control and prevent submission.

😯 Current Behavior

In Edge, Chrome, and Safari, the following error is received when submitting the form:

An invalid form control with name='name' is not focusable.

In Firefox, no error is thrown, no validation message is shown, and the form does not submit.

💁 Possible Solution

This answer on StackOverflow describes what's happening in more detail.

🔦 Context

Our users have filed bug reports expecting required and similar constraint validation attributes to work as documented.

🌍 Your Environment

• Browser Edge 98, Chrome 98, Safari 15.3
• Version [e.g. 1.8.0]

[claviska]

Following up on this with another realization. Not only is constraint validation broken, but the form won't even submit because of the error.

https://jsfiddle.net/7sx3bwzy/1/

I've updated the title to reflect that this is a much bigger issue than the initial report because it renders forms completely useless if you're using any constraint validation attributes.

Unfortunately, we have users who are impacted by this. Any tips as to where we should be looking to help solve this?

[EisenbergEffect]

Dropping a note here for you @nicholasrice This polyfill might be wroth looking at: https://github.com/calebdwilliams/element-internals-polyfill

[nicholasrice]

At this point I don't think this is an issue related to the ElementInternals implementation that we have and more of a component implementation issue. In this repro, only the text input repros the issue; the checkbox does not: https://stackblitz.com/edit/web-platform-xwkbet?file=index.html The big difference between these two is the use of delegatesFocus w/ an internal input element for the text field.

I haven't had a chance to dig in further, but my suspicion right now is that using delegatesFocus (to bring focus to the inner input element) along with elementInternals (to serve as form association) is confusing the browser.

[nicholasrice]

I've distilled the repro down to one that uses only DOM APIs (no FAST code): https://stackblitz.com/edit/typescript-rguvon?file=index.ts

The behavior appears inherent to using both the ElementInternals feature in conjunction with delegatesFocus, which is the approach that the following FAST controls leverage:

• button
• combobox
• number-field
• search
• text-area
• text-field

It appears that the browser throws when attempting to surface the validation UI for a form-associated custom element that implements delegatesFocus. I believe the two general paths forward here are to refactor these components to not require delegatesFocus or to remove form association from their implementation. I'll continue to investigate and update this thread with more detailed proposals.

[nicholasrice]

Here are a few ways I think we can fix this problem. The changes would affect the following components, all of which use ElementInternals (and form association) and focus delegation:

• button
• combobox
• number-field
• picker (@scomea do you know if picker should be defined with delegatesFocus or not? storybook and .spec.ts implementations differ)
• search
• text-area
• text-field

Wrapper Component

All of the above elements except combobox are analogs for native HTML elements. The reason we have custom element components for these components is primarily to facilitate visual design implementations. https://github.com/microsoft/fast/issues/2694 outlines the original research and proposal, with some follow ups by me recommending against a wrapper component for various reasons, namely the inability to style certain pseudo elements that form controls often use.

The wrapper component would be implemented as an FASTElement that wraps native input elements:

<fast-input>
  <label for="name" slot="label">Name</label>
  <input type="text" name="name" />
</fast-input>

The biggest benefits to this approach are:

1. Input element is in the light DOM, meaning form association works automatically, as do ARIA accessibility attributes
2. Reduced JavaScript footprint not having to implement large templates, attribute forwarding logic, form-association logic, etc
3. Platform features like autocomplete, validation, suggestion lists come for free.
4. Will work in all browsers, no need for ElementInternals checks and edge-cases (looking at you, WebKit).

The biggest risks of this approach are:

1. Not all components are a good fit for this model (checkbox/radio have issues, but may be achievable, but datepicker, range slider, and others will never be able to achieve styling goals due to slotted pseudo-element selector limitations), leading to fragmentation of implementation
   1. Additionally, browser suggestions modals for text-field suggestions aren't selectable, though I'd argue that is still better than not offering support at all, which is the current state of the controls.
2. It could be difficult to adapt to designs over time, depending on what those design changes are.

At this point, I'm starting to think the benefits of a wrapper model where applicable could be worth it. These form controls are not small components so we would see bundle improvements, less runtime JavaScript means improved rendering performance, platform features just work (though some might look like browser chrome and not "themed"), and accessibility will be improved (circumvents DOM boundary ID reference issues).

Remove delegatesFocus and fix bugs

We can also simply remove delegatesFocus and refactor the control to correctly handle focus. This would involve:

1. Not defining elements with delegatesFocus flag
2. Ensuring the host element itself cannot get focus (remove tabindex)
3. Patching up interaction model to handle click events correctly.

We'll need to handle are mouse clicks on the element that are not clicks within the nested input/button. In most cases this will be the before and after slotted elements, but clicks can also happen on host padding and other targets that may be in the shadow DOM.

This approach is likely the lowest-touch, but I do worry a bit about bug tail and pinning down edge cases.

Both Approaches

I believe every component except combobox and picker (if it is supposed to delegate focus) can be implemented more minimally than it is today using a wrapper model. While this is a big departure from the existing implementation, I think it has some compelling benefits from an accessibility and ease-of-use perspective. Combobox and (picker?) can have their implementation adjusted to not rely on delegatesFocus behavior.

[calebdwilliams]

This is a really simple fix. The ElementInternals.prototype.setValidity method takes three arguments:

• validityState:Partial<ValidityState>
• message?: string (required if any validity flag is set to true)
• validationTarget?: HTMLElement (must be included in the shadow root of the element being validated)

It looks like your problem is on this line. You just need to change that to

this.setValidity(this.proxy.validity, this.proxy.validationMessage, this.proxy);

The comment y'all have here refers to this behavior. For whatever reason the setValidity method doesn't respect delegatesFocus.

I've written a wrapper around the form-associated APIs for Open WC. The FormControlMixin implements this by allowing users to define a validationTarget that will be used for invalid controls.

The mixin works really well with FAST too, for whatever it's worth.

[nicholasrice]

Thank you @calebdwilliams!! I can't believe it's that simple of a fix; I'd completely forgotten about that argument. I'll take a stab at implementing the mixin you linked too, it would be great to align to that if we can.

[chrisdholt]

This is a really simple fix. The ElementInternals.prototype.setValidity method takes three arguments:

• validityState:Partial<ValidityState>
• message?: string (required if any validity flag is set to true)
• validationTarget?: HTMLElement (must be included in the shadow root of the element being validated)

It looks like your problem is on this line. You just need to change that to

this.setValidity(this.proxy.validity, this.proxy.validationMessage, this.proxy);

The comment y'all have here refers to this behavior. For whatever reason the setValidity method doesn't respect delegatesFocus.

I've written a wrapper around the form-associated APIs for Open WC. The FormControlMixin implements this by allowing users to define a validationTarget that will be used for invalid controls.

The mixin works really well with FAST too, for whatever it's worth.

Thank you @calebwilliams - it's worth quite a bit to know it works well!

[chrisdholt]

This has been published to both our latest alpha release of foundation (3.0.0-alpha.8) as well as back-ported into version 2.46.13.