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 c60e80d5c5d4c7..76f8afa4442add 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 dfe5810180d90e..125d62f52d30de 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 125d62f52d30de..6a86b4db9bf144 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 6a86b4db9bf144..d4d9ee1c5c1148 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 d4d9ee1c5c1148..4d24302c5e1cda 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 76f8afa4442add..a539a7188c77ed 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 4d24302c5e1cda..696658a7a6754c 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 696658a7a6754c..460d2ee3f27652 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 012b73f70001ae..607ac32ee4528f 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)" ] }