Skip to content
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

MessageEvent cannot be prevented even if cancelable is true #51767

Open
kettanaito opened this issue Feb 15, 2024 · 7 comments
Open

MessageEvent cannot be prevented even if cancelable is true #51767

kettanaito opened this issue Feb 15, 2024 · 7 comments

Comments

@kettanaito
Copy link

kettanaito commented Feb 15, 2024

Version

v18.14.2 (the same on LTS v20.11.0)

Platform

Darwin imac.home 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul 5 22:21:56 PDT 2023; root:xnu-8796.141.3~6/RELEASE_X86_64 x86_64

Subsystem

No response

What steps will reproduce the bug?

// example.js
let t = new EventTarget()
t.addEventListener('message', (e) => console.log('cancelable?', e.cancelable))
t.dispatchEvent(new MessageEvent('message', { cancelable: true }))
node example.js

How often does it reproduce? Is there a required condition?

The issue can be reproduced every time.

What is the expected behavior? Why is that the expected behavior?

I expect the following:

  1. e.cancelable is true in the event listener for "message'.
  2. Calling e.preventDefault() will set e.defaultPrevented to true.

In other words, I expect the same behavior as with the regular Event instance:

let t = new EventTarget()
t.addEventListener('message', (e) => console.log('cancelable?', e.cancelable))

// Regular Event behaves correctly and can be canceled. 
t.dispatchEvent(new Event('message', { cancelable: true }))

What do you see instead?

The cancelable attribute on the MessageEvent instance is always false, no matter the cancelable value in the event init dictionary.

Calling e.preventDefault() has no effect on e.defaultPrevented (much likely because the cancelable attribute is false.

Additional information

I'm constructing a MessageEvent that I then dispatch on EventTarget and want to have the listeners being able to cancel that event (prevent its default).

@kettanaito
Copy link
Author

This is either an intentional behavior or a bug. In case this is intentional, can we please get some clarification as to why and have the docs updated to mention this behavior difference?

Note that both Event and MessageEvent scenarios run correctly in the browser (both events are cancelable and their defaults can be prevented).

@kettanaito
Copy link
Author

The only detail I can spot is that the event init dictionary for the MessageEvent is being transformed here:

eventInitDict = webidl.converters.MessageEventInit(eventInitDict)

I don't know if this MessageEvent class is what we get as a global in Node.js though. I imagine it is since message events are WebSocket-specific events.

@KhafraDev
Copy link
Member

KhafraDev commented Feb 15, 2024

class MessageEvent extends Event {

undici's MessageEvent isn't exposed, I had no idea node implemented it honestly.

@kettanaito
Copy link
Author

@KhafraDev, it seems to be a global MessageEvent class, nothing from Undici. You can reproduce the issue using the code snippets I posted above even in a terminal (no dependencies).

@kettanaito
Copy link
Author

A one-liner reproduction:

> new MessageEvent('message', { cancelable: true }).cancelable
false

The base Event class is not affected though:

new Event('message', { cancelable: true }).cancelable
true

Can MessageEvent be faulty in this regard?

@KhafraDev
Copy link
Member

This is not reproducible on the latest version of node. I replaced the old MessageEvent with undici's.

@kettanaito
Copy link
Author

kettanaito commented Sep 27, 2024

That's good to hear! Thank you so much. Will remove the polyfill once we support v20/v22 as the minimal version. Feel free to close this if the issue is not reproducible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants