From 4e2cf9c06268afa983a769dc40ea8431baf66ce0 Mon Sep 17 00:00:00 2001
From: Zach Arend <zachzach@google.com>
Date: Fri, 16 Jun 2023 20:31:07 +0000
Subject: [PATCH] fix(material/chips): remove button role from editable chips

Removes the button role from editable input chips. Fix accessibility
issue in ChipRow where the chip action element is mislabeled as a
button (#27106). Remove butotn role by remove DOM node thtat used to
have button role and using the gridcell role element for the primary
chip action instead.

Tested cross-browser with supported screen readers on MacOS and Windows.
Tested on "Chips with input" example by verifying that a chip could be
added then edited after adding. Also verifying that AT read the chip's
aria description.

Testing Environment
 - macOS 13.4 (22F66) / VoiceOver
   - Chrome Version 114.0.5735.133 (Official Build) (arm64)
   - Firefox 114.0.1 (64-bit)
 - windows 10 Enteprise Version 22H2
   - JAWS VERSION 2020.2006.12 ILM
   - NVDA version 2022.3
   - Chrome Version 114.0.5735.134 (Official Build) (64-bit)
   - Firefox 114.0.2 (64-bit)

Fix #27106
---
 src/material/chips/chip-row.html    | 27 ++++++++++++---------------
 src/material/chips/chip-row.spec.ts | 10 ++++++----
 src/material/chips/chip.scss        |  2 +-
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/src/material/chips/chip-row.html b/src/material/chips/chip-row.html
index 218fdfc181d9..83c8607c2450 100644
--- a/src/material/chips/chip-row.html
+++ b/src/material/chips/chip-row.html
@@ -7,28 +7,25 @@
 </ng-container>
 
 
-<span class="mdc-evolution-chip__cell mdc-evolution-chip__cell--primary" role="gridcell">
-  <span
+<span class="mdc-evolution-chip__cell mdc-evolution-chip__cell--primary" role="gridcell"
     matChipAction
-    [attr.role]="editable ? 'button' : null"
     [tabIndex]="tabIndex"
     [disabled]="disabled"
     [attr.aria-label]="ariaLabel"
     [attr.aria-describedby]="_ariaDescriptionId">
-    <span class="mdc-evolution-chip__graphic mat-mdc-chip-graphic" *ngIf="leadingIcon">
-      <ng-content select="mat-chip-avatar, [matChipAvatar]"></ng-content>
-    </span>
-    <span class="mdc-evolution-chip__text-label mat-mdc-chip-action-label" [ngSwitch]="_isEditing">
-      <ng-container *ngSwitchCase="false"><ng-content></ng-content></ng-container>
+  <span class="mdc-evolution-chip__graphic mat-mdc-chip-graphic" *ngIf="leadingIcon">
+    <ng-content select="mat-chip-avatar, [matChipAvatar]"></ng-content>
+  </span>
+  <span class="mdc-evolution-chip__text-label mat-mdc-chip-action-label" [ngSwitch]="_isEditing">
+    <ng-container *ngSwitchCase="false"><ng-content></ng-content></ng-container>
 
-      <ng-container *ngSwitchCase="true">
-        <ng-content *ngIf="contentEditInput; else defaultMatChipEditInput"
-                    select="[matChipEditInput]"></ng-content>
-        <ng-template #defaultMatChipEditInput><span matChipEditInput></span></ng-template>
-      </ng-container>
+    <ng-container *ngSwitchCase="true">
+      <ng-content *ngIf="contentEditInput; else defaultMatChipEditInput"
+                  select="[matChipEditInput]"></ng-content>
+      <ng-template #defaultMatChipEditInput><span matChipEditInput></span></ng-template>
+    </ng-container>
 
-      <span class="mat-mdc-chip-primary-focus-indicator mat-mdc-focus-indicator"></span>
-    </span>
+    <span class="mat-mdc-chip-primary-focus-indicator mat-mdc-focus-indicator" aria-hidden="true"></span>
   </span>
 </span>
 
diff --git a/src/material/chips/chip-row.spec.ts b/src/material/chips/chip-row.spec.ts
index d90a6bbd3570..c6c44ee47d5e 100644
--- a/src/material/chips/chip-row.spec.ts
+++ b/src/material/chips/chip-row.spec.ts
@@ -256,14 +256,16 @@ describe('MDC-based Row Chips', () => {
         return chipNativeElement.querySelector('.mat-chip-edit-input')!;
       }
 
-      it('should set the role of the primary action based on whether it is editable', () => {
+      it('should set the role of the primary action to gridcell', () => {
         testComponent.editable = false;
         fixture.detectChanges();
-        expect(primaryAction.hasAttribute('role')).toBe(false);
+        expect(primaryAction.getAttribute('role')).toBe('gridcell');
 
         testComponent.editable = true;
         fixture.detectChanges();
-        expect(primaryAction.getAttribute('role')).toBe('button');
+        // Test regression of bug where element is mislabeled as a button role. Element that does not perform its
+        // action on click event is not a button by ARIA spec (#27106).
+        expect(primaryAction.getAttribute('role')).toBe('gridcell');
       });
 
       it('should not delete the chip on DELETE or BACKSPACE', () => {
@@ -346,7 +348,7 @@ describe('MDC-based Row Chips', () => {
         fixture.detectChanges();
 
         const primaryGridCell = (fixture.nativeElement as HTMLElement).querySelector(
-          '[role="gridcell"].mdc-evolution-chip__cell--primary .mat-mdc-chip-action',
+          '[role="gridcell"].mdc-evolution-chip__cell--primary.mat-mdc-chip-action',
         );
         expect(primaryGridCell)
           .withContext('expected to find the grid cell for the primary chip action')
diff --git a/src/material/chips/chip.scss b/src/material/chips/chip.scss
index 1253c1e5c960..edfd708032cb 100644
--- a/src/material/chips/chip.scss
+++ b/src/material/chips/chip.scss
@@ -118,7 +118,7 @@
 
   // Ensures that the trailing icon is pushed to the end if the chip has a set width.
   .mdc-evolution-chip__cell--primary {
-    width: 100%;
+    flex-basis: 100%;
   }
 
   // This element can be placed on a `button` node which usually has some user agent styles.