Skip to content

Commit 80e6254

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 80e6254

File tree

2 files changed

+39
-2
lines changed

2 files changed

+39
-2
lines changed

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

Lines changed: 35 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';
@@ -186,6 +186,40 @@ describe('OverlayOutsideClickDispatcher', () => {
186186

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

191225

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ 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.

0 commit comments

Comments
 (0)