Fix our WebIDL for Safari #909
Conversation
| [Throws] | ||
| void start(optional double when = 0, optional double grainOffset = 0, | ||
| optional double grainDuration); | ||
|
|
||
| [Throws] | ||
| void stop (optional double when = 0); |
There was a problem hiding this comment.
Note that this class didn't switch to AudioBufferSourceNode includes rustAudioScheduledSourceNode; because the mixin's start method conflicts with the start method in this class, so it was just easier to inline the few properties here.
alexcrichton
force-pushed
the
fix-webidl
branch
from
d074e10
to
5fb1bde
Compare
| @@ -32,7 +32,12 @@ interface AudioBufferSourceNode : AudioScheduledSourceNode { | |||
| attribute double loopStart; | |||
| attribute double loopEnd; | |||
|
|
|||
| attribute EventHandler onended; | |||
There was a problem hiding this comment.
Looks like the indentation of this line is wrong.
There was a problem hiding this comment.
Heh it is indeed wrong!
(a copy/paste error)
crates/webidl/src/lib.rs
Outdated
| probably stop using this class and instead \ | ||
| use the class that inherits from this, which \ | ||
| should include all the same methods and work \ | ||
| in all browsers in theory!"); |
There was a problem hiding this comment.
It would be nice if we could tailor the deprecation message for the particular interfaces. Something like:
// BaseAudioContext.webidl
[Deprecated="reason why this is deprecated detailed here..."]
interface BaseAudioContext {
// ...
};There was a problem hiding this comment.
Sure yeah, I can upate this
| @@ -16,8 +16,6 @@ features = [ | |||
| 'AudioDestinationNode', | |||
| 'AudioNode', | |||
| 'AudioParam', | |||
| 'AudioScheduledSourceNode', | |||
| 'BaseAudioContext', | |||
There was a problem hiding this comment.
Isn't removing features a breaking change?
There was a problem hiding this comment.
Oh these aren't removed from web-sys itself, just here because the features are no longer needed
There was a problem hiding this comment.
D'oh I totally misread what file this was
| let fm_osc; | ||
| let gain; | ||
| let fm_gain; | ||
| pub fn new() -> Result<FmOsc, JsValue> { |
This commit employs the strategy described in rustwasm#908 to apply a non-breaking change to fix WebIDL to be compatible with all browsers, including Safari. The problem here is that `BaseAudioContext` and `AudioScheduledSourceNode` are not types in Safari, but they are types in Firefox/Chrome. The fix here was to move the contents of these two interfaces into mixins, and then include the mixins in all classes which inherit from these two classes. That should have the same effect as defining the methods inherently on the original interface. Additionally a special `[RustDeprecated]` attribute to WebIDL was added to signify interfaces this has happened to. Currently it's directly tailored towards this case of "this intermediate class doesn't exist in all browsers", but we may want to refine and extend the deprecation message over time. Although it's possible we could do this as a breaking change to `web-sys` I'm hoping that we can do this as a non-breaking change for now and then eventually on the next breaking release batch all these changes together, deleting the intermediate classes. This is also hopefully a good trial run for how stable web-sys can be when it's actually stable! cc rustwasm#897 cc rustwasm#908
alexcrichton
force-pushed
the
fix-webidl
branch
from
5fb1bde
to
bb7271a
Compare
|
Updated! |
This commit employs the strategy described in #908 to apply a
non-breaking change to fix WebIDL to be compatible with all browsers,
including Safari.
The problem here is that
BaseAudioContextandAudioScheduledSourceNodeare not types in Safari, but they are types in Firefox/Chrome. The fix
here was to move the contents of these two interfaces into mixins, and
then include the mixins in all classes which inherit from these two
classes. That should have the same effect as defining the methods
inherently on the original interface.
Additionally a special
[RustDeprecated]attribute to WebIDL was addedto signify interfaces this has happened to. Currently it's directly
tailored towards this case of "this intermediate class doesn't exist in
all browsers", but we may want to refine and extend the deprecation
message over time.
Although it's possible we could do this as a breaking change to
web-sysI'm hoping that we can do this as a non-breaking change fornow and then eventually on the next breaking release batch all these
changes together, deleting the intermediate classes. This is also
hopefully a good trial run for how stable web-sys can be when it's
actually stable!
cc #897
cc #908