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

crisbeto · 2018-03-18T11:25:06Z

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.

mmalerba · 2018-03-19T17:44:11Z

src/lib/input/input.ts

@@ -299,6 +300,14 @@ 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. */
+  _getNativePlaceholderValue() {


Add a note that we can get rid of this once we remove the legacy form-field style, since its the only style the supports promoting the placeholder to a label

mmalerba · 2019-06-10T22:40:39Z

needs rebase

andrewseguin · 2019-07-09T18:47:45Z

src/material/input/input.ts

+    // 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 : null;


Small issue found in testing that needs to be resolved before we can merge.

This function calls the form field's _hideControlPlaceholder(), which checks the control's _shouldLabelFloat. In the case where the control is MatChipList, that property is a getter:

get shouldLabelFloat(): boolean { return !this.empty || this.focused; }

It checks the empty property of the MatChipList, which is also a getter:

get empty(): boolean { return (!this._chipInput || this._chipInput.empty) && this.chips.length === 0; }

Here we find that this.chips.length throws an exception because this.chips is undefined since the content of the list has not yet been initialized.

Here's a reproduction of the issue we are seeing: https://stackblitz.com/edit/components-issue-2obaho?file=app%2Fapp.component.ts

It's using the same hook timing of the placeholder host binding, and you can see in the debug console that an exception is thrown when calling _hideControlPlaceholder()

crisbeto · 2019-07-09T20:24:04Z

Thank you for looking into it @andrewseguin. I've added an extra null check for the chips and fixed the compilation errors.

andrewseguin · 2020-06-30T17:17:00Z

Looks like there seems to be a size change causing the test to fail

crisbeto · 2020-06-30T17:56:37Z

Fixed.

andrewseguin · 2020-06-30T21:50:24Z

Looks like another size error came up

andrewseguin · 2020-07-01T17:27:59Z

There are roughly 100-200 tests internally that fail due to selectors like input[placeholder="First name"]. The easiest way to fix these would be to continue having some attr data on the input that includes the placeholder, e.g. input[data-placeholder="First name"].

If that attribute is added ahead of this PR, then the tests could be migrated away from using the placeholder as a way to select inputs. Then this can be merged.

Ideally those tests use harnesses, but it will likely be a very long time until all those tests are migrated.

This is a prerequisite to make landing angular#10466 easier in g3. Mirrors the native placeholder value in a data attribute so that we can read it even when it has been removed.

This is a prerequisite to make landing #10466 easier in g3. Mirrors the native placeholder value in a data attribute so that we can read it even when it has been removed.

andrewseguin · 2020-07-02T15:12:21Z

We're seeing some cases of ExpressionChangedAfterChecked - you can see a minimal repro here: crisbeto#11

crisbeto · 2020-07-05T10:59:58Z

Thank you for the repro @andrewseguin, I've updated the PR to work around the test failure.

This is a prerequisite to make landing #10466 easier in g3. Mirrors the native placeholder value in a data attribute so that we can read it even when it has been removed.

andrewseguin · 2020-07-13T02:58:29Z

Needs rebase

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 angular#9721.

crisbeto · 2020-07-13T03:28:37Z

Rebased.

…ngular#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 angular#9721.

angular-automatic-lock-bot · 2020-08-14T00:08:33Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

crisbeto added the pr: needs review label Mar 18, 2018

crisbeto assigned mmalerba Mar 18, 2018

crisbeto requested a review from mmalerba as a code owner March 18, 2018 11:25

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 18, 2018

crisbeto added the Accessibility This issue is related to accessibility (a11y) label Mar 18, 2018

mmalerba reviewed Mar 19, 2018

View reviewed changes

crisbeto force-pushed the 9721/input-clear-placeholder branch from 4cd2c80 to a7cd75a Compare March 19, 2018 18:03

mmalerba approved these changes Mar 19, 2018

View reviewed changes

mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Mar 19, 2018

crisbeto added the target: patch This PR is targeted for the next patch release label Mar 19, 2018

crisbeto force-pushed the 9721/input-clear-placeholder branch from a7cd75a to 522e174 Compare March 29, 2018 14:24

crisbeto force-pushed the 9721/input-clear-placeholder branch 2 times, most recently from 824f3c5 to 7e8eaa0 Compare April 9, 2018 20:18

andrewseguin added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Apr 26, 2018

crisbeto force-pushed the 9721/input-clear-placeholder branch 2 times, most recently from 2eef585 to ebd9210 Compare September 4, 2018 20:48

crisbeto force-pushed the 9721/input-clear-placeholder branch from ebd9210 to 0a2b75b Compare September 25, 2018 21:17

mmalerba added aaa and removed aaa labels Apr 25, 2019

andrewseguin added the P2 The issue is important to a large percentage of users, with a workaround label May 30, 2019

crisbeto force-pushed the 9721/input-clear-placeholder branch from 0a2b75b to c9bfb11 Compare June 30, 2019 09:16

andrewseguin reviewed Jul 9, 2019

View reviewed changes

andrewseguin removed the action: merge The PR is ready for merge by the caretaker label Jul 9, 2019

crisbeto force-pushed the 9721/input-clear-placeholder branch from c9bfb11 to 7d7a7ae Compare July 9, 2019 20:21

crisbeto requested a review from jelbourn as a code owner July 9, 2019 20:21

crisbeto added the action: merge The PR is ready for merge by the caretaker label Jul 9, 2019

crisbeto force-pushed the 9721/input-clear-placeholder branch from c4dca9d to 1538051 Compare June 30, 2020 17:56

crisbeto requested a review from a team as a code owner June 30, 2020 17:56

crisbeto force-pushed the 9721/input-clear-placeholder branch 2 times, most recently from cfd81c3 to 11d40a6 Compare July 1, 2020 17:52

crisbeto mentioned this pull request Jul 1, 2020

refactor(input): mirror placeholder in data attribute #19831

Merged

crisbeto force-pushed the 9721/input-clear-placeholder branch from 11d40a6 to b258683 Compare July 5, 2020 10:59

andrewseguin added the needs rebase label Jul 13, 2020

crisbeto force-pushed the 9721/input-clear-placeholder branch from b258683 to d1a64c3 Compare July 13, 2020 03:28

crisbeto removed the needs rebase label Jul 13, 2020

andrewseguin added target: minor This PR is targeted for the next minor release and removed presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged target: patch This PR is targeted for the next patch release labels Jul 14, 2020

andrewseguin merged commit d05ba60 into angular:master Jul 14, 2020

andrewseguin mentioned this pull request Jul 17, 2020

fix(material-experimental/chips): fix empty check when no chips #20025

Merged

angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

crisbeto commented Mar 18, 2018

Uh oh!

mmalerba Mar 19, 2018

Uh oh!

crisbeto Mar 19, 2018

Uh oh!

mmalerba commented Jun 10, 2019

Uh oh!

andrewseguin Jul 9, 2019

Uh oh!

crisbeto commented Jul 9, 2019

Uh oh!

andrewseguin commented Jun 30, 2020

Uh oh!

crisbeto commented Jun 30, 2020

Uh oh!

andrewseguin commented Jun 30, 2020

Uh oh!

andrewseguin commented Jul 1, 2020

Uh oh!

andrewseguin commented Jul 2, 2020 •

edited

Loading

Uh oh!

crisbeto commented Jul 5, 2020

Uh oh!

andrewseguin commented Jul 13, 2020

Uh oh!

crisbeto commented Jul 13, 2020

Uh oh!

angular-automatic-lock-bot bot commented Aug 14, 2020

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

crisbeto commented Mar 18, 2018

Uh oh!

mmalerba Mar 19, 2018

Choose a reason for hiding this comment

Uh oh!

crisbeto Mar 19, 2018

Choose a reason for hiding this comment

Uh oh!

mmalerba commented Jun 10, 2019

Uh oh!

andrewseguin Jul 9, 2019

Choose a reason for hiding this comment

Uh oh!

crisbeto commented Jul 9, 2019

Uh oh!

andrewseguin commented Jun 30, 2020

Uh oh!

crisbeto commented Jun 30, 2020

Uh oh!

andrewseguin commented Jun 30, 2020

Uh oh!

andrewseguin commented Jul 1, 2020

Uh oh!

andrewseguin commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crisbeto commented Jul 5, 2020

Uh oh!

andrewseguin commented Jul 13, 2020

Uh oh!

crisbeto commented Jul 13, 2020

Uh oh!

angular-automatic-lock-bot bot commented Aug 14, 2020

Uh oh!

Uh oh!

andrewseguin commented Jul 2, 2020 •

edited

Loading