fix(input): placeholder being read out twice by some screen readers (#10466)

Currently we hide the native placeholder using CSS while we mirror its value in the form field label, however because the value is still in the DOM, some screen readers will read it out twice: once for the label and once for the attribute. These changes remove the attribute from the DOM if it isn't supposed to be shown.

Fixes #9721.

Author: crisbeto

goldens/size-test.yaml

@@ -1,8 +1,8 @@
 cdk/drag-drop/all-directives: 154096
 cdk/drag-drop/basic: 151353
 material-experimental/mdc-chips/basic: 147607
-material-experimental/mdc-form-field/advanced: 220897
-material-experimental/mdc-form-field/basic: 220060
+material-experimental/mdc-form-field/advanced: 251959
+material-experimental/mdc-form-field/basic: 251127
 material/autocomplete/without-optgroup: 210028
 material/button-toggle/standalone: 119486
 material/chips/basic: 162776

src/material/chips/chip-list.ts

@@ -250,7 +250,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
    * @docs-private
    */
   get empty(): boolean {
-    return (!this._chipInput || this._chipInput.empty) && this.chips.length === 0;
+    return (!this._chipInput || this._chipInput.empty) && (!this.chips || this.chips.length === 0);
   }
 
   /**
@@ -801,7 +801,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
 
   /** Checks whether any of the chips is focused. */
   private _hasFocusedChip() {
-    return this.chips.some(chip => chip._hasFocus);
+    return this.chips && this.chips.some(chip => chip._hasFocus);
   }
 
   /** Syncs the list's state with the individual chips. */

src/material/form-field/form-field.ts

@@ -411,7 +411,8 @@ export class MatFormField extends _MatFormFieldMixinBase
   }
 
   _shouldLabelFloat() {
-    return this._canLabelFloat && (this._control.shouldLabelFloat || this._shouldAlwaysFloat);
+    return this._canLabelFloat &&
+        ((this._control && this._control.shouldLabelFloat) || this._shouldAlwaysFloat);
   }
 
   _hideControlPlaceholder() {

src/material/input/input.spec.ts

@@ -367,21 +367,30 @@ describe('MatInput without forms', () => {
     let fixture = createComponent(MatInputPlaceholderAttrTestComponent);
     fixture.detectChanges();
 
-    let inputEl = fixture.debugElement.query(By.css('input'))!.nativeElement;
-
     expect(fixture.debugElement.query(By.css('label'))).toBeNull();
-    expect(inputEl.placeholder).toBe('');
 
     fixture.componentInstance.placeholder = 'Other placeholder';
     fixture.detectChanges();
 
     let labelEl = fixture.debugElement.query(By.css('label'))!;
 
-    expect(inputEl.placeholder).toBe('Other placeholder');
     expect(labelEl).not.toBeNull();
     expect(labelEl.nativeElement.textContent).toBe('Other placeholder');
   }));
 
