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.

Sign up for GitHub

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

Jump to bottom

Webaudio example fails with error in Safari 12 on macOS Mojave (works in Firefox) #897

Closed
kud1ing opened this issue Sep 27, 2018 · 10 comments
Closed

Webaudio example fails with error in Safari 12 on macOS Mojave (works in Firefox) #897

kud1ing opened this issue Sep 27, 2018 · 10 comments

Comments

@kud1ing
Copy link

kud1ing commented Sep 27, 2018 •
edited
Loading

The web development console says for https://rustwasm.github.io/wasm-bindgen/exbuild/webaudio/:

[Error] Unhandled Promise Rejection: ReferenceError: Can't find variable: AudioContext
	(anonymous function)
	promiseReactionJob
@kud1ing kud1ing changed the title Webaudio example gives error in Safari on Mac (works in Firefox) Webaudio example gives error in Safari 12 on Mac (works in Firefox) Sep 27, 2018
@kud1ing kud1ing changed the title Webaudio example gives error in Safari 12 on Mac (works in Firefox) Webaudio example fails with error in Safari 12 on Mac (works in Firefox) Sep 27, 2018
@kud1ing kud1ing changed the title Webaudio example fails with error in Safari 12 on Mac (works in Firefox) Webaudio example fails with error in Safari 12 on macOs Mojave (works in Firefox) Sep 27, 2018
@kud1ing kud1ing changed the title Webaudio example fails with error in Safari 12 on macOs Mojave (works in Firefox) Webaudio example fails with error in Safari 12 on macOS Mojave (works in Firefox) Sep 27, 2018
@chinedufn
Copy link
Contributor

Web audio is supported with a prefix in safari, so the example would need to fall back to webkitAudioContext if there was no AudioContext

From a quick search it doesn't seem like web-sys supports any prefixed APIs (I could very well be missing something though!).

If that's the case I'm also not sure if that's an intentional decision or not.

@alexcrichton
Copy link
Contributor

Thanks for the report and thanks for the info @chinedufn! We discussed this during today's meeting and the conclusion was that we'd like to add support to web-sys to whitelist APIs as "these are alternative class names". That way we can get this working by default even in Safari where it's renamed to webkitAudioContext.

@alexcrichton alexcrichton mentioned this issue Sep 28, 2018
Merged
alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Sep 28, 2018
@alexcrichton
Add catch(console.error) to all examples
bc36b6f 
Some examples have been failing to load in some browsers, and this
ensures that whenever the promise to load Rust code fails we log any
errors happening instead of accidentally failing silently.

This helped debug a bit in rustwasm#897
@alexcrichton alexcrichton mentioned this issue Sep 28, 2018
Merged
@alexcrichton
Copy link
Contributor

#906 fixes using webkitAudioContext on Safari, but unfortunately the example still doesn't work in Safari. The AudioScheduledSourceNode type doesn't exist in Safari (and edge presumably) but rather the methods are just listed in the parent classes. I've written up a more detailed issue about that at #908

alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Sep 28, 2018
@alexcrichton
Fix our WebIDL for Safari
d074e10 
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 alexcrichton mentioned this issue Sep 28, 2018
Merged
alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Sep 28, 2018
@alexcrichton
Fix our WebIDL for Safari
5fb1bde 
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
Copy link
Contributor

The combination of #909 and #906 should fix this issue

alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Oct 1, 2018
@alexcrichton
Fix our WebIDL for Safari
bb7271a 
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
@kud1ing kud1ing closed this as completed Oct 1, 2018
@alexcrichton
Copy link
Contributor

Ok I've confirmed the deployed example does indeed work in safari now!

@kud1ing
Copy link
Author

kud1ing commented Oct 2, 2018 •
edited
Loading

It works for me too now, thanks.

I've closed this issue yesterday already because the original problem above was fixed and the outstanding error was covered by #908. This too seems to be fixed (at least for this example) now.

@tat-m3dicine
Copy link

Do you guys have in mind for webkitOfflineAudioContext ? Currently new_with_context_options and new_with_number_of_channels_and_length_and_sample_rate returns OfflineAudioContext which safari doesn't support

@alexcrichton
Copy link
Contributor

@tat-m3dicine is that perhaps another case that's needed here? Would that fix the issue for you?

@tat-m3dicine
Copy link

@alexcrichton Indeed, it's missing another case. Atm, I will just deal with OfflineAudioContext using extern "C" {...}. Do you guys have in mind when will be the release date for this update?

@alexcrichton alexcrichton mentioned this issue Nov 20, 2019
Merged
@alexcrichton
Copy link
Contributor

I've created #1865 for this, @tat-m3dicine mind trying that out and seeing if it works for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@alexcrichton @kud1ing @chinedufn @tat-m3dicine