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

Event constructors are missing arguments #1067

Closed
RByers opened this issue Nov 10, 2016 · 6 comments
Closed

Event constructors are missing arguments #1067

RByers opened this issue Nov 10, 2016 · 6 comments
Assignees
Milestone

Comments

@RByers
Copy link

RByers commented Nov 10, 2016

The constructors added in #566 don't follow the pattern of other event constructors. I believe just saying [Constructpr]on OfflineAudioCompletionEvent means you can do new OfflineAudioCompletionEvent() but you can't do anything interesting with that because there's no good way to set the event properties (including the event type). Instead you want to take the event type string and a dictionary (which inherits from the EventInit dictionary) so that developers can do new OfflineAudioCompletionEvent("complete", {renderedBuffer: buf}).

Eg. see the definition of CustomEvent and MouseEvent.

@rtoy
Copy link
Member

rtoy commented Nov 10, 2016

Yes, this makes sense to me and we should do this.

@joeberkovitz joeberkovitz self-assigned this Nov 10, 2016
@joeberkovitz
Copy link
Contributor

This was my mistake.

@hoch
Copy link
Member

hoch commented Nov 10, 2016

@RByers Rick, I have few questions on this - just to understand how this custom event works.

  1. How do you target a certain OfflineAudioContext in the constructor of the event?
  2. If you fire the event without specifying a target context, what happens?
  3. What if the context is not started when an event is fired?
  4. What if the context is still rendering the audio buffer when an event is fired?

I would love to change the name of the event to OfflineRenderCompletionEvent if the WG agrees.

@RByers
Copy link
Author

RByers commented Nov 13, 2016

How do you target a certain OfflineAudioContext in the constructor of the event?

All the constructor lets you do is initialize an event object, passing values through directly - exactly as the browser does internally. So you just accept values in the "Init" dictionary that correspond to each of the properties on the event. For OfflineAudioCompletionEvent it looks like that means you'd take a renderedBuffer dictionary member (which itself references an AudioContext, right?). There's no smarts needed in the event though - it's just a dumb container of information.

If you fire the event without specifying a target context, what happens?

Events are dumb, so if the event type allows renderedBuffer to be undefined (or null) then someone could generate such an event from JS using the constructor. That's fine. It's no different then, for example, some JS generating a MouseEvent with non-sensical values.

What if the context is not started when an event is fired?
What if the context is still rendering the audio buffer when an event is fired?

Again none of this should matter - the event is just a dumb container, doesn't interact with any of the webaudio code inside the UA itself. Sure if a library actually generated bogus events it might confuse some other JS on the page, but that's fine (no different from all the other ways one piece of JS can confuse another).

Think of it this way: a DOM event is just a generic notification with some particular bundle of information attached. The information flow is one way from the browser to JS via the events. Creating a synthetic event in JS is completely hidden from (and so irrelevant to) any of the browser WebAudio code.

@rtoy rtoy assigned rtoy and unassigned joeberkovitz Nov 14, 2016
@rtoy
Copy link
Member

rtoy commented Nov 14, 2016

@joeberkovitz Since I was working on an intent to implement and ship for this in Chrome, I'm going to assign this to me, if you don't mind. Please feel free to change it back.

@rtoy
Copy link
Member

rtoy commented Nov 14, 2016

The proposed constructors look like

Constructor(DOMString type, AudioProcessingEventInit eventInitDict)
dictionary AudioProcessingEventInit : EventInit {
    required double playbackTime;
    required AudioBuffer inputBuffer;
    required AudioBuffer outputBuffer;
};

and

Constructor(DOMString type, OfflineAudioCompletionEventInit eventInitDict)
dictionary OfflineAudioCompletionEventInit : EventInit {
    required AudioBuffer renderedBuffer;
};

@rtoy rtoy closed this as completed in ed5a4a7 Nov 15, 2016
rtoy added a commit that referenced this issue Nov 15, 2016
Fix #1067: Add arguments to event constructors
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

No branches or pull requests

4 participants