fix: forms correctly focus the first invalid control instead of last

Previously all text fields would focus themselves when the form reports validity, meaning the last one got focus. In reality, reportValidity is supposed to focus the first invalid control.

I added a "call" method wrapper around the `onReportValidity` callback that handles focusing logic.

PiperOrigin-RevId: 597904790

Author: asyncLiz

labs/behaviors/on-report-validity.ts

@@ -7,6 +7,7 @@
 import {LitElement, isServer} from 'lit';
 
 import {ConstraintValidation} from './constraint-validation.js';
+import {WithElementInternals, internals} from './element-internals.js';
 import {MixinBase, MixinReturn} from './mixin.js';
 
 /**
@@ -47,6 +48,8 @@ export const onReportValidity = Symbol('onReportValidity');
 // Private symbol members, used to avoid name clashing.
 const privateCleanupFormListeners = Symbol('privateCleanupFormListeners');
 const privateDoNotReportInvalid = Symbol('privateDoNotReportInvalid');
+const privateIsSelfReportingValidity = Symbol('privateIsSelfReportingValidity');
+const privateCallOnReportValidity = Symbol('privateCallOnReportValidity');
 
 /**
  * Mixes in a callback for constraint validation when validity should be
@@ -81,7 +84,7 @@ const privateDoNotReportInvalid = Symbol('privateDoNotReportInvalid');
  * @return The provided class with `OnReportValidity` mixed in.
  */
 export function mixinOnReportValidity<
-  T extends MixinBase<LitElement & ConstraintValidation>,
+  T extends MixinBase<LitElement & ConstraintValidation & WithElementInternals>,
 >(base: T): MixinReturn<T, OnReportValidity> {
   abstract class OnReportValidityElement
     extends base
@@ -98,6 +101,13 @@ export function mixinOnReportValidity<
      */
     [privateDoNotReportInvalid] = false;
 
+    /**
+     * Used to determine if the control is reporting validity from itself, or
+     * if a `<form>` is causing the validity report. Forms have different
+     * control focusing behavior.
+     */
+    [privateIsSelfReportingValidity] = false;
+
     // Mixins must have a constructor with `...args: any[]`
     // tslint:disable-next-line:no-any
     constructor(...args: any[]) {
@@ -124,9 +134,7 @@ export function mixinOnReportValidity<
               // A normal bubbling phase event listener. By adding it here, we
               // ensure it's the last event listener that is called during the
               // bubbling phase.
-              if (!invalidEvent.defaultPrevented) {
-                this[onReportValidity](invalidEvent);
-              }
+              this[privateCallOnReportValidity](invalidEvent);
             },
             {once: true},
           );
@@ -149,15 +157,50 @@ export function mixinOnReportValidity<
     }
 
     override reportValidity() {
+      this[privateIsSelfReportingValidity] = true;
       const valid = super.reportValidity();
       // Constructor's invalid listener will handle reporting invalid events.
       if (valid) {
-        this[onReportValidity](null);
+        this[privateCallOnReportValidity](null);
       }
 
+      this[privateIsSelfReportingValidity] = false;
       return valid;
     }
 
+    [privateCallOnReportValidity](invalidEvent: Event | null) {
+      // Since invalid events do not bubble to parent listeners, and because
+      // our invalid listeners are added lazily after other listeners, we can
+      // reliably read `defaultPrevented` synchronously without worrying
+      // about waiting for another listener that could cancel it.
+      const wasCanceled = invalidEvent?.defaultPrevented;
+      if (wasCanceled) {
+        return;
+      }
+
+      this[onReportValidity](invalidEvent);
+
+      // If an implementation calls invalidEvent.preventDefault() to stop the
+      // platform popup from displaying, focusing is also prevented, so we need
+      // to manually focus.
+      const implementationCanceledFocus =
+        !wasCanceled && invalidEvent?.defaultPrevented;
+      if (!implementationCanceledFocus) {
+        return;
+      }
+
+      // The control should be focused when:
+      // - `control.reportValidity()` is called (self-reporting).
+      // - a form is reporting validity for its controls and this is the first
+      //   invalid control.
+      if (
+        this[privateIsSelfReportingValidity] ||
+        isFirstInvalidControlInForm(this[internals].form, this)
+      ) {
+        this.focus();
+      }
+    }
+
     [onReportValidity](invalidEvent: Event | null) {
       throw new Error('Implement [onReportValidity]');
     }
@@ -185,7 +228,7 @@ export function mixinOnReportValidity<
         this,
         form,
         () => {
-          this[onReportValidity](null);
+          this[privateCallOnReportValidity](null);
         },
         this[privateCleanupFormListeners].signal,
       );
@@ -328,3 +371,31 @@ function getFormValidateHooks(form: HTMLFormElement) {
 
   return FORM_VALIDATE_HOOKS.get(form)!;
 }
