From 58a0e7e036a2f914343d146ac78909f9cb3b163e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 6 Jan 2025 07:20:07 -0800 Subject: [PATCH] [skip ci] Add regression test for EventTarget (#48431) Summary: Changelog: [internal] Adds a regression test to make sure we implement the correct spec-compliant behavior for a possible bug in ~~the Web spec~~ __Chrome__: https://github.com/whatwg/dom/issues/1346 Edit: the bug is in the Chrome implementation, not in the spec. Differential Revision: D67758702 --- .../private/webapis/dom/events/EventTarget.js | 25 +++++++--- .../dom/events/__tests__/EventTarget-itest.js | 48 +++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/packages/react-native/src/private/webapis/dom/events/EventTarget.js b/packages/react-native/src/private/webapis/dom/events/EventTarget.js index 8d4e8bc9bd3e67..1e394ab99b5f7f 100644 --- a/packages/react-native/src/private/webapis/dom/events/EventTarget.js +++ b/packages/react-native/src/private/webapis/dom/events/EventTarget.js @@ -127,16 +127,17 @@ export default class EventTarget { } } - listenerList.push({ + const listener: EventListenerRegistration = { callback, passive, once, removed: false, - }); + }; + listenerList.push(listener); if (signal != null) { signal.addEventListener('abort', () => { - this.removeEventListener(processedType, callback, capture); + this.#removeEventListenerRegistration(listener, listenerList); }); } } @@ -180,9 +181,6 @@ export default class EventTarget { return; } } - - // NOTE: there is no step to remove the event listener from the signal, if - // set, as per the spec. See https://github.com/whatwg/dom/issues/1346. } dispatchEvent(event: Event): boolean { @@ -344,6 +342,21 @@ export default class EventTarget { } } + #removeEventListenerRegistration( + registration: EventListenerRegistration, + listenerList: Array, + ): void { + for (let i = 0; i < listenerList.length; i++) { + const listener = listenerList[i]; + + if (listener === registration) { + listener.removed = true; + listenerList.splice(i, 1); + return; + } + } + } + /** * This a "protected" method to be overridden by a subclass to allow event * propagation. diff --git a/packages/react-native/src/private/webapis/dom/events/__tests__/EventTarget-itest.js b/packages/react-native/src/private/webapis/dom/events/__tests__/EventTarget-itest.js index 6d82f51e6890d3..43dbc6d668b2d4 100644 --- a/packages/react-native/src/private/webapis/dom/events/__tests__/EventTarget-itest.js +++ b/packages/react-native/src/private/webapis/dom/events/__tests__/EventTarget-itest.js @@ -926,5 +926,53 @@ describe('EventTarget', () => { expect(listenerThatWillBeRemoved).not.toHaveBeenCalled(); }); }); + + describe('re-attaching a previous listener with a pending signal', () => { + // This is a regression test for https://github.com/whatwg/dom/issues/1346 + it('should NOT remove the new subscription when the signal for the old subscription is aborted', () => { + const [node] = createEventTargetHierarchyWithDepth(1); + + // Listener setup + + resetListenerCallOrder(); + + const listener = createListener(); + + const abortController = new AbortController(); + + node.addEventListener('custom', listener, { + signal: abortController.signal, + }); + + // Dispatch + + const event = new Event('custom'); + + node.dispatchEvent(event); + + expect(listener).toHaveBeenCalledTimes(1); + + node.removeEventListener('custom', listener); + + node.dispatchEvent(event); + + expect(listener).toHaveBeenCalledTimes(1); + + // Added without a signal + node.addEventListener('custom', listener); + + node.dispatchEvent(event); + + // Listener is called + expect(listener).toHaveBeenCalledTimes(2); + + abortController.abort(); + + node.dispatchEvent(event); + + // Listener is called + expect(listener).toHaveBeenCalledTimes(3); + }); + }); }); });