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.

Sign up for GitHub

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

Fixed type support checking for an empty src string #1797

Closed
wants to merge 1 commit into from
Closed

Fixed type support checking for an empty src string #1797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions src/js/media/html5.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,13 @@ vjs.Html5.nativeSourceHandler = {};
* @return {String} 'probably', 'maybe', or '' (empty string)
*/
vjs.Html5.nativeSourceHandler.canHandleSource = function(source){
var ext;
var match, ext;
Copy link
Member

Choose a reason for hiding this comment

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

I thought the plan was to go with explicit var declarations moving forward, or are instances like this an exception (no actual values set, just defined)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Waiting for @gkatsev's input to change what I'm using in practice.


function canPlayType(type){
// IE9 on Windows 7 without MediaPlayer throws an error here
// https://github.com/videojs/video.js/issues/519
try {
return !!vjs.TEST_VID.canPlayType(type);
return vjs.TEST_VID.canPlayType(type);
Copy link
Member

Choose a reason for hiding this comment

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

Why'd we revert from the !!?

Copy link
Member Author

Choose a reason for hiding this comment

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

When implementing the Flash source handler I decided to go with the maybe/probably/empty-string return values to be more similar to canPlayType. HTML5 needed to be updated to match that too.

} catch(e) {
return '';
}
Expand All @@ -322,11 +322,15 @@ vjs.Html5.nativeSourceHandler.canHandleSource = function(source){
// If a type was provided we should rely on that
if (source.type) {
return canPlayType(source.type);
} else {
} else if (source.src) {
// If no type, fall back to checking 'video/[EXTENSION]'
ext = source.src.match(/\.([^\/\?]+)(\?[^\/]+)?$/i)[1];
match = source.src.match(/\.([^\/\?]+)(\?[^\/]+)?$/i);
ext = match && match[1];

return canPlayType('video/'+ext);
}

return '';
};

/**
Expand Down
26 changes: 26 additions & 0 deletions test/unit/media.html5.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,29 @@ test('error events may not set the errors property', function() {
test('should have the source handler interface', function() {
ok(vjs.Html5.registerSourceHandler, 'has the registerSourceHandler function');
});

test('native source handler canHandleSource', function(){
var result;

// Stub the test video canPlayType (used in canHandleSource) to control results
var origCPT = vjs.TEST_VID.canPlayType;
vjs.TEST_VID.canPlayType = function(type){
if (type === 'video/mp4') {
return 'maybe';
}
return '';
};

var canHandleSource = vjs.Html5.nativeSourceHandler.canHandleSource;

equal(canHandleSource({ type: 'video/mp4', src: 'video.flv' }), 'maybe', 'Native source handler reported type support');
equal(canHandleSource({ src: 'http://www.example.com/video.mp4' }), 'maybe', 'Native source handler reported extension support');
// Test for issue videojs/video.js#1785 and other potential failures
equal(canHandleSource({ src: '' }), '', 'Native source handler handled empty src');
equal(canHandleSource({}), '', 'Native source handler handled empty object');
equal(canHandleSource({ src: 'foo' }), '', 'Native source handler handled bad src');
equal(canHandleSource({ type: 'foo' }), '', 'Native source handler handled bad type');

// Reset test video canPlayType
vjs.TEST_VID.canPlayType = origCPT;
});