From e349fe4d7c1cca132cc12ff971c19efd8ff4d401 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Fri, 15 Jun 2018 22:22:33 +0200
Subject: [PATCH] fix(select): label not being read out when using mat-label in
 mat-form-field (#11710)

Fixes the `mat-select` label not being read out by screen readers, if it's specified via a `mat-label` on a `mat-form-field`.
---
 src/lib/form-field/form-field.html |  1 +
 src/lib/form-field/form-field.ts   |  3 +++
 src/lib/select/select.spec.ts      | 39 ++++++++++++++++++++++++++++++
 src/lib/select/select.ts           | 23 +++++++++++++++---
 4 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/src/lib/form-field/form-field.html b/src/lib/form-field/form-field.html
index 13254c0732e..4323d75841c 100644
--- a/src/lib/form-field/form-field.html
+++ b/src/lib/form-field/form-field.html
@@ -12,6 +12,7 @@
         <!-- We add aria-owns as a workaround for an issue in JAWS & NVDA where the label isn't
              read if it comes before the control in the DOM. -->
         <label class="mat-form-field-label"
+               [id]="_labelId"
                [attr.for]="_control.id"
                [attr.aria-owns]="_control.id"
                [class.mat-empty]="_control.empty && !_shouldAlwaysFloat"
diff --git a/src/lib/form-field/form-field.ts b/src/lib/form-field/form-field.ts
index c9742ac89e1..197f0c05fba 100644
--- a/src/lib/form-field/form-field.ts
+++ b/src/lib/form-field/form-field.ts
@@ -186,6 +186,9 @@ export class MatFormField extends _MatFormFieldMixinBase
   // Unique id for the hint label.
   _hintLabelId: string = `mat-hint-${nextUniqueId++}`;
 
+  // Unique id for the internal form field label.
+  _labelId = `mat-form-field-label-${nextUniqueId++}`;
+
   /**
    * Whether the label should always float, never float or float as the user types.
    *
diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts
index 72f3debc0cd..4edeb9c0fcb 100644
--- a/src/lib/select/select.spec.ts
+++ b/src/lib/select/select.spec.ts
@@ -124,6 +124,7 @@ describe('MatSelect', () => {
         MultiSelect,
         SelectWithGroups,
         SelectWithGroupsAndNgContainer,
+        SelectWithFormFieldLabel,
       ]);
     }));
 
@@ -230,6 +231,29 @@ describe('MatSelect', () => {
           expect(select.getAttribute('tabindex')).toEqual('0');
         }));
 
+        it('should set `aria-labelledby` to form field label if there is no placeholder', () => {
+          fixture.destroy();
+
+          const labelFixture = TestBed.createComponent(SelectWithFormFieldLabel);
+          labelFixture.detectChanges();
+          select = labelFixture.debugElement.query(By.css('mat-select')).nativeElement;
+
+          expect(select.getAttribute('aria-labelledby')).toBeTruthy();
+          expect(select.getAttribute('aria-labelledby'))
+              .toBe(labelFixture.nativeElement.querySelector('label').getAttribute('id'));
+        });
+
+        it('should not set `aria-labelledby` if there is a placeholder', () => {
+          fixture.destroy();
+
+          const labelFixture = TestBed.createComponent(SelectWithFormFieldLabel);
+          labelFixture.componentInstance.placeholder = 'Thing selector';
+          labelFixture.detectChanges();
+          select = labelFixture.debugElement.query(By.css('mat-select')).nativeElement;
+
+          expect(select.getAttribute('aria-labelledby')).toBeFalsy();
+        });
+
         it('should select options via the UP/DOWN arrow keys on a closed select', fakeAsync(() => {
           const formControl = fixture.componentInstance.control;
           const options = fixture.componentInstance.options.toArray();
@@ -4561,3 +4585,18 @@ class SelectWithoutOptionCentering {
   @ViewChild(MatSelect) select: MatSelect;
   @ViewChildren(MatOption) options: QueryList<MatOption>;
 }
+
+@Component({
+  template: `
+    <mat-form-field>
+      <mat-label>Select a thing</mat-label>
+
+      <mat-select [placeholder]="placeholder">
+        <mat-option value="thing">A thing</mat-option>
+      </mat-select>
+    </mat-form-field>
+  `
+})
+class SelectWithFormFieldLabel {
+  placeholder: string;
+}
diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts
index 92db23b2a12..71133aa039a 100644
--- a/src/lib/select/select.ts
+++ b/src/lib/select/select.ts
@@ -189,8 +189,8 @@ export class MatSelectTrigger {}
     'role': 'listbox',
     '[attr.id]': 'id',
     '[attr.tabindex]': 'tabIndex',
-    '[attr.aria-label]': '_ariaLabel',
-    '[attr.aria-labelledby]': 'ariaLabelledby',
+    '[attr.aria-label]': '_getAriaLabel()',
+    '[attr.aria-labelledby]': '_getAriaLabelledby()',
     '[attr.aria-required]': 'required.toString()',
     '[attr.aria-disabled]': 'disabled.toString()',
     '[attr.aria-invalid]': 'errorState',
@@ -1017,12 +1017,27 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
   }
 
   /** Returns the aria-label of the select component. */
-  get _ariaLabel(): string | null {
-    // If an ariaLabelledby value has been set, the select should not overwrite the
+  _getAriaLabel(): string | null {
+    // If an ariaLabelledby value has been set by the consumer, the select should not overwrite the
     // `aria-labelledby` value by setting the ariaLabel to the placeholder.
     return this.ariaLabelledby ? null : this.ariaLabel || this.placeholder;
   }
 
+  /** Returns the aria-labelledby of the select component. */
+  _getAriaLabelledby(): string | null {
+    if (this.ariaLabelledby) {
+      return this.ariaLabelledby;
+    }
+
+    // Note: we use `_getAriaLabel` here, because we want to check whether there's a
+    // computed label. `this.ariaLabel` is only the user-specified label.
+    if (!this._parentFormField || this._getAriaLabel()) {
+      return null;
+    }
+
+    return this._parentFormField._labelId || null;
+  }
+
   /** Determines the `aria-activedescendant` to be set on the host. */
   _getAriaActiveDescendant(): string | null {
     if (this.panelOpen && this._keyManager && this._keyManager.activeItem) {