From 1a498a6a811dfd081becbc330c984b23a23de5e6 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Thu, 24 Feb 2022 17:06:53 +0100
Subject: [PATCH] fix(material/sort): remove role from header when disabled
 (#24477)

When the sort header is disabled, we make it non-interactive by clearing its `tabindex`, however it still has the `role="button"` which can throw off screen readers if it doesn't have text.

These changes also clear the `role` from the header if it's disabled.

(cherry picked from commit 229dd6e1748991b014d4e41005be2a74dda7167e)
---
 src/material/sort/sort-header.html |  2 +-
 src/material/sort/sort-header.ts   |  2 +-
 src/material/sort/sort.spec.ts     | 10 +++++++---
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/material/sort/sort-header.html b/src/material/sort/sort-header.html
index cc575585a601..db76622dd754 100644
--- a/src/material/sort/sort-header.html
+++ b/src/material/sort/sort-header.html
@@ -12,7 +12,7 @@
      [class.mat-sort-header-sorted]="_isSorted()"
      [class.mat-sort-header-position-before]="arrowPosition == 'before'"
      [attr.tabindex]="_isDisabled() ? null : 0"
-     role="button">
+     [attr.role]="_isDisabled() ? null : 'button'">
 
   <!--
     TODO(crisbeto): this div isn't strictly necessary, but we have to keep it due to a large
diff --git a/src/material/sort/sort-header.ts b/src/material/sort/sort-header.ts
index 3ece3422dde2..2b7465d11f2a 100644
--- a/src/material/sort/sort-header.ts
+++ b/src/material/sort/sort-header.ts
@@ -209,7 +209,7 @@ export class MatSortHeader
 
     this._sort.register(this);
 
-    this._sortButton = this._elementRef.nativeElement.querySelector('[role="button"]')!;
+    this._sortButton = this._elementRef.nativeElement.querySelector('.mat-sort-header-container')!;
     this._updateSortActionDescription(this._sortActionDescription);
   }
 
diff --git a/src/material/sort/sort.spec.ts b/src/material/sort/sort.spec.ts
index ec3688e0b10e..dccadfb60130 100644
--- a/src/material/sort/sort.spec.ts
+++ b/src/material/sort/sort.spec.ts
@@ -237,13 +237,15 @@ describe('MatSort', () => {
       component.sort('defaultA');
       expect(component.matSort.direction).toBe('asc');
       expect(container.getAttribute('tabindex')).toBe('0');
+      expect(container.getAttribute('role')).toBe('button');
 
       component.disabledColumnSort = true;
       fixture.detectChanges();
 
       component.sort('defaultA');
       expect(component.matSort.direction).toBe('asc');
-      expect(container.getAttribute('tabindex')).toBeFalsy();
+      expect(container.hasAttribute('tabindex')).toBe(false);
+      expect(container.hasAttribute('role')).toBe(false);
     });
 
     it('should allow for the cycling the sort direction to be disabled for all columns', () => {
@@ -417,7 +419,7 @@ describe('MatSort', () => {
     });
 
     it('should add a default aria description to sort buttons', () => {
-      const sortButton = fixture.nativeElement.querySelector('[role="button"]');
+      const sortButton = fixture.nativeElement.querySelector('.mat-sort-header-container');
       const descriptionId = sortButton.getAttribute('aria-describedby');
       expect(descriptionId).toBeDefined();
 
@@ -426,7 +428,9 @@ describe('MatSort', () => {
     });
 
     it('should add a custom aria description to sort buttons', () => {
-      const sortButton = fixture.nativeElement.querySelector('#defaultB [role="button"]');
+      const sortButton = fixture.nativeElement.querySelector(
+        '#defaultB .mat-sort-header-container',
+      );
       let descriptionId = sortButton.getAttribute('aria-describedby');
       expect(descriptionId).toBeDefined();