Skip to content

Commit 4cd2c80

Browse files
committed
fix(input): placeholder being read out twice by some screen readers
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.
1 parent c28549d commit 4cd2c80

File tree

2 files changed

+27
-7
lines changed

2 files changed

+27
-7
lines changed

src/lib/input/input.spec.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -373,26 +373,37 @@ describe('MatInput without forms', () => {
373373
expect(hint.getAttribute('id')).toBeTruthy();
374374
}));
375375

376-
it('supports placeholder attribute', fakeAsync(() => {
376+
it('should mirror the placeholder attribute into the label', fakeAsync(() => {
377377
let fixture = TestBed.createComponent(MatInputPlaceholderAttrTestComponent);
378378
fixture.detectChanges();
379379

380380
let inputEl = fixture.debugElement.query(By.css('input')).nativeElement;
381381

382382
expect(fixture.debugElement.query(By.css('label'))).toBeNull();
383-
expect(inputEl.placeholder).toBe('');
384383

385384
fixture.componentInstance.placeholder = 'Other placeholder';
386385
fixture.detectChanges();
387386

388387
let labelEl = fixture.debugElement.query(By.css('label'));
389388

390-
expect(inputEl.placeholder).toBe('Other placeholder');
391389
expect(labelEl).not.toBeNull();
392390
expect(labelEl.nativeElement.textContent).toMatch('Other placeholder');
393391
expect(labelEl.nativeElement.textContent).not.toMatch(/\*/g);
394392
}));
395393

394+
it('should not render the native placeholder when its value is mirrored in the label',
395+
fakeAsync(() => {
396+
const fixture = TestBed.createComponent(MatInputPlaceholderAttrTestComponent);
397+
fixture.componentInstance.placeholder = 'Enter a name';
398+
fixture.detectChanges();
399+
400+
const inputEl = fixture.debugElement.query(By.css('input')).nativeElement;
401+
const labelEl = fixture.debugElement.query(By.css('label'));
402+
403+
expect(inputEl.hasAttribute('placeholder')).toBe(false);
404+
expect(labelEl.nativeElement.textContent).toContain('Enter a name');
405+
}));
406+
396407
it('supports placeholder element', fakeAsync(() => {
397408
let fixture = TestBed.createComponent(MatInputPlaceholderElementTestComponent);
398409
fixture.detectChanges();
@@ -779,7 +790,7 @@ describe('MatInput without forms', () => {
779790
expect(container.classList).toContain('mat-form-field-hide-placeholder');
780791
expect(container.classList).not.toContain('mat-form-field-should-float');
781792
expect(label.textContent.trim()).toBe('Label');
782-
expect(input.getAttribute('placeholder')).toBe('Placeholder');
793+
expect(input.hasAttribute('placeholder')).toBe(false);
783794

784795
input.value = 'Value';
785796
fixture.detectChanges();

src/lib/input/input.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
} from '@angular/core';
2323
import {FormGroupDirective, NgControl, NgForm} from '@angular/forms';
2424
import {CanUpdateErrorState, ErrorStateMatcher, mixinErrorState} from '@angular/material/core';
25-
import {MatFormFieldControl} from '@angular/material/form-field';
25+
import {MatFormField, MatFormFieldControl} from '@angular/material/form-field';
2626
import {Subject} from 'rxjs/Subject';
2727
import {AutofillMonitor} from '@angular/cdk/text-field';
2828
import {getMatInputUnsupportedTypeError} from './input-errors';
@@ -68,7 +68,7 @@ export const _MatInputMixinBase = mixinErrorState(MatInputBase);
6868
// Native input properties that are overwritten by Angular inputs need to be synced with
6969
// the native input element. Otherwise property bindings for those don't work.
7070
'[attr.id]': 'id',
71-
'[attr.placeholder]': 'placeholder',
71+
'[attr.placeholder]': '_getNativePlaceholderValue()',
7272
'[disabled]': 'disabled',
7373
'[required]': 'required',
7474
'[readonly]': 'readonly',
@@ -218,7 +218,8 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
218218
@Optional() _parentFormGroup: FormGroupDirective,
219219
_defaultErrorStateMatcher: ErrorStateMatcher,
220220
@Optional() @Self() @Inject(MAT_INPUT_VALUE_ACCESSOR) inputValueAccessor: any,
221-
private _autofillMonitor: AutofillMonitor) {
221+
private _autofillMonitor: AutofillMonitor,
222+
@Optional() private _formField?: MatFormField) {
222223
super(_defaultErrorStateMatcher, _parentForm, _parentFormGroup, ngControl);
223224
// If no input value accessor was explicitly specified, use the element as the input value
224225
// accessor.
@@ -299,6 +300,14 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
299300
// FormsModule or ReactiveFormsModule, because Angular forms also listens to input events.
300301
}
301302

303+
/** Determines the value of the native `placeholder` attribute that should be used in the DOM. */
304+
_getNativePlaceholderValue() {
305+
// If we're hiding the native placeholder, it should also be cleared from the DOM, otherwise
306+
// screen readers will read it out twice: once from the label and once from the attribute.
307+
const formField = this._formField;
308+
return (!formField || !formField._hideControlPlaceholder()) ? this.placeholder : null;
309+
}
310+
302311
/** Does some manual dirty checking on the native input `value` property. */
303312
protected _dirtyCheckNativeValue() {
304313
const newValue = this.value;

0 commit comments

Comments
 (0)