From b83fe9e2f66e6d7d3e19ca5007a46d3569b47f8b Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 1 Dec 2017 19:50:54 +0100 Subject: [PATCH] fix(global-position-strategy): error if disposed before applied Fixes an error that is thrown by the `GlobalPositionStrategy` if its connected `OverlayRef` is disposed before the strategy is applied. This could happen, because the `OverlayRef` applies the position strategy asynchronously. Fixes #8758. --- .../position/global-position-strategy.spec.ts | 20 +++++++++++++++++-- .../position/global-position-strategy.ts | 7 +++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/cdk/overlay/position/global-position-strategy.spec.ts b/src/cdk/overlay/position/global-position-strategy.spec.ts index 4c804683993c..16f3186e2e46 100644 --- a/src/cdk/overlay/position/global-position-strategy.spec.ts +++ b/src/cdk/overlay/position/global-position-strategy.spec.ts @@ -5,6 +5,7 @@ import {OverlayModule, Overlay, OverlayRef, GlobalPositionStrategy} from '../ind describe('GlobalPositonStrategy', () => { let element: HTMLElement; let strategy: GlobalPositionStrategy; + let hasOverlayAttached: boolean; beforeEach(() => { TestBed.configureTestingModule({imports: [OverlayModule]}); @@ -15,11 +16,18 @@ describe('GlobalPositonStrategy', () => { element = document.createElement('div'); document.body.appendChild(element); - strategy.attach({overlayElement: element} as OverlayRef); + hasOverlayAttached = true; + strategy.attach({ + overlayElement: element, + hasAttached: () => hasOverlayAttached + } as OverlayRef); }); afterEach(() => { - element.parentNode!.removeChild(element); + if (element.parentNode) { + element.parentNode.removeChild(element); + } + strategy.dispose(); }); @@ -151,4 +159,12 @@ describe('GlobalPositonStrategy', () => { expect(element.style.marginTop).toBe('0px'); expect((element.parentNode as HTMLElement).style.alignItems).toBe('flex-start'); }); + + it('should not throw when attempting to apply after the overlay has been disposed', () => { + strategy.dispose(); + element.parentNode!.removeChild(element); + hasOverlayAttached = false; + + expect(() => strategy.apply()).not.toThrow(); + }); }); diff --git a/src/cdk/overlay/position/global-position-strategy.ts b/src/cdk/overlay/position/global-position-strategy.ts index d5b3fce50105..0c62b7e0a0fe 100644 --- a/src/cdk/overlay/position/global-position-strategy.ts +++ b/src/cdk/overlay/position/global-position-strategy.ts @@ -146,6 +146,13 @@ export class GlobalPositionStrategy implements PositionStrategy { * @returns Resolved when the styles have been applied. */ apply(): void { + // Since the overlay ref applies the strategy asynchronously, it could + // have been disposed before it ends up being applied. If that is the + // case, we shouldn't do anything. + if (!this._overlayRef.hasAttached()) { + return; + } + const element = this._overlayRef.overlayElement; if (!this._wrapper && element.parentNode) {