inteligr8/alfresco-ng2-components
1
0
Fork 0
mirror of https://github.com/Alfresco/alfresco-ng2-components.git synced 2026-09-09 18:03:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): fix ResizableDirective memory leak from orphaned document listeners [AAE-50892] (#12197)

Browse Source 
* fix(core): fix ResizableDirective memory leak from orphaned document listeners [AAE-50892]

Return unlisten teardown from Observable subscribers so RxJS properly
removes document event listeners when share() refcount drops to zero.

Previously, renderer.listen() unlisten functions were stored in mutable
fields that got overwritten on each drag cycle, orphaning old listeners
and retaining entire detached DOM subtrees via the closure chain.

* fix after second report
This commit is contained in:
Maurizio Vitale
2026-08-27 12:47:49 +01:00
committed by GitHub
parent 3f2c81de86
commit 1e40e5effd
2 changed files with 59 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/core/src/lib/datatable/directives/resizable/resizable.directive.spec.ts
+52 -15
View File
@@ -16,7 +16,7 @@
*/
import { TestBed } from '@angular/core/testing';
import { ElementRef, Injector, NgZone, Renderer2, runInInjectionContext } from '@angular/core';
import { ElementRef, EnvironmentInjector, NgZone, Renderer2, createEnvironmentInjector, runInInjectionContext } from '@angular/core';
import { ResizableDirective } from './resizable.directive';
describe('ResizableDirective', () => {
@@ -24,6 +24,7 @@ describe('ResizableDirective', () => {
let renderer: Renderer2;
let element: ElementRef;
let directive: ResizableDirective;
let testEnvInjector: EnvironmentInjector;
const scrollTop = 0;
const scrollLeft = 0;
@@ -39,8 +40,14 @@ describe('ResizableDirective', () => {
scrollLeft
};
let unlistenSpies: jasmine.Spy[];
const rendererMock = {
listen: jasmine.createSpy('listen'),
listen: jasmine.createSpy('listen').and.callFake(() => {
const spy = jasmine.createSpy(`unlisten-${unlistenSpies.length}`);
unlistenSpies.push(spy);
return spy;
}),
setStyle: jasmine.createSpy('setStyle')
};
@@ -53,6 +60,10 @@ describe('ResizableDirective', () => {
};
beforeEach(() => {
unlistenSpies = [];
rendererMock.listen.calls.reset();
rendererMock.setStyle.calls.reset();
TestBed.configureTestingModule({
imports: [ResizableDirective],
providers: [
@@ -64,29 +75,28 @@ describe('ResizableDirective', () => {
element = TestBed.inject(ElementRef);
renderer = TestBed.inject(Renderer2);
ngZone = TestBed.inject(NgZone);
const injector = TestBed.inject(Injector);
spyOn(ngZone, 'runOutsideAngular').and.callFake((fn) => fn());
spyOn(ngZone, 'run').and.callFake((fn) => fn());
const testInjector = Injector.create({
providers: [
testEnvInjector = createEnvironmentInjector(
[
{ provide: Renderer2, useValue: renderer },
{ provide: ElementRef, useValue: element },
{ provide: NgZone, useValue: ngZone }
],
parent: injector
});
TestBed.inject(EnvironmentInjector)
);
directive = runInInjectionContext(testInjector, () => new ResizableDirective());
directive = runInInjectionContext(testEnvInjector, () => new ResizableDirective());
directive.ngOnInit();
});
it('should attach mousedown event to document', () => {
expect(renderer.listen).toHaveBeenCalledWith('document', 'mousedown', jasmine.any(Function));
it('should not attach any document listeners on init', () => {
expect(renderer.listen).not.toHaveBeenCalled();
});
it('should attach mousemove event to document', () => {
it('should attach document mousemove listener only during active drag', () => {
const mouseDownEvent = new MouseEvent('mousedown');
directive.mousedown.next({ ...mouseDownEvent, resize: true });
@@ -94,10 +104,6 @@ describe('ResizableDirective', () => {
expect(renderer.listen).toHaveBeenCalledWith('document', 'mousemove', jasmine.any(Function));
});
it('should attach mouseup event to document', () => {
expect(renderer.listen).toHaveBeenCalledWith('document', 'mouseup', jasmine.any(Function));
});
it('should should set the cursor on mouse down', () => {
spyOn(directive.resizeStart, 'emit');
const mouseDownEvent = new MouseEvent('mousedown');
@@ -174,4 +180,35 @@ describe('ResizableDirective', () => {
expect(directive.keyboardResizing.emit).toHaveBeenCalledWith({ rectangle: { top: 0, left: 0, bottom: 0, right: step, width: step } });
});
it('should unregister document listeners on destroy', () => {
directive.mousedown.next({ ...new MouseEvent('mousedown'), resize: true });
expect(unlistenSpies.length).toBeGreaterThan(0);
testEnvInjector.destroy();
unlistenSpies.forEach((spy) => expect(spy).toHaveBeenCalledTimes(1));
});
it('should not accumulate mousemove listeners across repeated drag cycles', () => {
const listenCountAfterInit = rendererMock.listen.calls.count();
const mouseDownEvent = new MouseEvent('mousedown');
const mouseUpEvent = new MouseEvent('mouseup');
directive.mousedown.next({ ...mouseDownEvent, resize: true });
const listenCountAfterFirstDrag = rendererMock.listen.calls.count();
expect(listenCountAfterFirstDrag).toBeGreaterThan(listenCountAfterInit);
directive.mouseup.next(mouseUpEvent);
const unlistenedAfterFirstDrag = unlistenSpies.filter((spy) => spy.calls.count() > 0).length;
directive.mousedown.next({ ...mouseDownEvent, resize: true });
directive.mouseup.next(mouseUpEvent);
const unlistenedAfterSecondDrag = unlistenSpies.filter((spy) => spy.calls.count() > 0).length;
expect(unlistenedAfterSecondDrag).toBeGreaterThan(unlistenedAfterFirstDrag);
testEnvInjector.destroy();
unlistenSpies.forEach((spy) => expect(spy).toHaveBeenCalledTimes(1));
});
});
lib/core/src/lib/datatable/directives/resizable/resizable.directive.ts
+7 -28
View File
@@ -61,53 +61,35 @@ export class ResizableDirective implements OnInit, OnDestroy {
mousemove = new Subject<IResizeMouseEvent>();
private readonly pointerDown: Observable<IResizeMouseEvent>;
private readonly pointerMove: Observable<IResizeMouseEvent>;
private readonly pointerUp: Observable<IResizeMouseEvent>;
private startingRect: BoundingRectangle;
private currentRect: BoundingRectangle;
private unsubscribeMouseDown?: () => void;
private unsubscribeMouseMove?: () => void;
private unsubscribeMouseUp?: () => void;
private readonly destroyRef = inject(DestroyRef);
constructor() {
const renderer = this.renderer;
const zone = this.zone;
this.pointerDown = new Observable((observer: Observer<IResizeMouseEvent>) => {
zone.runOutsideAngular(() => {
this.unsubscribeMouseDown = renderer.listen('document', 'mousedown', (event: MouseEvent) => {
observer.next(event);
});
});
}).pipe(share());
// Document-level mousemove is needed for smooth drag tracking when cursor leaves the handle element.
// Only subscribed during active drag via share() refcount.
this.pointerMove = new Observable((observer: Observer<IResizeMouseEvent>) => {
let stopListening: () => void = () => {};
zone.runOutsideAngular(() => {
this.unsubscribeMouseMove = renderer.listen('document', 'mousemove', (event: MouseEvent) => {
observer.next(event);
});
});
}).pipe(share());
this.pointerUp = new Observable((observer: Observer<IResizeMouseEvent>) => {
zone.runOutsideAngular(() => {
this.unsubscribeMouseUp = renderer.listen('document', 'mouseup', (event: MouseEvent) => {
stopListening = renderer.listen('document', 'mousemove', (event: MouseEvent) => {
observer.next(event);
});
});
return stopListening;
}).pipe(share());
}
ngOnInit(): void {
const mousedown$ = merge(this.pointerDown, this.mousedown);
const mousedown$ = this.mousedown.asObservable();
const mousemove$ = merge(this.pointerMove, this.mousemove);
const mouseup$ = merge(this.pointerUp, this.mouseup);
const mouseup$ = this.mouseup.asObservable();
const mouseDrag: Observable<IResizeMouseEvent | ICoordinateX> = mousedown$
.pipe(
@@ -184,9 +166,6 @@ export class ResizableDirective implements OnInit, OnDestroy {
this.mousedown.complete();
this.mousemove.complete();
this.mouseup.complete();
this.unsubscribeMouseDown?.();
this.unsubscribeMouseMove?.();
this.unsubscribeMouseUp?.();
}
resizeByKeyboard(delta: number): void {