Skip to content

Commit 5040480

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 fe14d88 commit 5040480

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 {OverlayModule, OverlayContainer, Overlay} from '../index';
44
import {OverlayOutsideClickDispatcher} from './overlay-outside-click-dispatcher';
@@ -155,6 +155,40 @@ describe('OverlayOutsideClickDispatcher', () => {
155155

156156
overlayRef.dispose();
157157
});
158+
159+
it(
160+
'should not throw an error when when closing out related components via the' +
161+
' outsidePointerEvents emitter on background click',
162+
() => {
163+
const firstOverlayRef = overlay.create();
164+
firstOverlayRef.attach(new ComponentPortal(TestComponent));
165+
const secondOverlayRef = overlay.create();
166+
secondOverlayRef.attach(new ComponentPortal(TestComponent));
167+
const thirdOverlayRef = overlay.create();
168+
thirdOverlayRef.attach(new ComponentPortal(TestComponent));
169+
170+
const spy = jasmine.createSpy('background click handler spy').and.callFake(() => {
171+
// we close out both overlays from a single outside click event
172+
firstOverlayRef.detach();
173+
thirdOverlayRef.detach();
174+
});
175+
firstOverlayRef.outsidePointerEvents().subscribe(spy);
176+
secondOverlayRef.outsidePointerEvents().subscribe(spy);
177+
thirdOverlayRef.outsidePointerEvents().subscribe(spy);
178+
179+
const backgroundElement = document.createElement('div');
180+
document.body.appendChild(backgroundElement);
181+
182+
expect(fakeAsync(() => backgroundElement.click())).not.toThrowError();
183+
184+
expect(spy).toHaveBeenCalled();
185+
186+
backgroundElement.parentNode!.removeChild(backgroundElement);
187+
firstOverlayRef.dispose();
188+
secondOverlayRef.dispose();
189+
thirdOverlayRef.dispose();
190+
}
191+
);
158192
});
159193

160194

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ export class OverlayOutsideClickDispatcher extends BaseOverlayDispatcher {
6969
private _clickListener = (event: MouseEvent) => {
7070
// Get the target through the `composedPath` if possible to account for shadow DOM.
7171
const target = event.composedPath ? event.composedPath()[0] : event.target;
72-
const overlays = this._attachedOverlays;
72+
// We copy the array because the original may be modified asynchronously if the
73+
// outsidePointerEvents listener decides to detach overlays resulting in index errors inside
74+
// the for loop.
75+
const overlays = this._attachedOverlays.slice();
7376

7477
// Dispatch the mouse event to the top overlay which has subscribers to its mouse events.
7578
// We want to target all overlays for which the click could be considered as outside click.

0 commit comments

Comments
 (0)