WebAudio / web-audio-api

The Web Audio API v1.0, developed by the W3C Audio WG
https://webaudio.github.io/web-audio-api/
Other
1.04k stars 165 forks source link

Link event handlers IDL attributes to event types #2516

Closed tidoust closed 1 year ago

tidoust commented 1 year ago

Changes in #2498 introduce a weird phrasing for "fire an event", e.g. "Fire an event to onstatechange EventHandler". Correct phrasing should rather be "Fire an event named statechange".

This made me realize that the spec never associates the event handler IDL attributes that it defines with the actual event handler event type that gets fired. The spec also seems to assume that there will be at most one event handler (e.g. when it says that "This AudioBuffer is only valid while in the scope of the onaudioprocess function"), whereas onxxx is just one way to listen to xxx events, addEventListener may also be used.

Specs typically make the association between event handler IDL attributes and event handler event types explicit, e.g. as done in HTML through tables such as: https://html.spec.whatwg.org/multipage/webappapis.html#event-handlers-on-elements,-document-objects,-and-window-objects

... or in WebRTC through text in the definition of the IDL attribute: https://w3c.github.io/webrtc-pc/#ref-for-event-datachannel-bufferedamountlow-1

This pull request adds definitions of event types next to the definitions of the onxxx IDL attributes with which they are associated and uses references to these event types whenever the spec fires an event or mentions event handlers.

I tried to keep the changes minimal. I was going to argue that these changes are editorial in nature as they merely clarify something that did not create ambiguities for implementations. Now, you seem to track all changes with "proposed corrections", regardless of whether they're editorial or substantive so I suspect you cannot accept these changes as-is.

At a minimum, I think that the occurrences of "fire an event to onxxx" should be fixed (they only appear in proposed corrections so that seems doable without creating additional proposed corrections). I can prepare a separate PR to that effect. I can also look into creating appropriate "proposed corrections" structures for the other changes if they seem useful. That may not be worth the hassle ;)


Preview | Diff

tidoust commented 1 year ago

My only question is how to handle the propose addition markup on this. If you believe it's not necessary because this is a non-substantive change, we can simply merge this.

Yes, it's up to you but I would argue that these changes are corrections that do not affect conformance: they are meant to align the spec with the wording used in most other specs but I would be extremely surprised if the previous text had created any ambiguity for implementations that this update would resolve.

I don't think that the W3C process requires "proposed addition" markup for such changes. Unless the Audio Working Group resolved that all changes need to be wrapped in "proposed addition" markup, I would keep things simple, be it only to improve readability and ease maintenance on your end.

hoch commented 1 year ago

I would keep things simple, be it only to improve readability and ease maintenance on your end.

Agreed.

hoch commented 1 year ago

@padenot Friendly ping - I don't think this change is controversial. Would like to land if you're okay! :)