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

Jump to bottom

Augment userAgent detection #470

Closed
wants to merge 4 commits into from
Closed

Augment userAgent detection #470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
003d017
Augment userAgent detection
gkatsev Apr 26, 2013
0628c30
update android version. Add vjs.IS_OLD_ANDROID. update canPlayType fix
gkatsev May 8, 2013
0d39a0c
Merge branch 'master' into fix/userAgentDetection
gkatsev Jun 24, 2013
a1ce91f
Remove major and minor versions from vjs object since there isn't a c…
gkatsev Jun 24, 2013
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
36 changes: 27 additions & 9 deletions src/js/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,26 +304,44 @@ vjs.USER_AGENT = navigator.userAgent;
* @type {Boolean}
* @constant
*/
vjs.IS_IPHONE = !!vjs.USER_AGENT.match(/iPhone/i);
vjs.IS_IPAD = !!vjs.USER_AGENT.match(/iPad/i);
vjs.IS_IPOD = !!vjs.USER_AGENT.match(/iPod/i);
vjs.IS_IPHONE = (/iPhone/i).test(vjs.USER_AGENT);
vjs.IS_IPAD = (/iPad/i).test(vjs.USER_AGENT);
vjs.IS_IPOD = (/iPod/i).test(vjs.USER_AGENT);
vjs.IS_IOS = vjs.IS_IPHONE || vjs.IS_IPAD || vjs.IS_IPOD;

vjs.IOS_VERSION = (function(){
var match = vjs.USER_AGENT.match(/OS (\d+)_/i);
if (match && match[1]) { return match[1]; }
})();

vjs.IS_ANDROID = !!vjs.USER_AGENT.match(/Android.*AppleWebKit/i);
vjs.IS_ANDROID = (/Android/i).test(vjs.USER_AGENT);
Copy link
Member

Choose a reason for hiding this comment

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

This is the only one that worries me, because we currently use the webkit check to make sure we only override the canPlayType function in the default android browser. Changing this would match Firefox too. We just need a solution that takes that into consideration.
https://github.com/videojs/video.js/blob/master/src/js/media/html5.js#L231

Copy link
Member Author

Choose a reason for hiding this comment

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

That if statement also checks the version of android. Firefox does not report the version of android; only Chrome for Android and the Native Android browser do. So, we could add a check there to make sure the version isn't null, since null < 3 is true.

Copy link
Member Author

Choose a reason for hiding this comment

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

By the way, why are we check if the version is less than 3 if it is only broken on 2.2 and lower?

Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming it's still broken in 2.3 and fixed in 3.

On May 6, 2013, at 3:59 PM, Gary Katsevman [email protected] wrote:

In src/js/lib.js:

@@ -314,7 +314,7 @@ vjs.IOS_VERSION = (function(){
if (match && match[1]) { return match[1]; }
})();

-vjs.IS_ANDROID = !!vjs.USER_AGENT.match(/Android.*AppleWebKit/i);
By the way, why are we check if the version is less than 3 if it is only broken on 2.2 and lower?

—
Reply to this email directly or view it on GitHub.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, just the comment and code are in disagreement. I can do a quick check and see anyway.
Do you think that checking for null there is a good compromise?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it does say 2.2. Definitely worth checking then.

Checking for null how?

Copy link
Member Author

Choose a reason for hiding this comment

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

in media/html5.js, do a null check before checking the version, since firefox doesn't report report android version.

Copy link
Member

Choose a reason for hiding this comment

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

Oh that's interesting. I didn't realize Firefox didn't report Android version. Is that true for other browsers like opera mobile too?

Copy link
Member Author

Choose a reason for hiding this comment

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

According to http://www.useragentstring.com/pages/Opera Mobile/, some versions do report the android version and some don't. I'll do a quick check on that as well.
Checking for webkit in the long run, isn't a good idea, since chrome is moving to Blink at some point. We might want to have a separate check for webkit or something else for older androids.

vjs.ANDROID_VERSION = (function() {
var match = vjs.USER_AGENT.match(/Android (\d+)\./i);
if (match && match[1]) {
return match[1];
// This matches Android Major.Minor.Patch versions
// ANDROID_VERSION is Major.Minor as a Number, if Minor isn't available, then only Major is returned
var match = vjs.USER_AGENT.match(/Android (\d+)(?:\.(\d+))?(?:\.(\d+))*/i),
major,
minor;

if (!match) {
return null;
}

major = match[1] && parseFloat(match[1]);
minor = match[2] && parseFloat(match[2]);

if (major && minor) {
return parseFloat(match[1] + '.' + match[2]);
} else if (major) {
return major;
} else {
return null;
}
return null;
})();
// Old Android is defined as Version older than 2.3, and requiring a webkit version of the android browser
vjs.IS_OLD_ANDROID = vjs.IS_ANDROID && (/webkit/i).test(vjs.USER_AGENT) && vjs.ANDROID_VERSION < 2.3;

vjs.IS_FIREFOX = function(){ return !!vjs.USER_AGENT.match('Firefox'); };
vjs.IS_FIREFOX = (/Firefox/i).test(vjs.USER_AGENT);
vjs.IS_CHROME = (/Chrome/i).test(vjs.USER_AGENT);


/**
Expand Down
12 changes: 4 additions & 8 deletions src/js/media/html5.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,9 @@ vjs.Html5.Events = 'loadstart,suspend,abort,error,emptied,stalled,loadedmetadata

// HTML5 Feature detection and Device Fixes --------------------------------- //

// Android
if (vjs.IS_ANDROID) {

// Override Android 2.2 and less canPlayType method which is broken
if (vjs.ANDROID_VERSION < 3) {
document.createElement('video').constructor.prototype.canPlayType = function(type){
return (type && type.toLowerCase().indexOf('video/mp4') != -1) ? 'maybe' : '';
};
}
if (vjs.IS_OLD_ANDROID) {
document.createElement('video').constructor.prototype.canPlayType = function(type){
return (type && type.toLowerCase().indexOf('video/mp4') != -1) ? 'maybe' : '';
};
}