+
+/**
+ * Checks if a control is the first invalid control in a form.
+ *
+ * @param form The control's form. When `null`, the control doesn't have a form
+ *     and the method returns true.
+ * @param control The control to check.
+ * @return True if there is no form or if the control is the form's first
+ *     invalid control.
+ */
+function isFirstInvalidControlInForm(
+  form: HTMLFormElement | null,
+  control: HTMLElement,
+) {
+  if (!form) {
+    return true;
+  }
+
+  let firstInvalidControl: Element | undefined;
+  for (const element of form.elements) {
+    if (element.matches(':invalid')) {
+      firstInvalidControl = element;
+      break;
+    }
+  }
+
+  return firstInvalidControl === control;
+}

labs/behaviors/on-report-validity_test.ts

@@ -453,5 +453,87 @@ describe('mixinOnReportValidity()', () => {
           .toHaveBeenCalledTimes(1);
       });
     });
+
+    describe('focusing after preventing platform popup', () => {
+      it('should focus the control when calling reportValidity()', () => {
+        const control = new TestOnReportValidity();
+        control[onReportValidity] = (invalidEvent: Event | null) => {
+          invalidEvent?.preventDefault();
+        };
+
+        spyOn(control, 'focus');
+
+        control.required = true;
+        control.reportValidity();
+        expect(control.focus)
+          .withContext('is focused')
+          .toHaveBeenCalledTimes(1);
+      });
+
+      it('should only focus the first invalid control of a form', () => {
+        const firstControl = new TestOnReportValidity();
+        firstControl[onReportValidity] = (invalidEvent: Event | null) => {
+          invalidEvent?.preventDefault();
+        };
+
+        const secondControl = new TestOnReportValidity();
+        secondControl[onReportValidity] = (invalidEvent: Event | null) => {
+          invalidEvent?.preventDefault();
+        };
+
+        spyOn(firstControl, 'focus');
+        spyOn(secondControl, 'focus');
+
+        const form = document.createElement('form');
+        form.appendChild(firstControl);
+        form.appendChild(secondControl);
+        document.body.appendChild(form);
+
+        firstControl.required = true;
+        secondControl.required = true;
+        form.reportValidity();
+        form.remove();
+
+        expect(firstControl.focus)
+          .withContext('first control (invalid) is focused')
+          .toHaveBeenCalledTimes(1);
+        expect(secondControl.focus)
+          .withContext('second control (invalid) is not focused')
+          .not.toHaveBeenCalled();
+      });
+
+      it('should focus the control when calling control.reportValidity(), even if not the first invalid control of a form', () => {
+        const firstControl = new TestOnReportValidity();
+        firstControl[onReportValidity] = (invalidEvent: Event | null) => {
+          invalidEvent?.preventDefault();
+        };
+
+        const secondControl = new TestOnReportValidity();
+        secondControl[onReportValidity] = (invalidEvent: Event | null) => {
+          invalidEvent?.preventDefault();
+        };
+
+        spyOn(firstControl, 'focus');
+        spyOn(secondControl, 'focus');
+
+        const form = document.createElement('form');
+        form.appendChild(firstControl);
+        form.appendChild(secondControl);
+        document.body.appendChild(form);
+
+        firstControl.required = true;
+        secondControl.required = true;
+        secondControl.reportValidity();
+
+        expect(firstControl.focus)
+          .withContext('first control (invalid) is not focused')
+          .not.toHaveBeenCalled();
+        expect(secondControl.focus)
+          .withContext(
+            'second control (invalid, called reportValidity()) is focused',
+          )
+          .toHaveBeenCalledTimes(1);
+      });
+    });
   });
 });

select/internal/select.ts

@@ -308,16 +308,8 @@ export abstract class Select extends selectBaseClass {
   }
 
   [onReportValidity](invalidEvent: Event | null) {
-    if (invalidEvent?.defaultPrevented) {
-      return;
-    }
-
-    if (invalidEvent) {
-      // Prevent default pop-up behavior. This also prevents focusing, so we
-      // manually focus.
-      invalidEvent.preventDefault();
-      this.focus();
-    }
+    // Prevent default pop-up behavior.
+    invalidEvent?.preventDefault();
 
     const prevMessage = this.getErrorText();
     this.nativeError = !!invalidEvent;

textfield/internal/text-field.ts

@@ -772,16 +772,8 @@ export abstract class TextField extends textFieldBaseClass {
   }
 
   [onReportValidity](invalidEvent: Event | null) {
-    if (invalidEvent?.defaultPrevented) {
-      return;
-    }
-
-    if (invalidEvent) {
-      // Prevent default pop-up behavior. This also prevents focusing, so we
-      // manually focus.
-      invalidEvent.preventDefault();
-      this.focus();
-    }
+    // Prevent default pop-up behavior.
+    invalidEvent?.preventDefault();
 
     const prevMessage = this.getErrorText();
     this.nativeError = !!invalidEvent;