fix(material/core): remove tabindex from mat-option (#26917)

Remove the tabindex attribute added to MatOption components. MatOption
does not need tabindex because the parent component manages focus by
setting `aria-activedescendant` attribute. Previously, MatOption set
tabindex but was also a referenced by aria-activedescendant. This was
not the correct ARIA semantics. Align closer to ARIA spec by using only
aria-activedescendant rather than both.

Tabindex="-1" seems to be causing a problem in #26861 where VoiceOver
with Firefox moves DOM focus from the combobox to the option when
opening the listbox popup.

Unblocks #26861.

Author: zarend

src/material/core/option/option.ts

@@ -205,6 +205,9 @@ export class _MatOptionBase<T = any> implements FocusableOption, AfterViewChecke
   }
 
   /** Returns the correct tabindex for the option depending on disabled state. */
+  // This method is only used by `MatLegacyOption`. Keeping it here to avoid breaking the types.
+  // That's because `MatLegacyOption` use `MatOption` type in a few places such as
+  // `MatOptionSelectionChange`. It is safe to delete this when `MatLegacyOption` is deleted.
   _getTabIndex(): string {
     return this.disabled ? '-1' : '0';
   }
@@ -251,7 +254,6 @@ export class _MatOptionBase<T = any> implements FocusableOption, AfterViewChecke
   exportAs: 'matOption',
   host: {
     'role': 'option',
-    '[attr.tabindex]': '_getTabIndex()',
     '[class.mdc-list-item--selected]': 'selected',
     '[class.mat-mdc-option-multiple]': 'multiple',
     '[class.mat-mdc-option-active]': 'active',

src/material/select/select.spec.ts

@@ -824,17 +824,27 @@ describe('MDC-based MatSelect', () => {
               'mat-option',
             ) as NodeListOf<HTMLElement>;
 
-            options[3].focus();
+            select.focus();
+            multiFixture.detectChanges();
+            multiFixture.componentInstance.select._keyManager.setActiveItem(3);
+            multiFixture.detectChanges();
+
             expect(document.activeElement)
-              .withContext('Expected fourth option to be focused.')
-              .toBe(options[3]);
+              .withContext('Expected select to have DOM focus.')
+              .toBe(select);
+            expect(select.getAttribute('aria-activedescendant'))
+              .withContext('Expected fourth option to be activated.')
+              .toBe(options[3].id);
 
             multiFixture.componentInstance.control.setValue(['steak-0', 'sushi-7']);
             multiFixture.detectChanges();
 
             expect(document.activeElement)
-              .withContext('Expected fourth option to remain focused.')
-              .toBe(options[3]);
+              .withContext('Expected select to have DOM focus.')
+              .toBe(select);
+            expect(select.getAttribute('aria-activedescendant'))
+              .withContext('Expected fourth optino to remain activated.')
+              .toBe(options[3].id);
           }),
         );
 
@@ -1260,10 +1270,10 @@ describe('MDC-based MatSelect', () => {
             .toBe(true);
         }));
 
-        it('should set the tabindex of each option according to disabled state', fakeAsync(() => {
-          expect(options[0].getAttribute('tabindex')).toEqual('0');
-          expect(options[1].getAttribute('tabindex')).toEqual('0');
-          expect(options[2].getAttribute('tabindex')).toEqual('-1');
+        it('should omit the tabindex attribute on each option', fakeAsync(() => {
+          expect(options[0].hasAttribute('tabindex')).toBeFalse();
+          expect(options[1].hasAttribute('tabindex')).toBeFalse();
+          expect(options[2].hasAttribute('tabindex')).toBeFalse();
         }));
 
         it('should set aria-disabled for disabled options', fakeAsync(() => {