Skip to content

Commit 6bf0c04

Browse files
committed
fix(cdk/overlay): ensure error isn't thrown when many overlay items are closed from handler
When an event handler listening to the `outsidePointerEvents` observable closes out any open overlays this removes them from the OverlayOutsideClickDispatcher's array of overlay elements. This results in an error being thrown since the overlays array in the for-loop is modified (elements are removed) and the counter not accounting for this. This fix ensures that if the array of overlay refs is modified we don't get an off by 1 error when fetching an element from the array on subsequent iterations of the loop.
1 parent 7a2a732 commit 6bf0c04

File tree

2 files changed

+44
-3
lines changed

2 files changed

+44
-3
lines changed

src/cdk/overlay/dispatchers/overlay-outside-click-dispatcher.spec.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {TestBed, inject} from '@angular/core/testing';
1+
import {TestBed, inject, fakeAsync} from '@angular/core/testing';
22
import {Component, NgModule} from '@angular/core';
33
import {dispatchMouseEvent} from '@angular/cdk/testing/private';
44
import {OverlayModule, OverlayContainer, Overlay} from '../index';
@@ -55,7 +55,10 @@ describe('OverlayOutsideClickDispatcher', () => {
5555
'should dispatch mouse click events to the attached overlays',
5656
() => {
5757
const overlayOne = overlay.create();
58+
overlayOne.attach(new ComponentPortal(TestComponent));
5859
const overlayTwo = overlay.create();
60+
overlayTwo.attach(new ComponentPortal(TestComponent));
61+
5962
const overlayOneSpy = jasmine.createSpy('overlayOne mouse click event spy');
6063
const overlayTwoSpy = jasmine.createSpy('overlayTwo mouse click event spy');
6164

@@ -81,6 +84,7 @@ describe('OverlayOutsideClickDispatcher', () => {
8184
'should dispatch mouse click events to the attached overlays even when propagation is stopped',
8285
() => {
8386
const overlayRef = overlay.create();
87+
overlayRef.attach(new ComponentPortal(TestComponent));
8488
const spy = jasmine.createSpy('overlay mouse click event spy');
8589
overlayRef.outsidePointerEvents().subscribe(spy);
8690

@@ -186,6 +190,40 @@ describe('OverlayOutsideClickDispatcher', () => {
186190

187191
overlayRef.dispose();
188192
});
193+
194+
it(
195+
'should not throw an error when when closing out related components via the' +
196+
' outsidePointerEvents emitter on background click',
197+
fakeAsync(() => {
198+
const firstOverlayRef = overlay.create();
199+
firstOverlayRef.attach(new ComponentPortal(TestComponent));
200+
const secondOverlayRef = overlay.create();
201+
secondOverlayRef.attach(new ComponentPortal(TestComponent));
202+
const thirdOverlayRef = overlay.create();
203+
thirdOverlayRef.attach(new ComponentPortal(TestComponent));
204+
205+
const spy = jasmine.createSpy('background click handler spy').and.callFake(() => {
206+
// we close out both overlays from a single outside click event
207+
firstOverlayRef.detach();
208+
thirdOverlayRef.detach();
209+
});
210+
firstOverlayRef.outsidePointerEvents().subscribe(spy);
211+
secondOverlayRef.outsidePointerEvents().subscribe(spy);
212+
thirdOverlayRef.outsidePointerEvents().subscribe(spy);
213+
214+
const backgroundElement = document.createElement('div');
215+
document.body.appendChild(backgroundElement);
216+
217+
expect(() => backgroundElement.click()).not.toThrowError();
218+
219+
expect(spy).toHaveBeenCalled();
220+
221+
backgroundElement.parentNode!.removeChild(backgroundElement);
222+
firstOverlayRef.dispose();
223+
secondOverlayRef.dispose();
224+
thirdOverlayRef.dispose();
225+
}
226+
));
189227
});
190228

191229

src/cdk/overlay/dispatchers/overlay-outside-click-dispatcher.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,18 @@ export class OverlayOutsideClickDispatcher extends BaseOverlayDispatcher {
7171
private _clickListener = (event: MouseEvent) => {
7272
// Get the target through the `composedPath` if possible to account for shadow DOM.
7373
const target = event.composedPath ? event.composedPath()[0] : event.target;
74-
const overlays = this._attachedOverlays;
74+
// We copy the array because the original may be modified asynchronously if the
75+
// outsidePointerEvents listener decides to detach overlays resulting in index errors inside
76+
// the for loop.
77+
const overlays = this._attachedOverlays.slice();
7578

7679
// Dispatch the mouse event to the top overlay which has subscribers to its mouse events.
7780
// We want to target all overlays for which the click could be considered as outside click.
7881
// As soon as we reach an overlay for which the click is not outside click we break off
7982
// the loop.
8083
for (let i = overlays.length - 1; i > -1; i--) {
8184
const overlayRef = overlays[i];
82-
if (overlayRef._outsidePointerEvents.observers.length < 1) {
85+
if (overlayRef._outsidePointerEvents.observers.length < 1 || !overlayRef.hasAttached()) {
8386
continue;
8487
}
8588

0 commit comments

Comments
 (0)