Commit 6be31fb
lib: implement passive listener behavior per spec
Implements the WHATWG DOM specification for passive event listeners,
ensuring that calls to `preventDefault()` are correctly ignored within
a passive listener context.
An internal `kInPassiveListener` state is added to the Event object
to track when a passive listener is executing. The `preventDefault()`
method and the `returnValue` setter are modified to check this state,
as well as the event's `cancelable` property. This state is reliably
cleaned up within a `finally` block to prevent state pollution in
case a listener throws an error.
This resolves previously failing Web Platform Tests (WPT) in
`AddEventListenerOptions-passive.any.js`.
Refs: https://dom.spec.whatwg.org/#dom-event-preventdefault
PR-URL: #59995
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Mattias Buelens <mattias@buelens.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jason Zhang <xzha4350@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>1 parent b3295b8 commit 6be31fb
3 files changed
Lines changed: 24 additions & 13 deletions
File tree
- lib/internal
- test
- parallel
- wpt/status/dom
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -76,6 +76,7 @@ const { now } = require('internal/perf/utils'); | |||
| 76 | 76 | ||
| 77 | 77 | const kType = Symbol('type'); | |
| 78 | 78 | const kDetail = Symbol('detail'); | |
| 79 | + const kInPassiveListener = Symbol('kInPassiveListener'); | ||
| 79 | 80 | ||
| 80 | 81 | const isTrustedSet = new SafeWeakSet(); | |
| 81 | 82 | const isTrusted = ObjectGetOwnPropertyDescriptor({ | |
@@ -127,6 +128,7 @@ class Event { | |||
| 127 | 128 | ||
| 128 | 129 | this[kTarget] = null; | |
| 129 | 130 | this[kIsBeingDispatched] = false; | |
| 131 | + this[kInPassiveListener] = false; | ||
| 130 | 132 | } | |
| 131 | 133 | ||
| 132 | 134 | /** | |
@@ -178,6 +180,7 @@ class Event { | |||
| 178 | 180 | preventDefault() { | |
| 179 | 181 | if (!isEvent(this)) | |
| 180 | 182 | throw new ERR_INVALID_THIS('Event'); | |
| 183 | + if (!this.#cancelable || this[kInPassiveListener]) return; | ||
| 181 | 184 | this.#defaultPrevented = true; | |
| 182 | 185 | } | |
| 183 | 186 | ||
@@ -266,6 +269,19 @@ class Event { | |||
| 266 | 269 | return !this.#cancelable || !this.#defaultPrevented; | |
| 267 | 270 | } | |
| 268 | 271 | ||
| 272 | + /** | ||
| 273 | + * @type {boolean} | ||
| 274 | + */ | ||
| 275 | + set returnValue(value) { | ||
| 276 | + if (!isEvent(this)) | ||
| 277 | + throw new ERR_INVALID_THIS('Event'); | ||
| 278 | + | ||
| 279 | + if (!value) { | ||
| 280 | + if (!this.#cancelable || this[kInPassiveListener]) return; | ||
| 281 | + this.#defaultPrevented = true; | ||
| 282 | + } | ||
| 283 | + } | ||
| 284 | + | ||
| 269 | 285 | /** | |
| 270 | 286 | * @type {boolean} | |
| 271 | 287 | */ | |
@@ -760,7 +776,6 @@ class EventTarget { | |||
| 760 | 776 | throw new ERR_EVENT_RECURSION(event.type); | |
| 761 | 777 | ||
| 762 | 778 | this[kHybridDispatch](event, event.type, event); | |
| 763 | - | ||
| 764 | 779 | return event.defaultPrevented !== true; | |
| 765 | 780 | } | |
| 766 | 781 | ||
@@ -813,8 +828,8 @@ class EventTarget { | |||
| 813 | 828 | this[kRemoveListener](root.size, type, listener, capture); | |
| 814 | 829 | } | |
| 815 | 830 | ||
| 831 | + let arg; | ||
| 816 | 832 | try { | |
| 817 | - let arg; | ||
| 818 | 833 | if (handler.isNodeStyleListener) { | |
| 819 | 834 | arg = nodeValue; | |
| 820 | 835 | } else { | |
@@ -824,6 +839,9 @@ class EventTarget { | |||
| 824 | 839 | handler.callback.deref() : handler.callback; | |
| 825 | 840 | let result; | |
| 826 | 841 | if (callback) { | |
| 842 | + if (handler.passive && !handler.isNodeStyleListener) { | ||
| 843 | + arg[kInPassiveListener] = true; | ||
| 844 | + } | ||
| 827 | 845 | result = FunctionPrototypeCall(callback, this, arg); | |
| 828 | 846 | if (!handler.isNodeStyleListener) { | |
| 829 | 847 | arg[kIsBeingDispatched] = false; | |
@@ -833,6 +851,9 @@ class EventTarget { | |||
| 833 | 851 | addCatch(result); | |
| 834 | 852 | } catch (err) { | |
| 835 | 853 | emitUncaughtException(err); | |
| 854 | + } finally { | ||
| 855 | + if (arg?.[kInPassiveListener]) | ||
| 856 | + arg[kInPassiveListener] = false; | ||
| 836 | 857 | } | |
| 837 | 858 | ||
| 838 | 859 | handler = next; | |
Lines changed: 1 addition & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,6 +1,6 @@ | |||
| 1 | 1 | 'use strict'; | |
| 2 | 2 | ||
| 3 | - const common = require('../common'); | ||
| 3 | + require('../common'); | ||
| 4 | 4 | ||
| 5 | 5 | // Manually converted from https://github.com/web-platform-tests/wpt/blob/master/dom/events/AddEventListenerOptions-passive.html | |
| 6 | 6 | // in order to define the `document` ourselves | |
@@ -58,7 +58,6 @@ const { | |||
| 58 | 58 | testPassiveValue({}, true); | |
| 59 | 59 | testPassiveValue({ passive: false }, true); | |
| 60 | 60 | ||
| 61 | - common.skip('TODO: passive listeners is still broken'); | ||
| 62 | 61 | testPassiveValue({ passive: 1 }, false); | |
| 63 | 62 | testPassiveValue({ passive: true }, false); | |
| 64 | 63 | testPassiveValue({ passive: 0 }, true); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,13 +1,4 @@ | |||
| 1 | 1 | { | |
| 2 | - "AddEventListenerOptions-passive.any.js": { | ||
| 3 | - "fail": { | ||
| 4 | - "expected": [ | ||
| 5 | - "preventDefault should be ignored if-and-only-if the passive option is true", | ||
| 6 | - "returnValue should be ignored if-and-only-if the passive option is true", | ||
| 7 | - "passive behavior of one listener should be unaffected by the presence of other listeners" | ||
| 8 | - ] | ||
| 9 | - } | ||
| 10 | - }, | ||
| 11 | 2 | "Event-dispatch-listener-order.window.js": { | |
| 12 | 3 | "skip": "document is not defined" | |
| 13 | 4 | }, | |
0 commit comments