From 9973b90981b2414cc18dfb91279f204b7f50c080 Mon Sep 17 00:00:00 2001
From: Elizabeth Mitchell <lizmitchell@google.com>
Date: Wed, 27 Dec 2023 08:12:10 -0800
Subject: [PATCH] fix(textfield): counter showing when max length is 0 or
 removed

Fixes #4998

This also fixes an error being thrown in text field's validator when minlength/maxlength change to out of bounds if they're not set in the correct order.

PiperOrigin-RevId: 594013553
---
 field/internal/field.ts                       |  9 ++++--
 .../validators/text-field-validator.ts        | 16 +++++++---
 .../validators/text-field-validator_test.ts   | 32 +++++++++++++++++++
 textfield/internal/text-field.ts              | 12 ++++---
 4 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/field/internal/field.ts b/field/internal/field.ts
index 1ee21e56e..e60ee26a9 100644
--- a/field/internal/field.ts
+++ b/field/internal/field.ts
@@ -47,11 +47,16 @@ export class Field extends LitElement {
   private readonly slottedAriaDescribedBy!: HTMLElement[];
 
   private get counterText() {
-    if (this.count < 0 || this.max < 0) {
+    // Count and max are typed as number, but can be set to null when Lit removes
+    // their attributes. These getters coerce back to a number for calculations.
+    const countAsNumber = this.count ?? -1;
+    const maxAsNumber = this.max ?? -1;
+    // Counter does not show if count is negative, or max is negative or 0.
+    if (countAsNumber < 0 || maxAsNumber <= 0) {
       return '';
     }
 
-    return `${this.count} / ${this.max}`;
+    return `${countAsNumber} / ${maxAsNumber}`;
   }
 
   private get supportingOrErrorText() {
diff --git a/labs/behaviors/validators/text-field-validator.ts b/labs/behaviors/validators/text-field-validator.ts
index 256f04f55..4d4bedffb 100644
--- a/labs/behaviors/validators/text-field-validator.ts
+++ b/labs/behaviors/validators/text-field-validator.ts
@@ -181,14 +181,22 @@ export class TextFieldValidator extends Validator<TextFieldState> {
     // Use -1 to represent no minlength and maxlength, which is what the
     // platform input returns. However, it will throw an error if you try to
     // manually set it to -1.
-    if (state.minLength > -1) {
-      inputOrTextArea.minLength = state.minLength;
+    //
+    // While the type is `number`, it may actually be `null` at runtime.
+    // `null > -1` is true since `null` coerces to `0`, so we default null and
+    // undefined to -1.
+    //
+    // We set attributes instead of properties since setting a property may
+    // throw an out of bounds error in relation to the other property.
+    // Attributes will not throw errors while the state is updating.
+    if ((state.minLength ?? -1) > -1) {
+      inputOrTextArea.setAttribute('minlength', String(state.minLength));
     } else {
       inputOrTextArea.removeAttribute('minlength');
     }
 
-    if (state.maxLength > -1) {
-      inputOrTextArea.maxLength = state.maxLength;
+    if ((state.maxLength ?? -1) > -1) {
+      inputOrTextArea.setAttribute('maxlength', String(state.maxLength));
     } else {
       inputOrTextArea.removeAttribute('maxlength');
     }
diff --git a/labs/behaviors/validators/text-field-validator_test.ts b/labs/behaviors/validators/text-field-validator_test.ts
index 14d608fd0..12148bbf0 100644
--- a/labs/behaviors/validators/text-field-validator_test.ts
+++ b/labs/behaviors/validators/text-field-validator_test.ts
@@ -89,6 +89,38 @@ describe('TextFieldValidator', () => {
       expect(validity.valueMissing).withContext('valueMissing').toBeFalse();
       expect(validationMessage).withContext('validationMessage').toBe('');
     });
+
+    it('does not throw an error when setting minlength and maxlength out of bounds', () => {
+      type WritableInputState = {
+        -readonly [K in keyof InputState]: InputState[K];
+      };
+
+      const state: WritableInputState = {
+        type: 'text',
+        value: '',
+        required: true,
+        pattern: '',
+        min: '',
+        max: '',
+        minLength: 5,
+        maxLength: 10,
+        step: '',
+      };
+
+      const validator = new TextFieldValidator(() => ({
+        state,
+        renderedControl: null,
+      }));
+
+      // Compute initial validity with valid minlength of 5 and maxlength of 10
+      validator.getValidity();
+      // set to something that is out of bounds of current maxlength="10"
+      state.minLength = 20;
+
+      expect(() => {
+        validator.getValidity();
+      }).not.toThrow();
+    });
   });
 
   describe('type="email"', () => {
diff --git a/textfield/internal/text-field.ts b/textfield/internal/text-field.ts
index 98bab68b6..fcb9ef131 100644
--- a/textfield/internal/text-field.ts
+++ b/textfield/internal/text-field.ts
@@ -585,6 +585,10 @@ export abstract class TextField extends textFieldBaseClass {
     // tslint:disable-next-line:no-any
     const autocomplete = this.autocomplete as any;
 
+    // These properties may be set to null if the attribute is removed, and
+    // `null > -1` is incorrectly `true`.
+    const hasMaxLength = (this.maxLength ?? -1) > -1;
+    const hasMinLength = (this.minLength ?? -1) > -1;
     if (this.type === 'textarea') {
       return html`
         <textarea
@@ -595,8 +599,8 @@ export abstract class TextField extends textFieldBaseClass {
           aria-label=${ariaLabel}
           autocomplete=${autocomplete || nothing}
           ?disabled=${this.disabled}
-          maxlength=${this.maxLength > -1 ? this.maxLength : nothing}
-          minlength=${this.minLength > -1 ? this.minLength : nothing}
+          maxlength=${hasMaxLength ? this.maxLength : nothing}
+          minlength=${hasMinLength ? this.minLength : nothing}
           placeholder=${this.placeholder || nothing}
           ?readonly=${this.readOnly}
           ?required=${this.required}
@@ -631,9 +635,9 @@ export abstract class TextField extends textFieldBaseClass {
           ?disabled=${this.disabled}
           inputmode=${inputMode || nothing}
           max=${(this.max || nothing) as unknown as number}
-          maxlength=${this.maxLength > -1 ? this.maxLength : nothing}
+          maxlength=${hasMaxLength ? this.maxLength : nothing}
           min=${(this.min || nothing) as unknown as number}
-          minlength=${this.minLength > -1 ? this.minLength : nothing}
+          minlength=${hasMinLength ? this.minLength : nothing}
           pattern=${this.pattern || nothing}
           placeholder=${this.placeholder || nothing}
           ?readonly=${this.readOnly}