From 21cb08f95d35d9ef72e1df0fd0b3f618b90aa3ea Mon Sep 17 00:00:00 2001 From: shisama Date: Fri, 20 May 2022 02:09:59 +0900 Subject: [PATCH 1/7] events: add null check for the signal of EventTarget This will improve the Web compatibility. Passing null as the signal should throw an error. WPT says "Passing null as the signal should throw". Please see https://github.com/web-platform-tests/wpt/blob/master/dom/events/AddEventListenerOptions-signal.any.js --- lib/internal/event_target.js | 4 ++++ .../test-whatwg-events-add-event-listener-options-signal.js | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index c60e80d5c5d4..76f8afa4442a 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -575,6 +575,10 @@ class EventTarget { } type = String(type); + if (signal === null) { + throw new ERR_INVALID_ARG_TYPE('options.signal', 'AbortSignal', signal); + } + if (signal) { if (signal.aborted) { return; diff --git a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js index dfe5810180d9..125d62f52d30 100644 --- a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js +++ b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js @@ -4,6 +4,7 @@ require('../common'); const { strictEqual, + throws } = require('assert'); // Manually ported from: wpt@dom/events/AddEventListenerOptions-signal.any.js @@ -157,3 +158,7 @@ const { }, { once: true }); et.dispatchEvent(new Event('foo')); } +{ + const et = new EventTarget(); + throws(() => et.addEventListener('foo', () => {}, {signal: null})); +} From 5c5f40a5763687747a06b73d7826f7dfb348ab9f Mon Sep 17 00:00:00 2001 From: shisama Date: Fri, 20 May 2022 23:42:26 +0900 Subject: [PATCH 2/7] fix lint errors --- .../test-whatwg-events-add-event-listener-options-signal.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js index 125d62f52d30..6a86b4db9bf1 100644 --- a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js +++ b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js @@ -160,5 +160,7 @@ const { } { const et = new EventTarget(); - throws(() => et.addEventListener('foo', () => {}, {signal: null})); + throws(() => et.addEventListener('foo', () => {}, { signal: null }), { + message: 'The "options.signal" property must be an instance of AbortSignal. Received null', + }); } From 4d76ce338c5335152910db31a7d63f3bff7e4af1 Mon Sep 17 00:00:00 2001 From: Masashi Hirano Date: Fri, 20 May 2022 23:43:39 +0900 Subject: [PATCH 3/7] fix trailing comma Co-authored-by: Antoine du Hamel --- .../test-whatwg-events-add-event-listener-options-signal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js index 6a86b4db9bf1..d4d9ee1c5c11 100644 --- a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js +++ b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js @@ -4,7 +4,7 @@ require('../common'); const { strictEqual, - throws + throws, } = require('assert'); // Manually ported from: wpt@dom/events/AddEventListenerOptions-signal.any.js From a10f6899dc7f9f5f51cb3311048f6231854fb0c9 Mon Sep 17 00:00:00 2001 From: Masashi Hirano Date: Fri, 20 May 2022 23:54:49 +0900 Subject: [PATCH 4/7] fix error message in test Co-authored-by: Antoine du Hamel --- .../test-whatwg-events-add-event-listener-options-signal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js index d4d9ee1c5c11..4d24302c5e1c 100644 --- a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js +++ b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js @@ -161,6 +161,6 @@ const { { const et = new EventTarget(); throws(() => et.addEventListener('foo', () => {}, { signal: null }), { - message: 'The "options.signal" property must be an instance of AbortSignal. Received null', + name: 'TypeError', }); } From 8607445d8625b411fefcbdd51425d0af6259db62 Mon Sep 17 00:00:00 2001 From: shisama Date: Sat, 21 May 2022 22:58:19 +0900 Subject: [PATCH 5/7] fix touse validateAbortSignal to check signal --- lib/internal/event_target.js | 6 ++---- .../test-whatwg-events-add-event-listener-options-signal.js | 6 ++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 76f8afa4442a..a539a7188c77 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -34,7 +34,7 @@ const { ERR_INVALID_THIS, } } = require('internal/errors'); -const { validateObject, validateString, validateInternalField } = require('internal/validators'); +const { validateAbortSignal, validateObject, validateString, validateInternalField } = require('internal/validators'); const { customInspectSymbol, @@ -575,9 +575,7 @@ class EventTarget { } type = String(type); - if (signal === null) { - throw new ERR_INVALID_ARG_TYPE('options.signal', 'AbortSignal', signal); - } + validateAbortSignal(signal, 'options.signal'); if (signal) { if (signal.aborted) { diff --git a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js index 4d24302c5e1c..696658a7a675 100644 --- a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js +++ b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js @@ -160,7 +160,9 @@ const { } { const et = new EventTarget(); - throws(() => et.addEventListener('foo', () => {}, { signal: null }), { - name: 'TypeError', + [1, {}, null, false, 'hi'].forEach((signal) => { + throws(() => et.addEventListener('foo', () => {}, { signal }), { + name: 'TypeError', + }); }); } From 2fae6f2f04acd97c06020046c9486aa26457940e Mon Sep 17 00:00:00 2001 From: Masashi Hirano Date: Sun, 22 May 2022 00:42:35 +0900 Subject: [PATCH 6/7] Update test/parallel/test-eventtarget-whatwg-signal.js Co-authored-by: Antoine du Hamel --- .../test-whatwg-events-add-event-listener-options-signal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js index 696658a7a675..460d2ee3f276 100644 --- a/test/parallel/test-whatwg-events-add-event-listener-options-signal.js +++ b/test/parallel/test-whatwg-events-add-event-listener-options-signal.js @@ -160,7 +160,7 @@ const { } { const et = new EventTarget(); - [1, {}, null, false, 'hi'].forEach((signal) => { + [1, 1n, {}, [], null, true, 'hi', Symbol(), () => {}].forEach((signal) => { throws(() => et.addEventListener('foo', () => {}, { signal }), { name: 'TypeError', }); From 8c29e224c00b665658dbc8250ef60172b5cc83dd Mon Sep 17 00:00:00 2001 From: shisama Date: Sun, 2 Oct 2022 15:34:56 +0900 Subject: [PATCH 7/7] Update WPT events test --- test/wpt/status/dom/events.json | 1 - 1 file changed, 1 deletion(-) diff --git a/test/wpt/status/dom/events.json b/test/wpt/status/dom/events.json index 012b73f70001..607ac32ee452 100644 --- a/test/wpt/status/dom/events.json +++ b/test/wpt/status/dom/events.json @@ -11,7 +11,6 @@ "AddEventListenerOptions-signal.any.js": { "fail": { "expected": [ - "Passing null as the signal should throw", "Passing null as the signal should throw (listener is also null)" ] }