Skip to content

Commit c0efe17

Browse files
crisbetoandrewseguin
authored andcommitted
fix(overlay): events not being unbound from destroyed backdrop (#16268)
Fixes the `click` and `transitionend` event handlers not being removed from the backdrop that is being destroyed. Since they're referring to things inside the `OverlayRef`, they could cause some memory to be retained.
1 parent a9a266a commit c0efe17

File tree

2 files changed

+33
-4
lines changed

2 files changed

+33
-4
lines changed

src/cdk/overlay/overlay-ref.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
3737
private _positionStrategy: PositionStrategy | undefined;
3838
private _scrollStrategy: ScrollStrategy | undefined;
3939
private _locationChanges: SubscriptionLike = Subscription.EMPTY;
40+
private _backdropClickHandler = (event: MouseEvent) => this._backdropClick.next(event);
4041

4142
/**
4243
* Reference to the parent of the `_host` at the time it was detached. Used to restore
@@ -390,8 +391,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
390391

391392
// Forward backdrop clicks such that the consumer of the overlay can perform whatever
392393
// action desired when such a click occurs (usually closing the overlay).
393-
this._backdropElement.addEventListener('click',
394-
(event: MouseEvent) => this._backdropClick.next(event));
394+
this._backdropElement.addEventListener('click', this._backdropClickHandler);
395395

396396
// Add class to fade-in the backdrop after one frame.
397397
if (typeof requestAnimationFrame !== 'undefined') {
@@ -431,8 +431,13 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
431431
let timeoutId: number;
432432
let finishDetach = () => {
433433
// It may not be attached to anything in certain cases (e.g. unit tests).
434-
if (backdropToDetach && backdropToDetach.parentNode) {
435-
backdropToDetach.parentNode.removeChild(backdropToDetach);
434+
if (backdropToDetach) {
435+
backdropToDetach.removeEventListener('click', this._backdropClickHandler);
436+
backdropToDetach.removeEventListener('transitionend', finishDetach);
437+
438+
if (backdropToDetach.parentNode) {
439+
backdropToDetach.parentNode.removeChild(backdropToDetach);
440+
}
436441
}
437442

438443
// It is possible that a new portal has been attached to this overlay since we started

src/cdk/overlay/overlay.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,30 @@ describe('Overlay', () => {
745745
.toBeLessThan(children.indexOf(host), 'Expected backdrop to be before the host in the DOM');
746746
});
747747

748+
it('should remove the event listener from the backdrop', () => {
749+
let overlayRef = overlay.create(config);
750+
overlayRef.attach(componentPortal);
751+
752+
viewContainerFixture.detectChanges();
753+
let backdrop = overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement;
754+
expect(backdrop).toBeTruthy();
755+
expect(backdrop.classList).not.toContain('cdk-overlay-backdrop-showing');
756+
757+
let backdropClickHandler = jasmine.createSpy('backdropClickHander');
758+
overlayRef.backdropClick().subscribe(backdropClickHandler);
759+
760+
backdrop.click();
761+
expect(backdropClickHandler).toHaveBeenCalledTimes(1);
762+
763+
overlayRef.detach();
764+
dispatchFakeEvent(backdrop, 'transitionend');
765+
zone.simulateZoneExit();
766+
viewContainerFixture.detectChanges();
767+
768+
backdrop.click();
769+
expect(backdropClickHandler).toHaveBeenCalledTimes(1);
770+
});
771+
748772
});
749773

750774
describe('panelClass', () => {

0 commit comments

Comments
 (0)