Last modified: 2011-03-13 18:04:34 UTC
wikibits.js currently does if (clientPC.indexOf('opera') != -1) { var is_opera = true; var is_opera_preseven = (window.opera && !document.childNodes); var is_opera_seven = (window.opera && document.childNodes); var is_opera_95 = (clientPC.search(/opera\/(9.[5-9]|[1-9][0-9])/)!=-1); } Could this be changed to var is_opera = (clientPC.indexOf('opera') != -1); var is_opera_preseven = is_opera && (window.opera && !document.childNodes); var is_opera_seven = is_opera && (window.opera && document.childNodes); var is_opera_95 = is_opera && (clientPC.search(/opera\/(9.[5-9]|[1-9][0-9])/)!=-1); This ensures that these variable are indeed defined. Also, could we have additional variables is_IE and is_IE_pre_7? I currently use var is_IE = !!window.ActiveXObject; var is_IE_pre_7 = is_IE && (function(agent) { var version = new RegExp('MSIE ([\\d.]+)').exec(agent); return version ? (parseFloat(version[1]) < 7) : false; })(navigator.userAgent); (This is tested and working, see [[:commons:MediaWiki:Tooltips.js]].)
Just noticed bug 12766 (adding IE detection; closed by Tim as WONTFIX)...
We probably need a generic object BrowserDetection, and then do BrowserDetection.isIE() etc.
Well, if you want to wrap it in an object BrowserDetection, then I'd rather have one function BrowserDetection.getBrowser () returning "ie", "gecko", "opera", etc (or numeric "constants" defined for these, such as BrowserDetection.IE, BrowserDetection.GECKO, etc.), plus an additional function BrowserDetection.getVersion () returning the browser version as a float (5.5, 2.0, 7.0 etc) or null if unknown. Cache the results of the browser detection inside the BrowserDetection object to avoid having to re-evaluate it each an every time one of these functions is called.
As much as possible, "browser detection" of this sort should be taken out and shot. When it's necessary to check browser capabilities to perform some conditional operations, you should check *only the specific browser capabilities* relevant. More 'general' browser detection should be a last resort only, if there is *no way whatsoever* to detect the specific feature needed or bug to work around.
Sure, we know that. But as you say, sometimes it still can't be helped, because there is no capability to test for, or the test would be exceedingly expensive. Having variables is_IE etc. for these cases is useful, and it wouldn't need to be reinvented. The problem with the BrowserDetection object proposed in comment 2 is just that it'd break many existing scripts if the existing is_gecko, is_opera, etc. variables were removed. var is_IE = !!window.attachEvent; would be even better.
That would be obviously wrong, as other browsers may well choose to implement window.attachEvent for IE compatibility. You should *only* check for window.attachEvent if you want to use window.attachEvent; not for general "am I running IE?" checks. I strongly recommend removing these generic checks where possible. Scripts incorrectly using them should be corrected to do proper checks.
