fix IE version detection in old IE #2653

Merged
merged 1 commit into from Sep 15, 2014

Projects

None yet

3 participants

@SergioCrisostomo
Member

Fixes #2652

@arian arian commented on an outdated diff Sep 15, 2014
Source/Browser/Browser.js
@@ -46,7 +46,7 @@ var parse = function(ua, platform){
var Browser = this.Browser = parse(navigator.userAgent, navigator.platform);
if (Browser.name == 'ie'){
- Browser.version = document.documentMode;
+ Browser.version = document.documentMode/*<ltIE8>*/ || parseFloat(navigator.appVersion.match(/MSIE ([\d.]+)/)[1])/*</ltIE8>*/;
@arian
arian Sep 15, 2014 Member

Doesn't the Browser object at this point already contain the version? And in the case document.documentMode is available, it should that instead.

if (Browser.name == 'ie' && document.documentMode){
    Browser.version = document.documentMode;
}
@SergioCrisostomo
Member

@arian that makes much more sense.
You are right, we had already the value from before and were writing-over with a undefined.

Updated the pull request.

@arian arian commented on an outdated diff Sep 15, 2014
Source/Browser/Browser.js
@@ -45,8 +45,8 @@ var parse = function(ua, platform){
var Browser = this.Browser = parse(navigator.userAgent, navigator.platform);
-if (Browser.name == 'ie'){
- Browser.version = document.documentMode;
+if (Browser.name == 'ie' && document.documentMode){
+ Browser.version = document.documentMode;
@arian
arian Sep 15, 2014 Member

You changed the tab to 4 spaces...

@ibolmo ibolmo added the bug label Sep 15, 2014
@ibolmo ibolmo added this to the 1.5.2 milestone Sep 15, 2014
@ibolmo ibolmo merged commit 09b99e5 into mootools:master Sep 15, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment