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

Handle promise rejection for sound.play() in preload #2162

Merged
merged 1 commit into from Dec 10, 2018

Conversation

epicfaace
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

audio play() called before user interaction #1902

Handle promise rejection for sound.play() in preload. This removes the error, "play() failed because the user didn't interact with the document first."

Test Coverage

Tested on:

  • Desktop Chrome

Mac OS X

Additional Information

Copy link
Contributor

@RoboErikG RoboErikG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, hit approve too quickly. Couple problems,

  1. This uses an ES6 feature. Blockly is still written in ES5 for compatibility with older browsers.
  2. This pull request is against master, it should be against develop.

@epicfaace epicfaace changed the base branch from master to develop December 10, 2018 23:03
@epicfaace
Copy link
Contributor Author

@RoboErikG thanks, I've fixed those problems.

Why doesn't Blockly use Babel?

@AnmAtAnm
Copy link
Contributor

AnmAtAnm commented Jan 3, 2019

@RoboErikG @rachel-fenichel
I think this is still a problem for older browsers. The return value of play() will be undefined in older browsers, and that needs to be tested before calling catch(..). Fixing this would probably make this a solid solution.
Unfortunately, this was pushed into the latest release.

@epicfaace I don't think Babel would fix this particular case.

@rachel-fenichel
Copy link
Collaborator

@AnmAtAnm so does this now fail differently for older browsers, and in a way that we need to be concerned about? Or is it just that this solution doesn't get rid of the warning in the older browsers?

@tuxianchao
Copy link

In my project, play-request-was-interrupted this is how I solved the problem.

@NeilFraser
Copy link
Member

Any changes to sound need to be tested on all browsers (including IE), iOS, and Android. Sound is a surprisingly immature and inconsistent beast. The current Rube Goldberg method in which Blockly plays sound is a result of playing whack-a-mole until a working solution emerged. Specs and logic are irrelevant, it's all driven by the random implementations of each browser.

@tuxianchao
Copy link

@RoboErikG @rachel-fenichel
I think this is still a problem for older browsers. The return value of play() will be undefined in older browsers, and that needs to be tested before calling catch(..). Fixing this would probably make this a solid solution.
Unfortunately, this was pushed into the latest release.

@epicfaace I don't think Babel would fix this particular case.

may be like this,check play() method return promise before pause()

var playPromise =  sound.play();
if(playPromise !== undefined){
     playPromise.then(function(){
         sound.pause();
     }).catch(function(error){
          console.error(error);
         //....
     });
}

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

Successfully merging this pull request may close these issues.

None yet

6 participants