-
-
Notifications
You must be signed in to change notification settings - Fork 33.4k
lib: implement passive listener behavior per spec #59995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lib: implement passive listener behavior per spec #59995
Conversation
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #59995 +/- ##
==========================================
- Coverage 88.45% 88.44% -0.02%
==========================================
Files 703 703
Lines 207794 207815 +21
Branches 40024 40034 +10
==========================================
- Hits 183805 183792 -13
- Misses 15971 16015 +44
+ Partials 8018 8008 -10
🚀 New features to boost your workflow:
|
ee8c8fd
to
402d8cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Commit Queue failed- Loading data for nodejs/node/pull/59995 ✔ Done loading data for nodejs/node/pull/59995 ----------------------------------- PR info ------------------------------------ Title lib: implement passive listener behavior per spec (#59995) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch Amemome:lib/implement-events-passive-listener-spec -> nodejs:main Labels author ready, needs-ci, commit-queue-squash Commits 8 - lib: implement passive listener preventDefault behavior per spec - test: remove passing AddEventListenerOptions-passive.any.js WPT status - lib: Ensure kInPassiveListener state is cleaned up on error - lib: fix lint error - lib: fix passive listener implementation with cancelable checks - test: enable passive listener tests after implementation fix - test: fix lint error by removing unused common variable assignment - Merge branch 'nodejs:main' into lib/implement-events-passive-listener… Committers 2 - BCD1me <152237136+Amemome@users.noreply.github.com> - GitHub <noreply@github.com> PR-URL: https://github.com/nodejs/node/pull/59995 Refs: https://dom.spec.whatwg.org/#dom-event-preventdefault 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> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/59995 Refs: https://dom.spec.whatwg.org/#dom-event-preventdefault 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> -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 24 Sep 2025 06:44:19 GMT ✔ Approvals: 5 ✔ - Daeyeon Jeong (@daeyeon): https://github.com/nodejs/node/pull/59995#pullrequestreview-3291751784 ✔ - Mattias Buelens (@MattiasBuelens): https://github.com/nodejs/node/pull/59995#pullrequestreview-3293254003 ✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/59995#pullrequestreview-3295749506 ✔ - Jason Zhang (@jazelly): https://github.com/nodejs/node/pull/59995#pullrequestreview-3296937245 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/59995#pullrequestreview-3297832070 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2025-10-02T01:25:08Z: https://ci.nodejs.org/job/node-test-pull-request/69502/ - Querying data for job/node-test-pull-request/69502/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 59995 From https://github.com/nodejs/node * branch refs/pull/59995/merge -> FETCH_HEAD ✔ Fetched commits as 51df7b92bc0a..2c8534ed1e9b -------------------------------------------------------------------------------- [main 6d31a3c5b1] lib: implement passive listener preventDefault behavior per spec Author: BCD1me <152237136+Amemome@users.noreply.github.com> Date: Wed Sep 24 15:29:30 2025 +0900 1 file changed, 25 insertions(+), 1 deletion(-) [main 415016f039] test: remove passing AddEventListenerOptions-passive.any.js WPT status Author: BCD1me <152237136+Amemome@users.noreply.github.com> Date: Wed Sep 24 15:41:15 2025 +0900 1 file changed, 9 deletions(-) [main e5f52560b9] lib: Ensure kInPassiveListener state is cleaned up on error Author: BCD1me <152237136+Amemome@users.noreply.github.com> Date: Sat Sep 27 21:40:25 2025 +0900 1 file changed, 3 insertions(+), 4 deletions(-) [main de9f55d0ec] lib: fix lint error Author: BCD1me <152237136+Amemome@users.noreply.github.com> Date: Sat Sep 27 21:57:18 2025 +0900 1 file changed, 1 insertion(+), 1 deletion(-) [main 3e62f50f35] lib: fix passive listener implementation with cancelable checks Author: BCD1me <152237136+Amemome@users.noreply.github.com> Date: Wed Oct 1 13:29:01 2025 +0900 1 file changed, 5 insertions(+), 7 deletions(-) [main 9d2dc1012e] test: enable passive listener tests after implementation fix Author: BCD1me <152237136+Amemome@users.noreply.github.com> Date: Wed Oct 1 13:34:35 2025 +0900 1 file changed, 1 deletion(-) error: commit 2c8534ed1e9ba7316370102515aa168909bb8c83 is a merge but no -m option was given. fatal: cherry-pick failed [main fa532ce720] test: fix lint error by removing unused common variable assignment Author: BCD1me <152237136+Amemome@users.noreply.github.com> Date: Wed Oct 1 14:07:41 2025 +0900 1 file changed, 1 insertion(+), 1 deletion(-) ✘ Failed to apply patcheshttps://github.com/nodejs/node/actions/runs/18216332994 |
@Amemome unfortunately our automation is failing to land this. Could you squash your commits and rebase on top of main? |
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
2c8534e
to
24e6f21
Compare
Landed in 6f941fc |
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>
Implement Web API spec-compliant behavior
for passive event listeners
where preventDefault() and returnValue setting are ignored during
passive listener execution.
passive listener execution
Fixes WPT test cases that were previously failing in
AddEventListenerOptions-passive.any.js and
satisfies expected behaviors:
if-and-only-if the passive option is true
if-and-only-if the passive option is true
by other listeners
Refs: https://dom.spec.whatwg.org/#dom-event-preventdefault