fix(material/menu): getLabel not working if text is inside indirect descendant node (#20705)

Currently `MatMenuItem.getLabel` only looks at direct descendant text nodes in order
to figure out its text while excluding things like icons. This breaks down if there's
some kind of wrapper in the menu item.

These changes fix the issue by using `textContent` and excluding icons specifically.

Fixes #20200.

Author: crisbeto

src/material-experimental/mdc-menu/menu.spec.ts

@@ -628,10 +628,18 @@ describe('MDC-based MatMenu', () => {
     expect(fixture.componentInstance.items.first.getLabel()).toBe('Item');
   });
 
-  it('should filter out non-text nodes when figuring out the label', () => {
+  it('should filter out icon nodes when figuring out the label', () => {
     const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
     fixture.detectChanges();
-    expect(fixture.componentInstance.items.last.getLabel()).toBe('Item with an icon');
+    const items = fixture.componentInstance.items.toArray();
+    expect(items[2].getLabel()).toBe('Item with an icon');
+  });
+
+  it('should get the label of an item if the text is not in a direct descendant node', () => {
+    const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
+    fixture.detectChanges();
+    const items = fixture.componentInstance.items.toArray();
+    expect(items[3].getLabel()).toBe('Item with text inside span');
   });
 
   it('should set the proper focus origin when opening by mouse', fakeAsync(() => {
@@ -2297,9 +2305,12 @@ describe('MatMenu default overrides', () => {
       <button mat-menu-item> Item </button>
       <button mat-menu-item disabled> Disabled </button>
       <button mat-menu-item disableRipple>
-        <fake-icon>unicorn</fake-icon>
+        <mat-icon>unicorn</mat-icon>
         Item with an icon
       </button>
+      <button mat-menu-item>
+        <span>Item with text inside span</span>
+      </button>
       <button *ngFor="let item of extraItems" mat-menu-item> {{item}} </button>
     </mat-menu>
   `
@@ -2520,7 +2531,7 @@ class SubmenuDeclaredInsideParentMenu {
 
 
 @Component({
-  selector: 'fake-icon',
+  selector: 'mat-icon',
   template: '<ng-content></ng-content>'
 })
 class FakeIcon {}

src/material/menu/menu-item.ts

@@ -63,8 +63,6 @@ export class MatMenuItem extends _MatMenuItemMixinBase
   /** ARIA role for the menu item. */
   @Input() role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox' = 'menuitem';
 
-  private _document: Document;
-
   /** Stream that emits when the menu item is hovered. */
   readonly _hovered: Subject<MatMenuItem> = new Subject<MatMenuItem>();
 
@@ -79,7 +77,11 @@ export class MatMenuItem extends _MatMenuItemMixinBase
 
   constructor(
     private _elementRef: ElementRef<HTMLElement>,
-    @Inject(DOCUMENT) document?: any,
+    /**
+     * @deprecated `_document` parameter is no longer being used and will be removed.
+     * @breaking-change 12.0.0
+     */
+    @Inject(DOCUMENT) _document?: any,
     private _focusMonitor?: FocusMonitor,
     @Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel<MatMenuItem>) {
 
@@ -89,8 +91,6 @@ export class MatMenuItem extends _MatMenuItemMixinBase
     if (_parentMenu && _parentMenu.addItem) {
       _parentMenu.addItem(this);
     }
-
-    this._document = document;
   }
 
   /** Focuses the menu item. */
@@ -163,24 +163,16 @@ export class MatMenuItem extends _MatMenuItemMixinBase
 
   /** Gets the label to be used when determining whether the option should be focused. */
   getLabel(): string {
-    const element: HTMLElement = this._elementRef.nativeElement;
-    const textNodeType = this._document ? this._document.TEXT_NODE : 3;
-    let output = '';
-
-    if (element.childNodes) {
-      const length = element.childNodes.length;
-
-      // Go through all the top-level text nodes and extract their text.
-      // We skip anything that's not a text node to prevent the text from
-      // being thrown off by something like an icon.
-      for (let i = 0; i < length; i++) {
-        if (element.childNodes[i].nodeType === textNodeType) {
-          output += element.childNodes[i].textContent;
-        }
-      }
+    const clone = this._elementRef.nativeElement.cloneNode(true) as HTMLElement;
+    const icons = clone.querySelectorAll('mat-icon, .material-icons');
+
+    // Strip away icons so they don't show up in the text.
+    for (let i = 0; i < icons.length; i++) {
+      const icon = icons[i];
+      icon.parentNode?.removeChild(icon);
     }
 
-    return output.trim();
+    return clone.textContent?.trim() || '';
   }
 
   static ngAcceptInputType_disabled: BooleanInput;

src/material/menu/menu.spec.ts

@@ -626,10 +626,18 @@ describe('MatMenu', () => {
     expect(fixture.componentInstance.items.first.getLabel()).toBe('Item');
   });
 
-  it('should filter out non-text nodes when figuring out the label', () => {
+  it('should filter out icon nodes when figuring out the label', () => {
     const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
     fixture.detectChanges();
-    expect(fixture.componentInstance.items.last.getLabel()).toBe('Item with an icon');
+    const items = fixture.componentInstance.items.toArray();
+    expect(items[2].getLabel()).toBe('Item with an icon');
+  });
+
+  it('should get the label of an item if the text is not in a direct descendant node', () => {
+    const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
+    fixture.detectChanges();
+    const items = fixture.componentInstance.items.toArray();
+    expect(items[3].getLabel()).toBe('Item with text inside span');
   });
 
   it('should set the proper focus origin when opening by mouse', fakeAsync(() => {
@@ -2263,9 +2271,12 @@ describe('MatMenu default overrides', () => {
       <button mat-menu-item> Item </button>
       <button mat-menu-item disabled> Disabled </button>
       <button mat-menu-item disableRipple>
-        <fake-icon>unicorn</fake-icon>
+        <mat-icon>unicorn</mat-icon>
         Item with an icon
       </button>
+      <button mat-menu-item>
+        <span>Item with text inside span</span>
+      </button>
       <button *ngFor="let item of extraItems" mat-menu-item> {{item}} </button>
     </mat-menu>
   `
@@ -2486,7 +2497,7 @@ class SubmenuDeclaredInsideParentMenu {
 
 
 @Component({
-  selector: 'fake-icon',
+  selector: 'mat-icon',
   template: '<ng-content></ng-content>'
 })
 class FakeIcon {}

tools/public_api_guard/material/menu.d.ts

@@ -104,7 +104,8 @@ export declare class MatMenuItem extends _MatMenuItemMixinBase implements Focusa
     _parentMenu?: MatMenuPanel<MatMenuItem> | undefined;
     _triggersSubmenu: boolean;
     role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox';
-    constructor(_elementRef: ElementRef<HTMLElement>, document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel<MatMenuItem> | undefined);
+    constructor(_elementRef: ElementRef<HTMLElement>,
+    _document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel<MatMenuItem> | undefined);
     _checkDisabled(event: Event): void;
     _getHostElement(): HTMLElement;
     _getTabIndex(): string;