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

fix(viewer): Pause audio instead of muting it if autoplay is prevented #1068

Merged
merged 9 commits into from
Sep 17, 2019
Merged
9 changes: 9 additions & 0 deletions src/lib/viewers/media/MP3Viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ class MP3Viewer extends MediaBaseViewer {
this.mediaControls.show();
this.mediaControls.resizeTimeScrubber();
}

/**
* Auto-play was prevented, pause the audio
*
* @override
*/
handleAutoplayFail = () => {
this.pause();
};
}

export default MP3Viewer;
54 changes: 28 additions & 26 deletions src/lib/viewers/media/MediaBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const TIMESTAMP_UNIT_NAME = 'timestamp';
const INITIAL_TIME_IN_SECONDS = 0;
const ONE_MINUTE_IN_SECONDS = 60;
const ONE_HOUR_IN_SECONDS = 60 * ONE_MINUTE_IN_SECONDS;
const PLAY_PROMISE_NOT_SUPPORTED = 'play_promise_not_supported';

class MediaBaseViewer extends BaseViewer {
/**
Expand Down Expand Up @@ -303,35 +304,28 @@ class MediaBaseViewer extends BaseViewer {
}

/**
* Determines if media should autoplay based on cached settings value.
* Handler for autoplay failure
* Overridden in child class
*
* @protected
*/
handleAutoplayFail = () => {};

/**
* Autoplay the media
*
* @private
* @emits volume
* @return {Promise}
mxiao6 marked this conversation as resolved.
Show resolved Hide resolved
*/
autoplay() {
// Play may return a promise depending on browser support. This promise
// will resolve when playback starts. If it fails, we mute the video
// and try to play again.
// https://webkit.org/blog/7734/auto-play-policy-changes-for-macos/
const autoPlayPromise = this.mediaEl.play();

if (autoPlayPromise && typeof autoPlayPromise.then === 'function') {
this.handleRate();
return autoPlayPromise
.then(() => {
this.handleVolume();
mxiao6 marked this conversation as resolved.
Show resolved Hide resolved
})
.catch(() => {
// Auto-play was prevented, try muted play
this.setVolume(0);
this.mediaEl.play();
});
}

// Fallback to traditional autoplay tag if play does not return a promise
this.mediaEl.autoplay = true;
return Promise.resolve();
return this.play().catch(error => {
if (error.message === PLAY_PROMISE_NOT_SUPPORTED) {
// Fallback to traditional autoplay tag if mediaEl.play does not return a promise
this.mediaEl.autoplay = true;
} else {
this.handleAutoplayFail();
}
});
}

/**
Expand Down Expand Up @@ -570,7 +564,7 @@ class MediaBaseViewer extends BaseViewer {
* @param {number} start - start time in seconds
* @param {number} end - end time in seconds
* @emits play
* @return {void}
* @return {Promise}
mxiao6 marked this conversation as resolved.
Show resolved Hide resolved
*/
play(start, end) {
const hasValidStart = this.isValidTime(start);
Expand All @@ -584,10 +578,18 @@ class MediaBaseViewer extends BaseViewer {
this.setMediaTime(start);
}
if (arguments.length === 0 || hasValidStart) {
this.mediaEl.play();
// Play may return a promise depending on browser support. This promise
// will resolve when playback starts.
// https://webkit.org/blog/7734/auto-play-policy-changes-for-macos/
const playPromise = this.mediaEl.play();
this.handleRate();
this.handleVolume();

return playPromise && typeof playPromise.then === 'function'
? playPromise
: Promise.reject(new Error(PLAY_PROMISE_NOT_SUPPORTED));
}
return Promise.resolve();
}

/**
Expand Down
10 changes: 10 additions & 0 deletions src/lib/viewers/media/VideoBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ class VideoBaseViewer extends MediaBaseViewer {
onKeydown(key) {
return super.onKeydown(key);
}

/**
* Auto-play was prevented, try muted play
*
* @override
*/
handleAutoplayFail = () => {
this.setVolume(0);
this.play().catch(this.pause);
};
}

export default VideoBaseViewer;
8 changes: 8 additions & 0 deletions src/lib/viewers/media/__tests__/MP3Viewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,12 @@ describe('lib/viewers/media/MP3Viewer', () => {
mp3.loadUI();
});
});

describe('handleAutoplayFail()', () => {
it('should call pause', () => {
sandbox.stub(mp3, 'pause');
mp3.handleAutoplayFail();
expect(mp3.pause).to.be.called;
});
});
});
30 changes: 15 additions & 15 deletions src/lib/viewers/media/__tests__/MediaBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,32 +272,32 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
});

describe('autoplay()', () => {
beforeEach(() => {
media.mediaEl = {
play: sandbox.stub().returns(Promise.resolve()),
};
const PLAY_PROMISE_NOT_SUPPORTED = 'play_promise_not_supported';

sandbox.stub(media, 'isAutoplayEnabled').returns(true);
sandbox.stub(Browser, 'isIOS').returns(false);
beforeEach(() => {
media.mediaEl = {};
media.play = sandbox.stub().returns(Promise.resolve());
});

it('should set autoplay if setting is enabled and handle the promise if it is a valid promise', () => {
media.autoplay();
expect(media.mediaEl.play).to.be.called;
expect(media.play).to.be.called;
expect(media.mediaEl.autoplay).to.be.undefined;
});

it('should set autoplay to true if play does not return a promise', () => {
media.mediaEl.play.returns(undefined);
media.autoplay();
expect(media.mediaEl.autoplay).to.be.true;
it('should set autoplay to true if mediaEl.play does not return a promise', done => {
media.play.returns(Promise.reject(new Error(PLAY_PROMISE_NOT_SUPPORTED)));
media.autoplay().then(() => {
expect(media.mediaEl.autoplay).to.be.true;
done();
});
});

it('should muted autoplay if the promise is rejected', done => {
sandbox.stub(media, 'setVolume');
media.mediaEl.play = sandbox.stub().returns(Promise.reject());
it('should call handleAutoplayFail if the promise is rejected', done => {
sandbox.stub(media, 'handleAutoplayFail');
media.play.returns(Promise.reject(new Error('NotAllowedError')));
media.autoplay().then(() => {
expect(media.setVolume).to.be.calledWith(0);
expect(media.handleAutoplayFail).to.be.called;
done();
});
});
Expand Down
12 changes: 12 additions & 0 deletions src/lib/viewers/media/__tests__/VideoBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,16 @@ describe('lib/viewers/media/VideoBaseViewer', () => {
expect(videoBase.rootEl.classList.contains('bp-dark')).to.be.true;
});
});

describe('handleAutoplayFail()', () => {
it('should mute and play again', () => {
sandbox.stub(videoBase, 'setVolume');
videoBase.play = sandbox.stub().returns(Promise.reject());

videoBase.handleAutoplayFail();

expect(videoBase.setVolume).to.be.calledWith(0);
expect(videoBase.play).to.be.called;
});
});
});