-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
check if browser supports AudioContext.close in webaudio.js #962
Conversation
add check for browser support of AudioContext.close(). This is only supported by Chrome 43+ and FireFox 40+ for now. Edge and other non supported browsers will give an undefined error. https://developer.mozilla.org/en-US/docs/Web/API/AudioContext/close // check if browser supports AudioContext.close() if(typeof this.ac.close() !== 'undefined'){ this.ac.close(); }
src/webaudio.js
Outdated
@@ -286,7 +286,10 @@ WaveSurfer.WebAudio = { | |||
// close the audioContext if it was created by wavesurfer | |||
// not passed in as a parameter | |||
if (!this.params.audioContext) { | |||
this.ac.close(); | |||
// check if browser supports AudioContext.close() | |||
if(typeof this.ac.close() !== 'undefined'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add spaces around the parentheses like line 288? Weird our style checker didn't notice it, I thought it did before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thijstriemstra done
src/webaudio.js
Outdated
@@ -286,7 +286,10 @@ WaveSurfer.WebAudio = { | |||
// close the audioContext if it was created by wavesurfer | |||
// not passed in as a parameter | |||
if (!this.params.audioContext) { | |||
this.ac.close(); | |||
// check if browser supports AudioContext.close() | |||
if (typeof this.ac.close() !== 'undefined'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missed one
Also can you not use 'Update filename.js' as title for your PRs but give it a few word title that describes the change better. |
add check for browser support of AudioContext.close(). This is only supported by Chrome 43+ and FireFox 40+ for now. Edge and other non supported browsers will give an undefined error. #949
https://developer.mozilla.org/en-US/docs/Web/API/AudioContext/close
// check if browser supports AudioContext.close()
if(typeof this.ac.close() !== 'undefined'){
this.ac.close();
}