Skip to content

Commit

Permalink
@chemoish when the src redefines the current source, with a single so…
Browse files Browse the repository at this point in the history
…urce, have the current sources be set to null.

When the `currentSources()` are called after `src('http://example.com')`, expect the `currentSources()` to only return the newly set source.
This way sources, via `<source>` or `src([...])` list, should never be out of sync when redefining a single source.

Updating tests to specify behavior changes. Remove autoplay from fixtures to bypass `loadTech()` ramifications.

# Conflicts:
#	src/js/player.js
#	test/unit/player.js
  • Loading branch information
Carey Hinoki committed Aug 26, 2016
1 parent be8341d commit 5248d86
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 9 deletions.
5 changes: 4 additions & 1 deletion src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ class Player extends Component {
techOptions.startTime = this.cache_.currentTime;
}

this.cache_.sources = null;
this.cache_.source = source;
this.cache_.src = source.src;
}
Expand Down Expand Up @@ -1977,8 +1978,10 @@ class Player extends Component {
// the tech loop to check for a compatible technology
this.sourceList_([source]);
} else {
this.cache_.sources = null;
this.cache_.source = source;
this.cache_.src = source.src;

this.currentType_ = source.type || '';

// wait until the tech is ready to set the source
Expand Down Expand Up @@ -2083,7 +2086,7 @@ class Player extends Component {
* @method currentSource
*/
currentSource() {
return this.cache_.source || {};
return this.cache_.source || { src: this.currentSrc() };
}

/**
Expand Down
34 changes: 26 additions & 8 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ QUnit.test('should get current source from source tag', function(assert) {
const fixture = document.getElementById('qunit-fixture');

const html = [
'<video id="example_1" class="video-js" autoplay preload="none">',
'<video id="example_1" class="video-js" preload="none">',
'<source src="http://google.com" type="video/mp4">',
'<source src="http://hugo.com" type="video/webm">',
'</video>'
Expand All @@ -143,7 +143,7 @@ QUnit.test('should get current sources from source tag', function(assert) {
const fixture = document.getElementById('qunit-fixture');

const html = [
'<video id="example_1" class="video-js" autoplay preload="none">',
'<video id="example_1" class="video-js" preload="none">',
'<source src="http://google.com" type="video/mp4">',
'<source src="http://hugo.com" type="video/webm">',
'</video>'
Expand All @@ -158,20 +158,29 @@ QUnit.test('should get current sources from source tag', function(assert) {
assert.ok(player.currentSources()[0].type === 'video/mp4');
assert.ok(player.currentSources()[1].src === 'http://hugo.com');
assert.ok(player.currentSources()[1].type === 'video/webm');

// when redefining src expect sources to update accordingly
player.src('http://google.com');

assert.ok(player.currentSources()[0].src === 'http://google.com');
assert.ok(player.currentSources()[0].type === undefined);
assert.ok(player.currentSources()[1] === undefined);
});

QUnit.test('should get current source from src set', function(assert) {
const fixture = document.getElementById('qunit-fixture');

const html = '<video id="example_1" class="video-js" autoplay preload="none"></video>';
const html = '<video id="example_1" class="video-js" preload="none"></video>';

fixture.innerHTML += html;

const tag = document.getElementById('example_1');
const player = TestHelpers.makePlayer({}, tag);

// check for empty object
assert.ok(Object.keys(player.currentSource()).length === 0);
player.loadTech_('Html5');

// check for matching undefined src
assert.ok(player.currentSource().src === player.currentSrc());

player.src('http://google.com');

Expand All @@ -197,15 +206,17 @@ QUnit.test('should get current source from src set', function(assert) {
QUnit.test('should get current sources from src set', function(assert) {
const fixture = document.getElementById('qunit-fixture');

const html = '<video id="example_1" class="video-js" autoplay preload="none"></video>';
const html = '<video id="example_1" class="video-js" preload="none"></video>';

fixture.innerHTML += html;

const tag = document.getElementById('example_1');
const player = TestHelpers.makePlayer({}, tag);

// check for empty object
assert.ok(Object.keys(player.currentSources()[0]).length === 0);
player.loadTech_('Html5');

// check for matching undefined src
assert.ok(player.currentSource().src === player.currentSrc());

player.src([{
src: 'http://google.com'
Expand All @@ -230,6 +241,13 @@ QUnit.test('should get current sources from src set', function(assert) {
assert.ok(player.currentSources()[0].type === 'video/mp4');
assert.ok(player.currentSources()[1].src === 'http://hugo.com');
assert.ok(player.currentSources()[1].type === 'video/webm');

// when redefining src expect sources to update accordingly
player.src('http://hugo.com');

assert.ok(player.currentSources()[0].src === 'http://hugo.com');
assert.ok(player.currentSources()[0].type === undefined);
assert.ok(player.currentSources()[1] === undefined);
});

QUnit.test('should asynchronously fire error events during source selection', function(assert) {
Expand Down

0 comments on commit 5248d86

Please sign in to comment.