From acaed95b0d049705d77103cc127b1c6e2081672f Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 28 Nov 2018 16:53:28 +0100 Subject: [PATCH] fix(tabs): avoid hitting change detection if text content hasn't changed (#14251) Currently we trigger a change detection run when the content of the tab header changes. Since the `MutationObserver` callback can fire if the text node got swapped out, but the actual text didn't change, we could get into an infinite change detection loop if a poorly constructed data binding is used. These changes add a check that will only do extra work if the content has changed. Fixes #14249. --- src/lib/tabs/tab-header.ts | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/lib/tabs/tab-header.ts b/src/lib/tabs/tab-header.ts index 30273380c20..25a9f9742d8 100644 --- a/src/lib/tabs/tab-header.ts +++ b/src/lib/tabs/tab-header.ts @@ -114,7 +114,8 @@ export class MatTabHeader extends _MatTabHeaderMixinBase /** Used to manage focus between the tabs. */ private _keyManager: FocusKeyManager; - private _selectedIndex: number = 0; + /** Cached text content of the header. */ + private _currentTextContent: string; /** The index of the active tab. */ @Input() @@ -128,6 +129,7 @@ export class MatTabHeader extends _MatTabHeaderMixinBase this._keyManager.updateActiveItemIndex(value); } } + private _selectedIndex: number = 0; /** Event emitted when the option is selected. */ @Output() readonly selectFocusedIndex = new EventEmitter(); @@ -237,16 +239,25 @@ export class MatTabHeader extends _MatTabHeaderMixinBase * Callback for when the MutationObserver detects that the content has changed. */ _onContentChanges() { - const zoneCallback = () => { - this.updatePagination(); - this._alignInkBarToSelectedTab(); - this._changeDetectorRef.markForCheck(); - }; + const textContent = this._elementRef.nativeElement.textContent; - // The content observer runs outside the `NgZone` by default, which - // means that we need to bring the callback back in ourselves. - // @breaking-change 8.0.0 Remove null check for `_ngZone` once it's a required parameter. - this._ngZone ? this._ngZone.run(zoneCallback) : zoneCallback(); + // We need to diff the text content of the header, because the MutationObserver callback + // will fire even if the text content didn't change which is inefficient and is prone + // to infinite loops if a poorly constructed expression is passed in (see #14249). + if (textContent !== this._currentTextContent) { + this._currentTextContent = textContent; + + const zoneCallback = () => { + this.updatePagination(); + this._alignInkBarToSelectedTab(); + this._changeDetectorRef.markForCheck(); + }; + + // The content observer runs outside the `NgZone` by default, which + // means that we need to bring the callback back in ourselves. + // @breaking-change 8.0.0 Remove null check for `_ngZone` once it's a required parameter. + this._ngZone ? this._ngZone.run(zoneCallback) : zoneCallback(); + } } /**