search

WebAudio / web-midi-api

The Web MIDI API, developed by the W3C Audio WG
http://webaudio.github.io/web-midi-api/
Other
321 stars 55 forks source link

Make MIDIMessageEventInit data and MIDIConnectionEventInit port explicitly nullable #257

Closed mjwilson-google closed 8 months ago

mjwilson-google commented 8 months ago

252 made MIDIMessageEvent data and MIDIConnectionEvent port nullable.

Based on this comment: https://github.com/WebAudio/web-midi-api/issues/233#issuecomment-1131480459 I didn't make the init dictionary members nullable.

However, Chromium currently has these dictionary members nullable: https://crsrc.org/c/third_party/blink/renderer/modules/webmidi/midi_connection_event_init.idl;l=11 https://crsrc.org/c/third_party/blink/renderer/modules/webmidi/midi_message_event_init.idl;l=11

And Firefox has port nullable (but not data): https://searchfox.org/mozilla-central/source/dom/webidl/MIDIConnectionEvent.webidl#23 https://searchfox.org/mozilla-central/source/dom/webidl/MIDIMessageEvent.webidl#24

Since the intent was to codify the existing Chromium behavior, I think this means that we should make them both nullable.

This actually does make a difference, since testing in Chromium we get the current behavior:

new MIDIMessageEvent("something", {data: null}).data null

new MIDIConnectionEvent("something", {port: null}).port null

But after aligning the IDL with the current spec we get the following:

new MIDIMessageEvent("something", {data: null}).data VM261:1 Uncaught TypeError: Failed to construct 'MIDIMessageEvent': Failed to read the 'data' property from 'MIDIMessageEventInit': Failed to convert value to 'Uint8Array'. at :1:1

new MIDIConnectionEvent("something", {port: null}).port VM285:1 Uncaught TypeError: Failed to construct 'MIDIConnectionEvent': Failed to read the 'port' property from 'MIDIConnectionEventInit': Failed to convert value to 'MIDIPort'. at :1:1

@padenot especially, what do you think?

mjwilson-google commented 8 months ago

This is related to https://github.com/WebAudio/web-midi-api/issues/168 and https://github.com/WebAudio/web-midi-api/issues/233.

mjwilson-google commented 8 months ago

After further discussion with the Chromium API owners, this change isn't appropriate. Even though it will change Chromium behavior in this specific case, keeping dictionary members not required and not adding the ? is better for web platform conventions so we shouldn't merge this change. I will close this.