Skip to content

Commit

Permalink
fix: throw error on muted resolution rejection during autoplay (#7293)
Browse files Browse the repository at this point in the history
Previously, when autoplay was set to any or muted, we would accidentally swallow the autoplay rejection when we reset the muted state back to what it was. Instead, we want to re-throw the error.
To get it working, we also had to update our tests to try/catch in our fake promise.
  • Loading branch information
roman-bc-dev committed Jun 30, 2021
1 parent 0f70787 commit f9fb1d3
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 11 deletions.
19 changes: 12 additions & 7 deletions src/js/player.js
Expand Up @@ -1430,7 +1430,9 @@ class Player extends Component {
return;
}

const muted = () => {
// Save original muted() value, set muted to true, and attempt to play().
// On promise rejection, restore muted from saved value
const resolveMuted = () => {
const previouslyMuted = this.muted();

this.muted(true);
Expand All @@ -1448,21 +1450,24 @@ class Player extends Component {
return;
}

return mutedPromise.catch(restoreMuted);
return mutedPromise.catch(err => {
restoreMuted();
throw new Error(`Rejection at manualAutoplay. Restoring muted value. ${err ? err : ''}`);
});
};

let promise;

// if muted defaults to true
// the only thing we can do is call play
if (type === 'any' && this.muted() !== true) {
if (type === 'any' && !this.muted()) {
promise = this.play();

if (isPromise(promise)) {
promise = promise.catch(muted);
promise = promise.catch(resolveMuted);
}
} else if (type === 'muted' && this.muted() !== true) {
promise = muted();
} else if (type === 'muted' && !this.muted()) {
promise = resolveMuted();
} else {
promise = this.play();
}
Expand All @@ -1473,7 +1478,7 @@ class Player extends Component {

return promise.then(() => {
this.trigger({type: 'autoplay-success', autoplay: type});
}).catch((e) => {
}).catch(() => {
this.trigger({type: 'autoplay-failure', autoplay: type});
});
}
Expand Down
18 changes: 14 additions & 4 deletions test/unit/autoplay.test.js
Expand Up @@ -29,18 +29,25 @@ QUnit.module('autoplay', {

fixture.appendChild(videoTag);

// this promise fake will act right away
// it will also only act on catch calls
// These mock promises immediately execute,
// effectively synchronising promise chains for testing

// This will only act on catch calls
this.rejectPromise = {
then(fn) {
return this;
},
catch(fn) {
fn();
try {
fn();
} catch (err) {
return this;
}
return this;
}
};

// This will only act on then calls
this.resolvePromise = {
then(fn) {
fn();
Expand Down Expand Up @@ -274,7 +281,10 @@ QUnit.test('option = "any" play, no muted, rejection leads to muted then play',
assert.equal(this.player.autoplay(), 'any', 'player.autoplay getter');
assert.equal(this.player.tech_.autoplay(), false, 'tech.autoplay getter');

// muted called twice here, as muted is value is restored on failure.
// The workflow described here:
// Call play() -> on rejection, attempt to set mute to true ->
// call play() again -> on rejection, set original mute value ->
// catch failure at the end of promise chain
this.player.tech_.trigger('loadstart');
assert.equal(this.counts.play, 2, 'play count');
assert.equal(this.counts.muted, 2, 'muted count');
Expand Down

0 comments on commit f9fb1d3

Please sign in to comment.