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);
IEatBagels
12.6k3 gold badges48 silver badges99 bronze badges
asked Oct 9, 2015 at 14:57
\$\endgroup\$

1 Answer 1

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();
answered Oct 10, 2015 at 9:59
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.