Skip to content

Commit

Permalink
[skip ci] Add regression test for EventTarget (facebook#48431)
Browse files Browse the repository at this point in the history
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__: whatwg/dom#1346

Edit: the bug is in the Chrome implementation, not in the spec.

Differential Revision: D67758702
  • Loading branch information
rubennorte authored and facebook-github-bot committed Jan 6, 2025
1 parent 348128e commit 58a0e7e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -344,6 +342,21 @@ export default class EventTarget {
}
}

#removeEventListenerRegistration(
registration: EventListenerRegistration,
listenerList: Array<EventListenerRegistration>,
): 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
});

0 comments on commit 58a0e7e

Please sign in to comment.