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

feat: Add methods to add and remove <source> elements #8886

Merged
merged 5 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -3710,6 +3710,43 @@ class Player extends Component {
return false;
}

/**
* Add a <source> element to the <video> element.
*
* @param {string} srcUrl
* The URL of the video source.
*
* @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_) {
return false;
}

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

/**
* Remove a <source> element from the <video> element by its URL.
*
* @param {string} srcUrl
* The URL of the source to remove.
*
* @return {boolean}
* Returns true if the source element was successfully removed, false otherwise.
*/
removeSourceElement(srcUrl) {
if (!this.tech_) {
return false;
}

return this.tech_.removeSourceElement(srcUrl);
}

/**
* Begin loading the src data.
*/
Expand Down
61 changes: 61 additions & 0 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,67 @@
this.setSrc(src);
}

/**
* Add a <source> element to the <video> element.
*
* @param {string} srcUrl
* The URL of the video source.
*
* @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 false;
}

const sourceAttributes = { src: srcUrl };

if (mimeType) {
sourceAttributes.type = mimeType;
}

const sourceElement = Dom.createEl('source', {}, sourceAttributes);

this.el_.appendChild(sourceElement);

return true;
}

/**
* Remove a <source> element from the <video> element by its URL.
*
* @param {string} srcUrl
* The URL of the source to remove.
*
* @return {boolean}
* Returns true if the source element was successfully removed, false otherwise.
*/
removeSourceElement(srcUrl) {
if (!srcUrl) {
log.error('Source URL is required to remove the source element.');
return false;

Check warning on line 829 in src/js/tech/html5.js

View check run for this annotation

Codecov / codecov/patch

src/js/tech/html5.js#L828-L829

Added lines #L828 - L829 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this function returns a boolean but addSourceElement does not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, both probably should. The reason I didn't have addSourceElement() return a boolean is because originally I hadn't added that early return so a source element would always be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

}

const sourceElements = this.el_.querySelectorAll('source');

for (const sourceElement of sourceElements) {
if (sourceElement.src === srcUrl) {
this.el_.removeChild(sourceElement);

return true;
}
}

log.warn(`No matching source element found with src: ${srcUrl}`);

return false;
}

/**
* Reset the tech by removing all sources and then calling
* {@link Html5.resetMediaElement}.
Expand Down
54 changes: 54 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3611,3 +3611,57 @@ QUnit.test('smooth seeking set to true should update the display time components
player.dispose();
});

QUnit.test('addSourceElement calls tech method with correct args', function(assert) {
const player = TestHelpers.makePlayer();
const addSourceElementSpy = sinon.spy(player.tech_, 'addSourceElement');
const srcUrl = 'http://example.com/video.mp4';
const mimeType = 'video/mp4';

player.addSourceElement(srcUrl, mimeType);

assert.ok(addSourceElementSpy.calledOnce, 'addSourceElement method called');
assert.ok(addSourceElementSpy.calledWith(srcUrl, mimeType), 'addSourceElement called with correct arguments');

addSourceElementSpy.restore();
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');
const srcUrl = 'http://example.com/video.mp4';

player.removeSourceElement(srcUrl);

assert.ok(removeSourceElementSpy.calledOnce, 'removeSourceElement method called');
assert.ok(removeSourceElementSpy.calledWith(srcUrl), 'removeSourceElement called with correct arguments');

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();
});
76 changes: 76 additions & 0 deletions test/unit/tech/html5.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -905,3 +905,79 @@ QUnit.test('supportsFullScreen is always with `webkitEnterFullScreen`', function

tech.el_ = oldEl;
});

QUnit.test('addSourceElement adds a valid source element with srcUrl and mimeType', function(assert) {
const videoEl = document.createElement('video');

tech.el_ = videoEl;

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

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');
});

QUnit.test('addSourceElement adds a valid source element without a mimeType', function(assert) {
const videoEl = document.createElement('video');

tech.el_ = videoEl;

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

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');
});

QUnit.test('addSourceElement does not add a source element for invalid source URL', function(assert) {
const videoEl = document.createElement('video');

tech.el_ = videoEl;

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');
});

QUnit.test('removeSourceElement removes a source element by its URL', function(assert) {
const videoEl = document.createElement('video');

tech.el_ = videoEl;

tech.addSourceElement('http://example.com/video1.mp4');
tech.addSourceElement('http://example.com/video2.mp4');

let sourceElement = videoEl.querySelector('source[src="http://example.com/video1.mp4"]');

assert.ok(sourceElement, 'Source element for video1.mp4 was added');

const removed = tech.removeSourceElement('http://example.com/video1.mp4');

assert.ok(removed, 'Source element was successfully removed');
sourceElement = videoEl.querySelector('source[src="http://example.com/video1.mp4"]');
assert.notOk(sourceElement, 'Source element for video1.mp4 was removed');
});

QUnit.test('removeSourceElement does not remove a source element if URL does not match', function(assert) {
const videoEl = document.createElement('video');

tech.el_ = videoEl;

tech.addSourceElement('http://example.com/video.mp4');

const removed = tech.removeSourceElement('http://example.com/invalid.mp4');

assert.notOk(removed, 'No source element was removed for non-matching URL');
});
2 changes: 2 additions & 0 deletions test/unit/tech/tech-faker.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ class TechFaker extends Tech {
}
return 'movie.mp4';
}
addSourceElement() {}
removeSourceElement() {}
load() {
}
currentSrc() {
Expand Down