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

Silence "The play() request was interrupted by a call to pause()" #3518

Closed
wants to merge 2 commits into from

Conversation

mister-ben
Copy link
Contributor

Description

On browsers that return a promise for play() (so far Chrome), an AbortError DOMException is thrown if a pause() occurs before playback starts. This doesn't break anything, but logs an annoying error to the console.

Specific Changes proposed

Just catch the exception and do nothing.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Unit Tests updated or fixed
  • Reviewed by Two Core Contributors

// Catch/silence error when a pause interrupts a play request
// on browsers which return a promise
if (playPromise !== undefined) {
playPromise.catch((e) => {});
Copy link
Member

Choose a reason for hiding this comment

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

catch might be an issue on IE8.
Also, we should probably verify this is a promise before proceeding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, reworked that.

@@ -517,3 +517,20 @@ QUnit.test('Html5#reset calls Html5.resetMediaElement when called', function() {

Html5.resetMediaElement = oldResetMedia;
});

QUnit.test('Exception in play promise should be caught', function() {
Copy link
Member

Choose a reason for hiding this comment

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

I think this test should be wrapped in a condition to check for Promise support. When it gets merged and runs in a browser that doesn't support Promise it will likely blow up.

Copy link
Member

Choose a reason for hiding this comment

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

we're running es6-shim on tests, so, Promise is always available.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes. 👍

@misteroneill
Copy link
Member

In that case, LGTM

@gkatsev
Copy link
Member

gkatsev commented Aug 11, 2016

LGTM

@gkatsev gkatsev added confirmed patch This PR can be added to a patch release. labels Aug 11, 2016
@gkatsev
Copy link
Member

gkatsev commented Aug 15, 2016

This fixes #3474

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed patch This PR can be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants