Skip to content

Commit 76613ce

Browse files
cursoragentmacuzi
andcommitted
fix(browser-utils): Remove DOM instrumentation click listeners with stable capture flag
Fixes #24702 Co-Authored-By: Cursor Agent <cursoragent@cursor.com> Co-authored-by: lamberto <macuzi@users.noreply.github.com>
1 parent d5a1b35 commit 76613ce

2 files changed

Lines changed: 68 additions & 5 deletions

File tree

‎packages/browser-utils/src/instrumentation/dom.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ type InstrumentedElement = Element & {
1919
__sentry_instrumentation_handlers__?: {
2020
[key in 'click' | 'keypress']?: {
2121
handler?: unknown;
22+
capture?: boolean;
2223
/** The number of custom listeners attached to this element */
2324
refCount: number;
2425
};
@@ -82,7 +83,8 @@ export function instrumentDOM(): void {
8283
if (!handlerForType.handler) {
8384
const handler = makeDOMEventHandler(triggerDOMHandler);
8485
handlerForType.handler = handler;
85-
originalAddEventListener.call(this, type, handler, options);
86+
handlerForType.capture = typeof options === 'boolean' ? options : !!options?.capture;
87+
originalAddEventListener.call(this, type, handler, handlerForType.capture);
8688
}
8789

8890
handlerForType.refCount++;
@@ -110,7 +112,7 @@ export function instrumentDOM(): void {
110112
handlerForType.refCount--;
111113
// If there are no longer any custom handlers of the current type on this element, we can remove ours, too.
112114
if (handlerForType.refCount <= 0) {
113-
originalRemoveEventListener.call(this, type, handlerForType.handler, options);
115+
originalRemoveEventListener.call(this, type, handlerForType.handler, handlerForType.capture);
114116
handlerForType.handler = undefined;
115117
delete handlers[type]; // eslint-disable-line @typescript-eslint/no-dynamic-delete
116118
}
Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,73 @@
1-
import { describe, expect, it } from 'vitest';
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { afterEach, describe, expect, it } from 'vitest';
25
import { instrumentDOM } from '../../src/instrumentation/dom';
36
import { WINDOW } from '../../src/types';
47

58
// @ts-expect-error - idk
69
WINDOW.XMLHttpRequest = undefined;
710

8-
describe('instrumentXHR', () => {
9-
it('it does not throw if XMLHttpRequest is a key on window but not defined', () => {
11+
describe('instrumentDOM', () => {
12+
afterEach(() => {
13+
// @ts-expect-error - idk
14+
WINDOW.XMLHttpRequest = undefined;
15+
});
16+
17+
it('does not throw if XMLHttpRequest is a key on window but not defined', () => {
1018
expect(instrumentDOM).not.toThrow();
1119
});
20+
21+
it('does not leak document click listeners when removeEventListener uses mismatched capture options', () => {
22+
instrumentDOM();
23+
24+
const live = {
25+
capture: new Set<EventListenerOrEventListenerObject>(),
26+
bubble: new Set<EventListenerOrEventListenerObject>(),
27+
};
28+
const patchedAdd = EventTarget.prototype.addEventListener;
29+
const patchedRemove = EventTarget.prototype.removeEventListener;
30+
31+
const captureFlag = (options?: boolean | AddEventListenerOptions | EventListenerOptions): boolean =>
32+
typeof options === 'boolean' ? options : !!options?.capture;
33+
34+
EventTarget.prototype.addEventListener = function (
35+
type: string,
36+
listener: EventListenerOrEventListenerObject,
37+
options?: boolean | AddEventListenerOptions,
38+
) {
39+
if (this === document && type === 'click') {
40+
(captureFlag(options) ? live.capture : live.bubble).add(listener);
41+
}
42+
return patchedAdd.call(this, type, listener, options);
43+
};
44+
45+
EventTarget.prototype.removeEventListener = function (
46+
type: string,
47+
listener: EventListenerOrEventListenerObject,
48+
options?: boolean | EventListenerOptions,
49+
) {
50+
if (this === document && type === 'click') {
51+
(captureFlag(options) ? live.capture : live.bubble).delete(listener);
52+
}
53+
return patchedRemove.call(this, type, listener, options);
54+
};
55+
56+
const baseline = live.capture.size + live.bubble.size;
57+
58+
const never = () => {};
59+
const onCapture = () => {};
60+
const onBubble = () => {};
61+
62+
for (let i = 0; i < 20; i++) {
63+
document.addEventListener('click', onCapture, true);
64+
document.addEventListener('click', onBubble);
65+
document.removeEventListener('click', never);
66+
document.removeEventListener('click', never);
67+
document.removeEventListener('click', onCapture, true);
68+
document.removeEventListener('click', onBubble);
69+
}
70+
71+
expect(live.capture.size + live.bubble.size - baseline).toBe(0);
72+
});
1273
});

0 commit comments

Comments
 (0)