+  it('should not render the native placeholder when its value is mirrored in the label',
+    fakeAsync(() => {
+      const fixture = createComponent(MatInputPlaceholderAttrTestComponent);
+      fixture.componentInstance.placeholder = 'Enter a name';
+      fixture.detectChanges();
+
+      const inputEl = fixture.debugElement.query(By.css('input')).nativeElement;
+      const labelEl = fixture.debugElement.query(By.css('label'));
+
+      expect(inputEl.hasAttribute('placeholder')).toBe(false);
+      expect(labelEl.nativeElement.textContent).toContain('Enter a name');
+    }));
+
   it('supports placeholder element', fakeAsync(() => {
     let fixture = createComponent(MatInputPlaceholderElementTestComponent);
     fixture.detectChanges();
@@ -911,8 +920,8 @@ describe('MatInput without forms', () => {
 
     expect(container.classList).toContain('mat-form-field-hide-placeholder');
     expect(container.classList).not.toContain('mat-form-field-should-float');
-    expect(label.textContent).toBe('Label');
-    expect(input.getAttribute('placeholder')).toBe('Placeholder');
+    expect(label.textContent.trim()).toBe('Label');
+    expect(input.hasAttribute('placeholder')).toBe(false);
 
     input.value = 'Value';
     fixture.detectChanges();
@@ -981,6 +990,12 @@ describe('MatInput without forms', () => {
       expect(label.classList).not.toContain('mat-form-field-empty');
     }));
 
+  it('should not throw when there is a default ngIf on the label element', fakeAsync(() => {
+    expect(() => {
+      createComponent(MatInputWithDefaultNgIf).detectChanges();
+    }).not.toThrow();
+  }));
+
 });
 
 describe('MatInput with forms', () => {
@@ -2221,3 +2236,18 @@ class CustomMatInputAccessor {
   set value(_value: any) {}
   private _value = null;
 }
+
+
+// Note that the DOM structure is slightly weird, but it's
+// testing a specific g3 issue. See the discussion on #10466.
+@Component({
+  template: `
+    <mat-form-field appearance="outline">
+      <mat-label *ngIf="true">My Label</mat-label>
+      <ng-container *ngIf="true">
+        <input matInput>
+      </ng-container>
+    </mat-form-field>
+  `
+})
+class MatInputWithDefaultNgIf {}

src/material/input/input.ts

@@ -30,7 +30,7 @@ import {
   ErrorStateMatcher,
   mixinErrorState,
 } from '@angular/material/core';
-import {MatFormFieldControl} from '@angular/material/form-field';
+import {MatFormFieldControl, MatFormField} from '@angular/material/form-field';
 import {Subject} from 'rxjs';
 import {getMatInputUnsupportedTypeError} from './input-errors';
 import {MAT_INPUT_VALUE_ACCESSOR} from './input-value-accessor';
@@ -77,7 +77,10 @@ const _MatInputMixinBase: CanUpdateErrorStateCtor & typeof MatInputBase =
     // Native input properties that are overwritten by Angular inputs need to be synced with
     // the native input element. Otherwise property bindings for those don't work.
     '[attr.id]': 'id',
-    '[attr.placeholder]': 'placeholder',
+    '[attr.placeholder]': '_getPlaceholderAttribute()',
+    // At the time of writing, we have a lot of customer tests that look up the input based on its
+    // placeholder. Since we sometimes omit the placeholder attribute from the DOM to prevent screen
+    // readers from reading it twice, we have to keep it somewhere in the DOM for the lookup.
     '[attr.data-placeholder]': 'placeholder',
     '[disabled]': 'disabled',
     '[required]': 'required',
@@ -233,8 +236,9 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
     _defaultErrorStateMatcher: ErrorStateMatcher,
     @Optional() @Self() @Inject(MAT_INPUT_VALUE_ACCESSOR) inputValueAccessor: any,
     private _autofillMonitor: AutofillMonitor,
-    ngZone: NgZone) {
-
+    ngZone: NgZone,
+    // @breaking-change 8.0.0 `_formField` parameter to be made required.
+    @Optional() private _formField?: MatFormField) {
     super(_defaultErrorStateMatcher, _parentForm, _parentFormGroup, ngControl);
 
     const element = this._elementRef.nativeElement;
@@ -350,6 +354,16 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
     // FormsModule or ReactiveFormsModule, because Angular forms also listens to input events.
   }
 
+  /** Determines the value of the native `placeholder` attribute that should be used in the DOM. */
+  _getPlaceholderAttribute() {
+    // If we're hiding the native placeholder, it should also be cleared from the DOM, otherwise
+    // screen readers will read it out twice: once from the label and once from the attribute.
+    // TODO: can be removed once we get rid of the `legacy` style for the form field, because it's
+    // the only one that supports promoting the placeholder to a label.
+    const formField = this._formField;
+    return (!formField || !formField._hideControlPlaceholder()) ? this.placeholder : undefined;
+  }
+
   /** Does some manual dirty checking on the native input `value` property. */
   protected _dirtyCheckNativeValue() {
     const newValue = this._elementRef.nativeElement.value;

src/material/input/testing/input-harness.ts

@@ -71,8 +71,12 @@ export class MatInputHarness extends MatFormFieldControlHarness {
 
   /** Gets the placeholder of the input. */
   async getPlaceholder(): Promise<string> {
-    // The "placeholder" property of the native input is never undefined.
-    return (await (await this.host()).getProperty('placeholder'))!;
+    const host = await this.host();
+    const [nativePlaceholder, fallback] = await Promise.all([
+      host.getProperty('placeholder'),
+      host.getAttribute('data-placeholder')
+    ]);
+    return nativePlaceholder || fallback || '';
   }
 
   /** Gets the id of the input. */

tools/public_api_guard/material/input.d.ts

@@ -40,9 +40,10 @@ export declare class MatInput extends _MatInputMixinBase implements MatFormField
     get value(): string;
     set value(value: string);
     constructor(_elementRef: ElementRef<HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement>, _platform: Platform,
-    ngControl: NgControl, _parentForm: NgForm, _parentFormGroup: FormGroupDirective, _defaultErrorStateMatcher: ErrorStateMatcher, inputValueAccessor: any, _autofillMonitor: AutofillMonitor, ngZone: NgZone);
+    ngControl: NgControl, _parentForm: NgForm, _parentFormGroup: FormGroupDirective, _defaultErrorStateMatcher: ErrorStateMatcher, inputValueAccessor: any, _autofillMonitor: AutofillMonitor, ngZone: NgZone, _formField?: MatFormField | undefined);
     protected _dirtyCheckNativeValue(): void;
     _focusChanged(isFocused: boolean): void;
+    _getPlaceholderAttribute(): string | undefined;
     protected _isBadInput(): boolean;
     protected _isNeverEmpty(): boolean;
     _onInput(): void;
@@ -59,7 +60,7 @@ export declare class MatInput extends _MatInputMixinBase implements MatFormField
     static ngAcceptInputType_required: BooleanInput;
     static ngAcceptInputType_value: any;
     static ɵdir: i0.ɵɵDirectiveDefWithMeta<MatInput, "input[matInput], textarea[matInput], select[matNativeControl],      input[matNativeControl], textarea[matNativeControl]", ["matInput"], { "disabled": "disabled"; "id": "id"; "placeholder": "placeholder"; "required": "required"; "type": "type"; "errorStateMatcher": "errorStateMatcher"; "value": "value"; "readonly": "readonly"; }, {}, never>;
-    static ɵfac: i0.ɵɵFactoryDef<MatInput, [null, null, { optional: true; self: true; }, { optional: true; }, { optional: true; }, null, { optional: true; self: true; }, null, null]>;
+    static ɵfac: i0.ɵɵFactoryDef<MatInput, [null, null, { optional: true; self: true; }, { optional: true; }, { optional: true; }, null, { optional: true; self: true; }, null, null, { optional: true; }]>;
 }
 
 export declare class MatInputModule {