3
\$\begingroup\$

I'll be honest this isn't all my code, I have made some tweaks to bring it more up to date. The main goal is to do some browser detection and add classes to whichever element we want. I am simply looking for your input regarding the code.

// Detect browser and add appropriate classes to calling element, typically 'html'
// Usage: $('html').add_browser_classes();
// Test function: alert('html classes: ' + jQuery('html').attr('class'));

(function($) {
    $.fn.add_browser_classes = function() {

        // Begin with the negation classes

        $(this).addClass('javascript not-webkit not-firefox not-opera not-ie');

        if (/Chrome/.test(navigator.userAgent) && /Google Inc/.test(navigator.vendor)) {
            $(this).removeClass('not-webkit').addClass('webkit chrome');
        } else if (/Safari/.test(navigator.userAgent) && /Apple Computer/.test(navigator.vendor)) {
            $(this).removeClass('not-webkit').addClass('webkit safari');
        } else if (/Firefox/.test(navigator.userAgent)) {
            $(this).removeClass('not-firefox').addClass('firefox');
        } else if (window.opera && window.opera.buildNumber) {
            $(this).removeClass('not-opera').addClass('opera');
        } else if (Function('/*@cc_on return /^10/.test(@_jscript_version) @*/')()) {
            $(this).removeClass('not-ie').addClass('ie ie10');
        } else if (!document.querySelector) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie7 lt-ie11 lt-ie10 lt-ie9 lt-ie8');
        } else if (!document.addEventListener) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie8 lt-ie11 lt-ie10 lt-ie9');
        } else if (!window.atob) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie9 lt-ie11 lt-ie10');
        } else if (!document.__proto__) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie10 lt-ie11');
        } else if (!!(navigator.userAgent.match(/Trident/) && !navigator.userAgent.match(/MSIE/))){                                  
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie11');                                           
        }
        //The last condition checks to see if the browser is IE 11 or greater
        //If we need to test for specific versions of IE 11 and above replace the above else if conditional to this -
        //!!(navigator.userAgent.match(/Trident/) && navigator.userAgent.match(/rv[ :]11/))
        //if it isnt obvious change the 11 in the conditional to the version you are looking for
        return this;
    };
})(jQuery);
\$\endgroup\$
1
\$\begingroup\$

First problem you'll face is that IE, Opera, Firefox, Chrome and Safari aren't the only browsers in town. There are a lot of other browsers! Even the Steam app is just a browser. Anything that uses a form of WebView, or embedded browser (like game client patcher news sections) is also a browser. That equates to a lot of UA strings to account for.

The next problem is the most common problem with this approach: UA strings can be spoofed. Making it worse, browsers up to this day still spoof their UA to pretend like some other browser. For instance, Chrome 45 has the "Safari" and "AppleWebKit". With your code up there, you can easily mistake it for Safari or some other WebKit browser, even when Chrome 45 isn't Safari nor WebKit.

Now if you do want to do it in this approach and not mind the problem of playing catch-up with the latest browsers, then your first problem would be the use of jQuery. You seem to use it just to add and remove classes. Why not construct your determinations in an array, then just form a string afterwards?

var determination = [];

if(chrome) determination.push('isChrome');
if(webkit) determination.push('isWebkit');

var classes = determination.join(' ');

Another thing I notice is that you add your classes on the element you're attaching the plugin to. This also means your script runs every time you attach it, which is unnecessary. Why not run this script just once, during page load? For CSS purposes, you can attach your determinations as a class of <html> and prefix your styles with the class. If you use it for conditional scripts, you can store your determinations in a global variable and use indexOf.

// Put classes to <html>
document.documentElement.className = document.documentElement.className + ' ' + classes;

// Hide all inputs on all non-ie browsers
.not-ie input{
  display: none;
}

// Checking if IE with JS
if(~determinations.indexOf('lt-ie11')) alert('Y U NO USE NEW BROWSER???');

Now for your tests. As mentioned earlier, you might want to be more specific on your tests so that you won't be confusing a browser with another. Also, your tests are tied to each other. I don't know where the IE tests start and end. Everything's all over the place. Try breaking them apart into "plugins" so that you can easily just add in new tests.

BrowserTester.addTest('IE test', function(ua){
  // Add to this array all the classes you want to be put for this browser
  var classes = [];

  // do comparisons to check for IE

  // Return a string of classes for this test.
  return classes.join(' ');

});

// Do the same for other browsers
BrowserTester.addTest('Chrome', function(ua){...});
BrowserTester.addTest('Safari5', function(ua){...});
BrowserTester.addTest('Safari6', function(ua){...});

// Run tests. Add classes to <html>, expose an API to check which browser.
BrowserTester.run();
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.