Skip to content

Commit 16e8376

Browse files
crisbetowagnermaciel
authored andcommitted
fix(menu): throw error if menu contains its own trigger (#19943)
If the consumer puts a trigger inside a menu and passes the menu back to the trigger, we'll go into an infinite loop and throw a cryptic error message. These changes detect such a case and throw a better error. Example of a case that will be flagged: ``` <mat-menu #menu="matMenu"> <button [matMenuTriggerFor]="menu"></button> </mat-menu> ``` Fixes #19941. (cherry picked from commit 1b7ba0b)
1 parent ae75fab commit 16e8376

File tree

5 files changed

+57
-4
lines changed

5 files changed

+57
-4
lines changed

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,13 @@ describe('MDC-based MatMenu', () => {
712712
}).toThrowError(/must pass in an mat-menu instance/);
713713
});
714714

715+
it('should throw if assigning a menu that contains the trigger', () => {
716+
expect(() => {
717+
const fixture = createComponent(InvalidRecursiveMenu, [], [FakeIcon]);
718+
fixture.detectChanges();
719+
}).toThrowError(/menu cannot contain its own trigger/);
720+
});
721+
715722
it('should be able to swap out a menu after the first time it is opened', fakeAsync(() => {
716723
const fixture = createComponent(DynamicPanelMenu);
717724
fixture.detectChanges();
@@ -2579,3 +2586,14 @@ class SimpleMenuWithRepeaterInLazyContent {
25792586
@ViewChild(MatMenu, {static: false}) menu: MatMenu;
25802587
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
25812588
}
2589+
2590+
2591+
@Component({
2592+
template: `
2593+
<mat-menu #menu="matMenu">
2594+
<button [matMenuTriggerFor]="menu"></button>
2595+
</mat-menu>
2596+
`
2597+
})
2598+
class InvalidRecursiveMenu {
2599+
}

src/material/menu/menu-errors.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,14 @@ export function throwMatMenuInvalidPositionY() {
3737
throw Error(`yPosition value must be either 'above' or below'.
3838
Example: <mat-menu yPosition="above" #menu="matMenu"></mat-menu>`);
3939
}
40+
41+
42+
/**
43+
* Throws an exception for the case when a menu is assigned
44+
* to a trigger that is placed inside the same menu.
45+
* @docs-private
46+
*/
47+
export function throwMatMenuRecursiveError() {
48+
throw Error(`matMenuTriggerFor: menu cannot contain its own trigger. Assign a menu that is ` +
49+
`not a parent of the trigger or move the trigger outside of the menu.`);
50+
}

src/material/menu/menu-trigger.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,13 @@ import {
3232
Output,
3333
Self,
3434
ViewContainerRef,
35+
isDevMode,
3536
} from '@angular/core';
3637
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
3738
import {asapScheduler, merge, of as observableOf, Subscription} from 'rxjs';
3839
import {delay, filter, take, takeUntil} from 'rxjs/operators';
3940
import {MatMenu} from './menu';
40-
import {throwMatMenuMissingError} from './menu-errors';
41+
import {throwMatMenuMissingError, throwMatMenuRecursiveError} from './menu-errors';
4142
import {MatMenuItem} from './menu-item';
4243
import {MatMenuPanel} from './menu-panel';
4344
import {MenuPositionX, MenuPositionY} from './menu-positions';
@@ -121,6 +122,10 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
121122
this._menuCloseSubscription.unsubscribe();
122123

123124
if (menu) {
125+
if (isDevMode() && menu === this._parentMenu) {
126+
throwMatMenuRecursiveError();
127+
}
128+
124129
this._menuCloseSubscription = menu.close.asObservable().subscribe(reason => {
125130
this._destroyMenu();
126131

@@ -370,7 +375,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
370375
* matMenuTriggerFor. If not, an exception is thrown.
371376
*/
372377
private _checkMenu() {
373-
if (!this.menu) {
378+
if (isDevMode() && !this.menu) {
374379
throwMatMenuMissingError();
375380
}
376381
}

src/material/menu/menu.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,13 @@ describe('MatMenu', () => {
747747
}).toThrowError(/must pass in an mat-menu instance/);
748748
});
749749

750+
it('should throw if assigning a menu that contains the trigger', () => {
751+
expect(() => {
752+
const fixture = createComponent(InvalidRecursiveMenu, [], [FakeIcon]);
753+
fixture.detectChanges();
754+
}).toThrowError(/menu cannot contain its own trigger/);
755+
});
756+
750757
it('should be able to swap out a menu after the first time it is opened', fakeAsync(() => {
751758
const fixture = createComponent(DynamicPanelMenu);
752759
fixture.detectChanges();
@@ -2622,3 +2629,14 @@ class LazyMenuWithOnPush {
26222629
@ViewChild('triggerEl', {read: ElementRef}) rootTrigger: ElementRef;
26232630
@ViewChild('menuItem', {read: ElementRef}) menuItemWithSubmenu: ElementRef;
26242631
}
2632+
2633+
2634+
@Component({
2635+
template: `
2636+
<mat-menu #menu="matMenu">
2637+
<button [matMenuTriggerFor]="menu"></button>
2638+
</mat-menu>
2639+
`
2640+
})
2641+
class InvalidRecursiveMenu {
2642+
}

src/material/menu/menu.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
ViewChild,
4040
ViewEncapsulation,
4141
OnInit,
42+
isDevMode,
4243
} from '@angular/core';
4344
import {merge, Observable, Subject, Subscription} from 'rxjs';
4445
import {startWith, switchMap, take} from 'rxjs/operators';
@@ -150,7 +151,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
150151
@Input()
151152
get xPosition(): MenuPositionX { return this._xPosition; }
152153
set xPosition(value: MenuPositionX) {
153-
if (value !== 'before' && value !== 'after') {
154+
if (isDevMode() && value !== 'before' && value !== 'after') {
154155
throwMatMenuInvalidPositionX();
155156
}
156157
this._xPosition = value;
@@ -161,7 +162,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
161162
@Input()
162163
get yPosition(): MenuPositionY { return this._yPosition; }
163164
set yPosition(value: MenuPositionY) {
164-
if (value !== 'above' && value !== 'below') {
165+
if (isDevMode() && value !== 'above' && value !== 'below') {
165166
throwMatMenuInvalidPositionY();
166167
}
167168
this._yPosition = value;

0 commit comments

Comments
 (0)