fix(zone.js): preserve accessor and non-enumerable event listener options

`copyEventListenerOptions` copied the caller's options with `{...options}`
before forwarding to the native `addEventListener`. Object spread only
copies own enumerable data properties, whereas the native call reads
each dictionary member via WebIDL — a plain `[[Get]]` per member, which
invokes accessors and ignores enumerability. The copy was therefore
lossy in a way the native call is not:

- `Object.defineProperty(opts, 'passive', { get })` (the shape used by
  MDN's passive-listener feature test) — the getter was never invoked,
  so libraries that use the feature test fall back to the legacy boolean
  and register every listener as non-passive.
- `Object.defineProperty(opts, 'capture', { get: () => true })` — the
  listener was silently registered on the bubbling phase.
- `Object.defineProperty(opts, 'once', { get: () => true })` — the
  listener fired on every dispatch.

`signal` was already special-cased for `AbortController.prototype.signal`
after #54142; that patch generalises the workaround to every recognised
member.

The copy itself was the correct fix for #54142 (frozen/readonly options)
and is preserved. The fix reads each recognised member from the source
via `[[Get]]` when the spread did not, which recovers accessors and
non-enumerable properties without double-invoking any getter. The list
of recognised members is hoisted to module scope so it isn't allocated
on every `patchEventTarget` invocation.

The call site is reordered to `buildEventListenerOptions(
copyEventListenerOptions(...))` so the passive-events code path also
spreads a normalised data object rather than the caller's raw input.

Fixes #70431

Co-authored-by: Matthieu Riegler <kyro38@gmail.com>
This commit is contained in:
Stefan
2026-08-27 16:14:10 +02:00
committed by Matthew Beck
parent 0904f90b13
commit 4c4a705ce3
2 changed files with 90 additions and 22 deletions
+23 -22
View File
@@ -92,6 +92,11 @@ export const globalSources: any = {};
const EVENT_NAME_SYMBOL_REGX = new RegExp('^' + ZONE_SYMBOL_PREFIX + '(\\w+)(true|false)$');
const IMMEDIATE_PROPAGATION_SYMBOL = zoneSymbol('propagationStopped');
// Recognised members of the `AddEventListenerOptions` WebIDL dictionary, used
// by `copyEventListenerOptions` to recover accessors and non-enumerable
// properties that a caller-supplied options object may expose.
const KNOWN_EVENT_LISTENER_OPTIONS = ['capture', 'once', 'passive', 'signal'];
function prepareEventNames(eventName: string, eventNameToString?: (eventName: string) => string) {
const falseEventName = (eventNameToString ? eventNameToString(eventName) : eventName) + FALSE_STR;
const trueEventName = (eventNameToString ? eventNameToString(eventName) : eventName) + TRUE_STR;
@@ -381,7 +386,8 @@ export function patchEventTarget(
return {passive: true};
}
if (typeof options === 'object' && options.passive !== false) {
return {...options, passive: true};
options.passive = true;
return options;
}
return options;
}
@@ -492,27 +498,22 @@ export function patchEventTarget(
const passiveEvents: string[] = _global[zoneSymbol('PASSIVE_EVENTS')];
function copyEventListenerOptions(options: any) {
if (typeof options === 'object' && options !== null) {
// We need to destructure the target `options` object since it may
// be frozen or sealed (possibly provided implicitly by a third-party
// library), or its properties may be readonly.
const newOptions: any = {...options};
// The `signal` option was recently introduced, which caused regressions in
// third-party scenarios where `AbortController` was directly provided to
// `addEventListener` as options. For instance, in cases like
// `document.addEventListener('keydown', callback, abortControllerInstance)`,
// which is valid because `AbortController` includes a `signal` getter, spreading
// `{...options}` wouldn't copy the `signal`. Additionally, using `Object.create`
// isn't feasible since `AbortController` is a built-in object type, and attempting
// to create a new object directly with it as the prototype might result in
// unexpected behavior.
if (options.signal) {
newOptions.signal = options.signal;
}
return newOptions;
if (typeof options !== 'object' || options === null) {
return options;
}
return options;
// Spread copies own enumerable properties, invoking any getters exactly once.
const newOptions: any = {...options};
// Anything the spread could not see (inherited accessors such as
// `AbortController.prototype.signal`, or non-enumerable properties defined
// via `Object.defineProperty`) is read directly from the source, exactly once. Reading
// from `options` rather than `newOptions` also gives prototype getters the
// correct receiver.
for (const key of KNOWN_EVENT_LISTENER_OPTIONS) {
if (!Object.hasOwn(newOptions, key) && key in options) {
newOptions[key] = options[key];
}
}
return newOptions;
}
const makeAddListener = function (
@@ -556,7 +557,7 @@ export function patchEventTarget(
}
const passive = !!passiveEvents && passiveEvents.indexOf(eventName) !== -1;
const options = copyEventListenerOptions(buildEventListenerOptions(arguments[2], passive));
const options = buildEventListenerOptions(copyEventListenerOptions(arguments[2]), passive);
const signal: AbortSignal | undefined = options?.signal;
if (signal?.aborted) {
// the signal is an aborted one, just return without attaching the event listener.
@@ -2184,6 +2184,73 @@ describe('Zone Browser', function () {
expect(logs).toEqual(['click2']);
});
// Options exposed as accessors or non-enumerable properties must still
// reach the native call. https://github.com/angular/angular/issues/54142
describe('event listener options built with accessors', function () {
it('should invoke a non-enumerable `passive` getter (MDN feature-detect pattern)', function () {
let getCount = 0;
const opts = Object.defineProperty({}, 'passive', {
get: () => {
getCount++;
return false;
},
});
const listener = () => {};
button.addEventListener('click', listener, opts as any);
expect(getCount).toBe(1);
button.removeEventListener('click', listener, opts as any);
});
it('should honour `capture: true` supplied as an accessor', function () {
const opts = Object.defineProperty({}, 'capture', {get: () => true});
const inner = document.createElement('span');
button.appendChild(inner);
let phase = -1;
const listener = (e: Event) => {
phase = e.eventPhase;
};
button.addEventListener('click', listener, opts as any);
inner.dispatchEvent(clickEvent);
expect(phase).toBe(Event.CAPTURING_PHASE);
button.removeEventListener('click', listener, opts as any);
phase = -1;
inner.dispatchEvent(clickEvent);
expect(phase).toBe(-1);
button.removeChild(inner);
});
it('should honour `once: true` supplied as an accessor', function () {
const opts = Object.defineProperty({}, 'once', {get: () => true});
let callCount = 0;
button.addEventListener('click', () => callCount++, opts as any);
button.dispatchEvent(clickEvent);
button.dispatchEvent(clickEvent);
expect(callCount).toBe(1);
});
// `AbortController.prototype.signal` is a prototype accessor, so an
// own-properties-only copy would drop it.
it('should honour `signal` on an AbortController passed as options', function () {
const ac = new AbortController();
const logs: string[] = [];
button.addEventListener('click', () => logs.push('click'), ac);
button.dispatchEvent(clickEvent);
ac.abort();
button.dispatchEvent(clickEvent);
expect(logs).toEqual(['click']);
expect(button.eventListeners!('click').length).toBe(0);
});
// https://github.com/angular/angular/pull/55796
it('should accept a frozen options object', function () {
const opts = Object.freeze({capture: true, once: true});
const listener = () => {};
expect(() => button.addEventListener('click', listener, opts as any)).not.toThrow();
button.removeEventListener('click', listener, opts as any);
});
});
// https://github.com/angular/angular/issues/56148
it('should store the remove abort listener on the task itself and not the task data', function () {
const logs: string[] = [];