Skip to content

Commit

Permalink
addSourceElement should return boolean too, update tests
Browse files Browse the repository at this point in the history
  • Loading branch information
alex-barstow committed Oct 8, 2024
1 parent 633aed8 commit b81d154
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 11 deletions.
15 changes: 11 additions & 4 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -3718,11 +3718,16 @@ class Player extends Component {
*
* @param {string} [mimeType]
* The MIME type of the video source. Optional but recommended.
*
* @return {boolean}
* Returns true if the source element was successfully added, false otherwise.
*/
addSourceElement(srcUrl, mimeType) {
if (this.tech_) {
this.tech_.addSourceElement(srcUrl, mimeType);
if (!this.tech_) {
return false;
}

return this.tech_.addSourceElement(srcUrl, mimeType);
}

/**
Expand All @@ -3735,9 +3740,11 @@ class Player extends Component {
* Returns true if the source element was successfully removed, false otherwise.
*/
removeSourceElement(srcUrl) {
if (this.tech_) {
return this.tech_.removeSourceElement(srcUrl);
if (!this.tech_) {
return false;
}

return this.tech_.removeSourceElement(srcUrl);
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -791,11 +791,14 @@ class Html5 extends Tech {
*
* @param {string} [mimeType]
* The MIME type of the video source. Optional but recommended.
*
* @return {boolean}
* Returns true if the source element was successfully added, false otherwise.
*/
addSourceElement(srcUrl, mimeType) {
if (!srcUrl) {
log.error('Invalid source URL.');
return;
return false;
}

const sourceAttributes = { src: srcUrl };
Expand All @@ -807,6 +810,8 @@ class Html5 extends Tech {
const sourceElement = Dom.createEl('source', {}, sourceAttributes);

this.el_.appendChild(sourceElement);

return true;
}

/**
Expand Down
26 changes: 26 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3626,6 +3626,19 @@ QUnit.test('addSourceElement calls tech method with correct args', function(asse
player.dispose();
});

QUnit.test('addSourceElement returns false if no tech', function(assert) {
const player = TestHelpers.makePlayer();
const srcUrl = 'http://example.com/video.mp4';
const mimeType = 'video/mp4';

player.tech_ = undefined;

const added = player.addSourceElement(srcUrl, mimeType);

assert.notOk(added, 'Returned false');
player.dispose();
});

QUnit.test('removeSourceElement calls tech method with correct args', function(assert) {
const player = TestHelpers.makePlayer();
const removeSourceElementSpy = sinon.spy(player.tech_, 'removeSourceElement');
Expand All @@ -3639,3 +3652,16 @@ QUnit.test('removeSourceElement calls tech method with correct args', function(a
removeSourceElementSpy.restore();
player.dispose();
});

QUnit.test('removeSourceElement returns false if no tech', function(assert) {
const player = TestHelpers.makePlayer();
const srcUrl = 'http://example.com/video.mp4';
const mimeType = 'video/mp4';

player.tech_ = undefined;

const removed = player.removeSourceElement(srcUrl, mimeType);

assert.notOk(removed, 'Returned false');
player.dispose();
});
12 changes: 6 additions & 6 deletions test/unit/tech/html5.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -914,10 +914,10 @@ QUnit.test('addSourceElement adds a valid source element with srcUrl and mimeTyp
const srcUrl = 'http://example.com/video.mp4';
const mimeType = 'video/mp4';

tech.addSourceElement(srcUrl, mimeType);

const added = tech.addSourceElement(srcUrl, mimeType);
const sourceElement = videoEl.querySelector('source');

assert.ok(added, 'Returned true');
assert.ok(sourceElement, 'A source element was added');
assert.equal(sourceElement.src, srcUrl, 'Source element has correct src');
assert.equal(sourceElement.type, mimeType, 'Source element has correct type');
Expand All @@ -930,10 +930,10 @@ QUnit.test('addSourceElement adds a valid source element without a mimeType', fu

const srcUrl = 'http://example.com/video2.mp4';

tech.addSourceElement(srcUrl);

const added = tech.addSourceElement(srcUrl);
const sourceElement = videoEl.querySelector('source');

assert.ok(added, 'Returned true');
assert.ok(sourceElement, 'A source element was added even without a type');
assert.equal(sourceElement.src, srcUrl, 'Source element has correct src');
assert.notOk(sourceElement.type, 'Source element does not have a type attribute');
Expand All @@ -944,10 +944,10 @@ QUnit.test('addSourceElement does not add a source element for invalid source UR

tech.el_ = videoEl;

tech.addSourceElement('');

const added = tech.addSourceElement('');
const sourceElement = videoEl.querySelector('source');

assert.notOk(added, 'Returned false');
assert.notOk(sourceElement, 'No source element was added for missing src');
});

Expand Down

0 comments on commit b81d154

Please sign in to comment.