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

OscillatorOptions periodicWave and type behavior defined inconsistently #1173

Closed
rtoy opened this issue Mar 21, 2017 · 2 comments
Closed

OscillatorOptions periodicWave and type behavior defined inconsistently #1173

rtoy opened this issue Mar 21, 2017 · 2 comments
Assignees
Milestone

Comments

@rtoy
Copy link
Member

rtoy commented Mar 21, 2017

In https://webaudio.github.io/web-audio-api/#dictionary-oscillatoroptions-members, the description for the periodicWave member says

The PeriodicWave for the OscillatorNode. If this is specified, then the type member must either be unspecified or set to "custom".

It's not possible for the type member to be unspecified because it has a default value of "sine".

Two options here:

  1. Remove the default value for type, so that if you specify a periodicWave, the type defaults to 'custom' and otherwise throws an error if the type is given and not "custom".
  2. Change the description and remove the part about type being unspecified. If you want a periodicWave, you have to specify explicitly a type of "custom"

Option 1 is convenient; option 2 is more strictly defined.

Not sure which is better.

@rtoy
Copy link
Member Author

rtoy commented Mar 21, 2017

Option 1 has the potential to break existing uses of OscillatorNode constructors. For example, Chrome launched the constructors without these defaults (because they weren't defined at that time). This means new OscillatorNode(c, {periodicWave: wave}) would now signal an error with option 1 because type defaults to "sine", which isn't compatible.

A third option: if periodicWave is given, then any value for type is ignored; it is treated as if type were set to "custom". I think this would it backward compatible.

@joeberkovitz
Copy link
Contributor

We're going with option 3: if periodicWave is given, then any value for type is ignored; it is treated as if type were set to "custom".

@rtoy rtoy self-assigned this Mar 30, 2017
rtoy added a commit to rtoy/web-audio-api that referenced this issue Mar 30, 2017
In the OscillatorOptions dictionary, if both type and periodicWave are
specified, the actual type is ignored and treated as if it were set to
"custom".

This implements option three from WebAudio#1173.
foolip added a commit to foolip/browser-compat-data that referenced this issue Sep 15, 2022
These notes have been in BCD since the very beginning:
mdn#433

Chrome 55 is the right version for the constructors overall:
https://storage.googleapis.com/chromium-find-releases-static/579.html#57909df69be0cf0b2ac42e5c9c018d4a00ee9766

The Chrome 59 change is this one:
https://storage.googleapis.com/chromium-find-releases-static/ef0.html#ef006156fd62853a893617ca104f4ac2045e19c1

Adding default values to dictionary members is by itself not necessarily
a web observable changes at all. This was confirmed with one case
comparing Chrome 58 and 59:
http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=10708

One part of this was observable, with the OscillatorNode constructor:
https://bugs.chromium.org/p/chromium/issues/detail?id=710471

This was a spec bug, and fixed in the spec around the same time:
WebAudio/web-audio-api#1173

Overall, this change does not seem noteworthy enough to record in BCD.
Elchi3 pushed a commit to mdn/browser-compat-data that referenced this issue Sep 15, 2022
These notes have been in BCD since the very beginning:
#433

Chrome 55 is the right version for the constructors overall:
https://storage.googleapis.com/chromium-find-releases-static/579.html#57909df69be0cf0b2ac42e5c9c018d4a00ee9766

The Chrome 59 change is this one:
https://storage.googleapis.com/chromium-find-releases-static/ef0.html#ef006156fd62853a893617ca104f4ac2045e19c1

Adding default values to dictionary members is by itself not necessarily
a web observable changes at all. This was confirmed with one case
comparing Chrome 58 and 59:
http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=10708

One part of this was observable, with the OscillatorNode constructor:
https://bugs.chromium.org/p/chromium/issues/detail?id=710471

This was a spec bug, and fixed in the spec around the same time:
WebAudio/web-audio-api#1173

Overall, this change does not seem noteworthy enough to record in BCD.
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

2 participants