Skip to content

Commit

Permalink
fix(viewer): Pause audio instead of muting it if autoplay is prevented (
Browse files Browse the repository at this point in the history
#1068)

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

* fix(viewer): clean up

* fix(viewer): small change

* fix(viewer): avoid duplicating autoplay logic

* fix(viewer): fix typo

* perf(sidebar): force play to always return a promise

* perf(sidebar): remove await async

* perf(sidebar): minor changes
  • Loading branch information
Mingze authored and mergify[bot] committed Sep 17, 2019
1 parent 83424d0 commit 09b22cf
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 41 deletions.
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}
*/
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();
})
.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}
*/
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;
});
});
});

0 comments on commit 09b22cf

Please sign in to comment.