From c53a4194e9796b5bd862ac166e66f0c16d4d8bcd Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Thu, 30 Nov 2023 15:59:04 -0800 Subject: [PATCH] fix(behaviors): validation not reporting when form tries to submit We handle `form.reportValidity()` with hooks, but we weren't handling `form.submit()`. PiperOrigin-RevId: 586813244 --- labs/behaviors/on-report-validity.ts | 69 +++++++++++++++++++---- labs/behaviors/on-report-validity_test.ts | 34 ++++++++++- 2 files changed, 88 insertions(+), 15 deletions(-) diff --git a/labs/behaviors/on-report-validity.ts b/labs/behaviors/on-report-validity.ts index 01834e82a8..71f0401261 100644 --- a/labs/behaviors/on-report-validity.ts +++ b/labs/behaviors/on-report-validity.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {LitElement} from 'lit'; +import {LitElement, isServer} from 'lit'; import {ConstraintValidation} from './constraint-validation.js'; import {MixinBase, MixinReturn} from './mixin.js'; @@ -46,6 +46,7 @@ export const onReportValidity = Symbol('onReportValidity'); // Private symbol members, used to avoid name clashing. const privateCleanupFormListeners = Symbol('privateCleanupFormListeners'); +const privateDoNotReportInvalid = Symbol('privateDoNotReportInvalid'); /** * Mixes in a callback for constraint validation when validity should be @@ -91,23 +92,67 @@ export function mixinOnReportValidity< */ [privateCleanupFormListeners] = new AbortController(); - override reportValidity() { - let invalidEvent = null as Event | null; - const cleanupInvalidListener = new AbortController(); + /** + * Used to determine if an invalid event should report validity. Invalid + * events from `checkValidity()` do not trigger reporting. + */ + [privateDoNotReportInvalid] = false; + + // Mixins must have a constructor with `...args: any[]` + // tslint:disable-next-line:no-any + constructor(...args: any[]) { + super(...args); + if (isServer) { + return; + } + this.addEventListener( 'invalid', - (event) => { - invalidEvent = event; + (invalidEvent) => { + // Listen for invalid events dispatched by a `
` when it tries to + // submit and the element is invalid. We ignore events dispatched when + // calling `checkValidity()` as well as untrusted events, since the + // `reportValidity()` and ``-dispatched events are always + // trusted. + if (this[privateDoNotReportInvalid] || !invalidEvent.isTrusted) { + return; + } + + this.addEventListener( + 'invalid', + () => { + // 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); + } + }, + {once: true}, + ); + }, + { + // Listen during the capture phase, which will happen before the + // bubbling phase. That way, we can add a final event listener that + // will run after other event listeners, and we can check if it was + // default prevented. This works because invalid does not bubble. + capture: true, }, - {signal: cleanupInvalidListener.signal}, ); + } + + override checkValidity() { + this[privateDoNotReportInvalid] = true; + const valid = super.checkValidity(); + this[privateDoNotReportInvalid] = false; + return valid; + } + override reportValidity() { const valid = super.reportValidity(); - cleanupInvalidListener.abort(); - // event may be null, so check for strict `true`. If null it should still - // be reported. - if (invalidEvent?.defaultPrevented !== true) { - this[onReportValidity](invalidEvent); + // Constructor's invalid listener will handle reporting invalid events. + if (valid) { + this[onReportValidity](null); } return valid; diff --git a/labs/behaviors/on-report-validity_test.ts b/labs/behaviors/on-report-validity_test.ts index 17b3b56353..631c6a02f4 100644 --- a/labs/behaviors/on-report-validity_test.ts +++ b/labs/behaviors/on-report-validity_test.ts @@ -15,14 +15,17 @@ import { mixinConstraintValidation, } from './constraint-validation.js'; import {mixinElementInternals} from './element-internals.js'; +import {mixinFocusable} from './focusable.js'; import {getFormValue, mixinFormAssociated} from './form-associated.js'; import {mixinOnReportValidity, onReportValidity} from './on-report-validity.js'; import {CheckboxValidator} from './validators/checkbox-validator.js'; describe('mixinOnReportValidity()', () => { - const baseClass = mixinOnReportValidity( - mixinConstraintValidation( - mixinFormAssociated(mixinElementInternals(LitElement)), + const baseClass = mixinFocusable( + mixinOnReportValidity( + mixinConstraintValidation( + mixinFormAssociated(mixinElementInternals(LitElement)), + ), ), ); @@ -139,6 +142,31 @@ describe('mixinOnReportValidity()', () => { expect(control[onReportValidity]).toHaveBeenCalledWith(null); }); + it('should be called with invalid event when invalid form tries to submit', () => { + const control = new TestOnReportValidity(); + control[onReportValidity] = jasmine.createSpy('onReportValidity'); + const form = document.createElement('form'); + form.appendChild(control); + form.addEventListener( + 'submit', + (event) => { + // Prevent the test page from actually reloading. This shouldn't + // happen, but we add it just in case the control fails and reports + // as valid and the form tries to submit. + event.preventDefault(); + }, + {capture: true}, + ); + + document.body.appendChild(form); + control.required = true; + form.requestSubmit(); + form.remove(); + expect(control[onReportValidity]).toHaveBeenCalledWith( + jasmine.any(Event), + ); + }); + it('should clean up when form is unassociated and not call when non-parent form.reportValidity() is called', () => { const control = new TestOnReportValidity(); control[onReportValidity] = jasmine.createSpy('onReportValidity');