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

PeriodicWave's constructor is not valid IDL #1116

Closed
bzbarsky opened this issue Dec 17, 2016 · 5 comments
Closed

PeriodicWave's constructor is not valid IDL #1116

bzbarsky opened this issue Dec 17, 2016 · 5 comments
Assignees
Milestone

Comments

@bzbarsky
Copy link

The dictionary without required members in trailing position needs to be optional, per IDL.

@rtoy
Copy link
Member

rtoy commented Dec 19, 2016

In https://webaudio.github.io/web-audio-api/#periodicwaveoptions, the text says that one of real or imag dictionary member is required. Can that be expressed in IDL somehow? Or maybe we can default both of them to some sensible default and thus make both members truly optional?

@bzbarsky
Copy link
Author

Can that be expressed in IDL somehow?

Not at the moment.

Or maybe we can default both of them to some sensible default and thus make both members truly optional?

It's hard to say, because the behavior of this constructor is pretty much completely undefined. See #1120

@mdjp mdjp added this to the Web Audio V1 milestone Jan 5, 2017
@rtoy
Copy link
Member

rtoy commented Jan 6, 2017

In the teleconf, a couple of things were suggested:

  1. Leave it as is
  2. Make dictionary optional, and make the real and imag members truly optional. This can having PeriodicWave default to a sine wave if PeriodicWaveOptions is not given.

@bzbarsky
Copy link
Author

bzbarsky commented Jan 6, 2017

You can't leave it as is, because as is is not valid IDL....

@rtoy
Copy link
Member

rtoy commented Feb 23, 2017

The proposal in PR #1116 is option 2: Make the dictionary optional, and specify what happens if both real and imag are not given.

@mdjp mdjp added the V1 Blocker label Mar 2, 2017
@rtoy rtoy closed this as completed in a28c946 Mar 16, 2017
rtoy added a commit that referenced this issue Mar 16, 2017
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