Skip to content

Commit

Permalink
Fix: Revert media duration setting to on loadeddata (#759)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremy Press committed Apr 5, 2018
1 parent d34835c commit df798d1
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 25 deletions.
1 change: 1 addition & 0 deletions functional-tests/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ exports.SELECTOR_BOX_PREVIEW_LOGO = '.bp-default-logo';
exports.CLASS_BOX_PREVIEW_LOADING_WRAPPER = '.bp-loading-wrapper';
exports.SELECTOR_DOC_CURRENT_PAGE = '.bp-current-page';
exports.SELECTOR_MEDIA_TIMESTAMP = '.bp-media-controls-timecode';
exports.SELECTOR_MEDIA_DURATION = '.bp-media-controls-duration';
exports.SELECTOR_BOX_PREVIEW = '.bp';
exports.SELECTOR_BOX_PREVIEW_DOC = '.bp-doc';
exports.SELECTOR_BOX_PREVIEW_MP3 = '.bp-media-mp3';
Expand Down
4 changes: 1 addition & 3 deletions functional-tests/fileOptions_test.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
const {
SELECTOR_BOX_PREVIEW_LOADED,
SELECTOR_MEDIA_TIMESTAMP,
SELECTOR_DOC_CURRENT_PAGE,
SELECTOR_BOX_PREVIEW_LOGO,
CLASS_BOX_PREVIEW_LOADING_WRAPPER,
SELECTOR_BOX_PREVIEW_DOC,
SELECTOR_BOX_PREVIEW_MP3,
SELECTOR_BOX_PREVIEW_DASH,
SELECTOR_BOX_PREVIEW_MP4,
SELECTOR_BOX_PREVIEW_ERROR
SELECTOR_BOX_PREVIEW_MP4
} = require('./constants');

const { navigateToNextItem, makeNavAppear, navigateToPrevItem, waitForLoad } = require('./helpers');
Expand Down
45 changes: 45 additions & 0 deletions functional-tests/media-controls.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<!DOCTYPE html>
<html>

<head>
<meta charset="UTF-8">
<style>
.preview-container {
border: 1px solid #eee;
height: 490px;
width: 100%;
}
</style>
<link rel="stylesheet" type="text/css" href="../dist/dev/en-US/preview.css">
<script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=Promise"></script>
<script src="../dist/dev/en-US/preview.js"></script>

</head>

<body>
<div class="preview-container"></div>
<script>
var ACCESS_TOKEN = 'edRiUpeLD9A1nLyWqc1gZOt2OnSNNYp9';

var FILE_ID_VIDEO = '286114565199';
var FILE_ID_MP3 = '286509191779';

var preview = new Box.Preview();
preview.show(FILE_ID_VIDEO, ACCESS_TOKEN, {
container: '.preview-container',
showDownload: false,
collection: [FILE_ID_VIDEO, FILE_ID_MP3],
});

function disableDash() {
preview.disableViewers('Dash');

preview.show(FILE_ID_VIDEO, ACCESS_TOKEN, {
container: '.preview-container',
showDownload: false,
});
}
</script>
</body>

</html>
65 changes: 65 additions & 0 deletions functional-tests/media_controls_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const {
SELECTOR_MEDIA_TIMESTAMP,
SELECTOR_MEDIA_DURATION,
SELECTOR_BOX_PREVIEW_LOGO,
CLASS_BOX_PREVIEW_LOADING_WRAPPER,
SELECTOR_BOX_PREVIEW_MP3,
SELECTOR_BOX_PREVIEW_DASH,
SELECTOR_BOX_PREVIEW_MP4
} = require('./constants');

const { navigateToNextItem, makeNavAppear, waitForLoad } = require('./helpers');

const { CI } = process.env;
const DEFAULT_START = '0:00';
const VIDEO_DURATION = '3:52';
const AUDIO_DURATION = '7:47';
const SELECTOR_VIDEO = 'video';

Feature('Media Controls', { retries: CI ? 3 : 0 });

Before((I) => {
I.amOnPage('/functional-tests/media-controls.html');
I.waitForElement(SELECTOR_BOX_PREVIEW_LOGO);
waitForLoad(I);
I.waitForElement(SELECTOR_BOX_PREVIEW_DASH);
});

// Exclude IE as it can't handle media files with saucelabs
Scenario(
'Check that the media controls show the correct time current/total times @ci @chrome @firefox @edge @safari @android @ios',
{ retries: 3 },
(I) => {
// video (dash)
waitForLoad(I);
I.waitForElement(SELECTOR_BOX_PREVIEW_DASH);
makeNavAppear(I, SELECTOR_VIDEO);
I.waitForVisible(SELECTOR_MEDIA_TIMESTAMP);
I.seeTextEquals(DEFAULT_START, SELECTOR_MEDIA_TIMESTAMP);
I.seeTextEquals(VIDEO_DURATION, SELECTOR_MEDIA_DURATION);
navigateToNextItem(I);

// mp3
waitForLoad(I);
I.waitForElement(SELECTOR_BOX_PREVIEW_MP3);
makeNavAppear(I);
I.waitForVisible(SELECTOR_MEDIA_TIMESTAMP);
I.seeTextEquals(DEFAULT_START, SELECTOR_MEDIA_TIMESTAMP);
I.seeTextEquals(AUDIO_DURATION, SELECTOR_MEDIA_DURATION);

// video (mp4)
/* eslint-disable prefer-arrow-callback */
I.executeScript(function() {
window.disableDash();
});
I.waitForElement(CLASS_BOX_PREVIEW_LOADING_WRAPPER);
/* eslint-enable prefer-arrow-callback */
waitForLoad(I);
I.waitForElement(SELECTOR_BOX_PREVIEW_MP4);

makeNavAppear(I, SELECTOR_VIDEO);
I.waitForVisible(SELECTOR_MEDIA_TIMESTAMP);
I.seeTextEquals(DEFAULT_START, SELECTOR_MEDIA_TIMESTAMP);
I.seeTextEquals(VIDEO_DURATION, SELECTOR_MEDIA_DURATION);
}
);
18 changes: 12 additions & 6 deletions src/lib/viewers/media/DashViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,6 @@ class DashViewer extends VideoBaseViewer {
*/
shakaManifestLoadedHandler() {
this.calculateVideoDimensions();
this.loadUI();

if (this.hdVideoId !== -1) {
this.mediaControls.enableHDSettings();
}

this.loadSubtitles();
this.loadAlternateAudio();
}
Expand Down Expand Up @@ -503,6 +497,7 @@ class DashViewer extends VideoBaseViewer {
this.autoplay();
}

this.loadUI();
this.loadFilmStrip();
this.resize();
this.handleVolume();
Expand All @@ -518,6 +513,17 @@ class DashViewer extends VideoBaseViewer {
this.mediaContainerEl.focus();
}

/**
* @inheritdoc
*/
loadUI() {
super.loadUI();

if (this.hdVideoId !== -1) {
this.mediaControls.enableHDSettings();
}
}

/**
* Loads the film strip
*
Expand Down
50 changes: 34 additions & 16 deletions src/lib/viewers/media/__tests__/DashViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,6 @@ describe('lib/viewers/media/DashViewer', () => {
describe('shakaManifestLoadedHandler()', () => {
beforeEach(() => {
sandbox.stub(dash, 'calculateVideoDimensions');
sandbox.stub(dash, 'loadUI');
sandbox.stub(dash, 'loadSubtitles');
sandbox.stub(dash, 'loadAlternateAudio');
});
Expand All @@ -601,18 +600,6 @@ describe('lib/viewers/media/DashViewer', () => {
dash.shakaManifestLoadedHandler();

expect(dash.calculateVideoDimensions).to.be.called;
expect(dash.loadUI).to.be.called;
});

it('should enable HD settings if there is an HD rep', () => {
dash.hdVideoId = -1;
stubs.mockControls.expects('enableHDSettings').never();
dash.shakaManifestLoadedHandler();

dash.hdVideoId = 1;
stubs.mockControls.expects('enableHDSettings');
dash.shakaManifestLoadedHandler();

});

it('should load subtitles and additional audio tracks', () => {
Expand Down Expand Up @@ -657,6 +644,7 @@ describe('lib/viewers/media/DashViewer', () => {
sandbox.stub(dash, 'isAutoplayEnabled').returns(true);
sandbox.stub(dash, 'autoplay');
sandbox.stub(dash, 'loadFilmStrip');
sandbox.stub(dash, 'loadUI');
sandbox.stub(dash, 'resize');
sandbox.stub(dash, 'handleVolume');
sandbox.stub(dash, 'startBandwidthTracking');
Expand All @@ -665,6 +653,7 @@ describe('lib/viewers/media/DashViewer', () => {

dash.loadeddataHandler();
expect(dash.autoplay).to.be.called;
expect(dash.loadUI).to.be.called;
expect(dash.showMedia).to.be.called;
expect(dash.showPlayButton).to.be.called;
expect(dash.emit).to.be.calledWith(VIEWER_EVENT.load);
Expand All @@ -674,6 +663,36 @@ describe('lib/viewers/media/DashViewer', () => {
});
});

describe('loadUI()', () => {
beforeEach(() => {
stubs.loadUI = DashViewer.prototype.loadUI;
dash.mediaControls = {
enableHDSettings: sandbox.stub(),
removeListener: sandbox.stub(),
removeAllListeners: sandbox.stub(),
destroy: sandbox.stub()
};

Object.defineProperty(VideoBaseViewer.prototype, 'loadUI', { value: sandbox.mock() });
});

afterEach(() => {
Object.defineProperty(VideoBaseViewer.prototype, 'loadUI', { value: stubs.loadUI });
});

it('should enable HD settings if an HD rep exists', () => {
dash.hdVideoId = 3;
dash.loadUI();
expect(dash.mediaControls.enableHDSettings).to.be.called;
});

it('should do nothing if there is no HD rep', () => {
dash.hdVideoId = -1;
dash.loadUI();
expect(dash.mediaControls.enableHDSettings).to.not.be.called;
});
});

describe('loadFilmStrip()', () => {
beforeEach(() => {
dash.options = {
Expand Down Expand Up @@ -908,7 +927,7 @@ describe('lib/viewers/media/DashViewer', () => {
beforeEach(() => {
sandbox.stub(dash, 'enableAudioId');
});

it('should select correct audio', () => {
dash.audioTracks = [
{ language: 'eng', role: 'audio0' },
Expand All @@ -918,7 +937,7 @@ describe('lib/viewers/media/DashViewer', () => {
sandbox.stub(dash.cache, 'get').returns('1');

dash.handleAudioTrack();
expect(dash.enableAudioId).to.be.calledWith('audio1')
expect(dash.enableAudioId).to.be.calledWith('audio1');
});

it('should not select audio if index out of bounds', () => {
Expand All @@ -931,7 +950,6 @@ describe('lib/viewers/media/DashViewer', () => {

dash.handleAudioTrack();
expect(dash.enableAudioId).to.not.be.called;

});
});

Expand Down

0 comments on commit df798d1

Please sign in to comment.