r68487 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r68486‎ | r68487 | r68488 >
Date:21:15, 23 June 2010
Author:tparscal
Status:deferred (Comments)
Tags:
Comment:
Fixed bugs and added in browser detection utilities. Also added in the tabIndex counter thing.
Modified paths:
  • /branches/resourceloader/phase3/resources/core/mw.js (modified) (history)

Diff [purge]

Index: branches/resourceloader/phase3/resources/core/mw.js
@@ -1,5 +1,5 @@
22 /**
3 - * Core MediaWiki JavaScript Library
 3+ * JavaScript Backawrds Compatibility
44 */
55
66 // Make calling .indexOf() on an array work on older browsers
@@ -14,6 +14,10 @@
1515 };
1616 }
1717
 18+/**
 19+ * Core MediaWiki JavaScript Library
 20+ */
 21+
1822 // Extend window.mw rather than overriding it. This is a temporary fix designed to prevent
1923 // stuff from blowing up when usability.js (setting mw.usability) is run before this file.
2024 window.mw = $.extend( typeof window.mw === 'undefined' ? {} : window.mw, {
@@ -25,6 +29,8 @@
2630 /* Private Members */
2731
2832 var that = this;
 33+ // Decoded user agent string cache
 34+ var client = null;
2935
3036 /* Public Functions */
3137
@@ -65,16 +71,20 @@
6672 return url;
6773 };
6874 /**
69 - * RFC 3986 compliant URI component encoder
 75+ * RFC 3986 compliant URI component encoder - with identical behavior as PHP's urlencode function. Note: PHP's
 76+ * urlencode function prior to version 5.3 also escapes tildes, this does not. The naming here is not the same
 77+ * as PHP because PHP can't decide out to name things (underscores sometimes?), much less set a reasonable
 78+ * precedence for how things should be named in other environments. We use camelCase and action-subject here.
7079 */
71 - this.urlencode = function( string ) {
72 - return encodeURIComponent( string )
 80+ this.encodeUrlComponent = function( string ) {
 81+ return encodeURIComponent( new String( string ) )
7382 .replace(/!/g, '%21')
7483 .replace(/'/g, '%27')
7584 .replace(/\(/g, '%28')
7685 .replace(/\)/g, '%29')
77 - .replace(/\*/g, '%2A');
78 - }
 86+ .replace(/\*/g, '%2A')
 87+ .replace(/%20/g, '+');
 88+ };
7989 /**
8090 * Builds a query string from an object with key and values
8191 */
@@ -82,12 +92,200 @@
8393 if ( typeof parameters === 'object' ) {
8494 var parts = [];
8595 for ( var p in parameters ) {
86 - parts[parts.length] = that.urlencode( p ) + '=' + that.urlencode( parameters[p] );
 96+ parts[parts.length] = that.encodeUrlComponent( p ) + '=' + that.encodeUrlComponent( parameters[p] );
8797 }
8898 return parts.join( '&' );
8999 }
90100 return '';
91101 };
 102+ /**
 103+ * Returns an object containing information about the browser
 104+ *
 105+ * The resulting client object will be in the following format:
 106+ * {
 107+ * 'name': 'firefox',
 108+ * 'layout': 'gecko',
 109+ * 'os': 'linux'
 110+ * 'version': '3.5.1',
 111+ * 'versionBase': '3',
 112+ * 'versionNumber': 3.5,
 113+ * }
 114+ */
 115+ this.client = function() {
 116+ // Use the cached version if possible
 117+ if ( client === null ) {
 118+
 119+ /* Configuration */
 120+
 121+ // Name of browsers or layout engines we don't recognize
 122+ var uk = 'unknown';
 123+ // Generic version digit
 124+ var x = 'x';
 125+ // Strings found in user agent strings that need to be conformed
 126+ var wildUserAgents = [ 'Opera', 'Navigator', 'Minefield', 'KHTML', 'Chrome', 'PLAYSTATION 3'];
 127+ // Translations for conforming user agent strings
 128+ var userAgentTranslations = [
 129+ // Tons of browsers lie about being something they are not
 130+ [/(Firefox|MSIE|KHTML,\slike\sGecko|Konqueror)/, ''],
 131+ // Chrome lives in the shadow of Safari still
 132+ ['Chrome Safari', 'Chrome'],
 133+ // KHTML is the layout engine not the browser - LIES!
 134+ ['KHTML', 'Konqueror'],
 135+ // Firefox nightly builds
 136+ ['Minefield', 'Firefox'],
 137+ // This helps keep differnt versions consistent
 138+ ['Navigator', 'Netscape'],
 139+ // This prevents version extraction issues, otherwise translation would happen later
 140+ ['PLAYSTATION 3', 'PS3'],
 141+ ];
 142+ // Strings which precede a version number in a user agent string - combined and used as match 1 in
 143+ // version detectection
 144+ var versionPrefixes = [
 145+ 'camino', 'chrome', 'firefox', 'netscape', 'netscape6', 'opera', 'version', 'konqueror', 'lynx',
 146+ 'msie', 'safari', 'ps3'
 147+ ];
 148+ // Used as matches 2, 3 and 4 in version extraction - 3 is used as actual version number
 149+ var versionSuffix = '(\/|\;?\s|)([a-z0-9\.\+]*?)(\;|dev|rel|\\)|\s|$)';
 150+ // Names of known browsers
 151+ var browserNames = [
 152+ 'camino', 'chrome', 'firefox', 'netscape', 'konqueror', 'lynx', 'msie', 'opera', 'safari', 'ipod',
 153+ 'iphone', 'blackberry', 'ps3'
 154+ ];
 155+ // Tanslations for conforming browser names
 156+ var browserTranslations = [];
 157+ // Names of known layout engines
 158+ var layoutNames = ['gecko', 'konqueror', 'msie', 'opera', 'webkit'];
 159+ // Translations for conforming layout names
 160+ var layoutTranslations = [['konqueror', 'khtml'], ['msie', 'trident'], ['opera', 'presto']];
 161+ // Names of known operating systems
 162+ var osNames = ['win', 'mac', 'linux', 'sunos', 'solaris', 'iphone'];
 163+ // Translations for conforming operating system names
 164+ var osTranslations = [['sunos', 'solaris']];
 165+
 166+ /* Fucntions */
 167+
 168+ // Performs multiple replacements on a string
 169+ function translate( source, translations ) {
 170+ for ( var i = 0; i < translations.length; i++ ) {
 171+ source = source.replace( translations[i][0], translations[i][1] );
 172+ }
 173+ return source;
 174+ };
 175+
 176+ /* Pre-processing */
 177+
 178+ var userAgent = navigator.userAgent, match, browser = uk, layout = uk, os = uk, version = x;
 179+ if ( match = new RegExp( '(' + wildUserAgents.join( '|' ) + ')' ).exec( userAgent ) ) {
 180+ // Takes a userAgent string and translates given text into something we can more easily work with
 181+ userAgent = translate( userAgent, userAgentTranslations );
 182+ }
 183+ // Everything will be in lowercase from now on
 184+ userAgent = userAgent.toLowerCase();
 185+
 186+ /* Extraction */
 187+
 188+ if ( match = new RegExp( '(' + browserNames.join( '|' ) + ')' ).exec( userAgent ) ) {
 189+ browser = translate( match[1], browserTranslations );
 190+ }
 191+ if ( match = new RegExp( '(' + layoutNames.join( '|' ) + ')' ).exec( userAgent ) ) {
 192+ layout = translate( match[1], layoutTranslations );
 193+ }
 194+ if ( match = new RegExp( '(' + osNames.join( '|' ) + ')' ).exec( navigator.platform.toLowerCase() ) ) {
 195+ var os = translate( match[1], osTranslations );
 196+ }
 197+ if ( match = new RegExp( '(' + versionPrefixes.join( '|' ) + ')' + versionSuffix ).exec( userAgent ) ) {
 198+ version = match[3];
 199+ }
 200+
 201+ /* Edge Cases -- did I mention about how user agent string lie? */
 202+
 203+ // Decode Safari's crazy 400+ version numbers
 204+ if ( name.match( /safari/ ) && version > 400 ) {
 205+ version = '2.0';
 206+ }
 207+ // Expose Opera 10's lies about being Opera 9.8
 208+ if ( name === 'opera' && version >= 9.8) {
 209+ version = userAgent.match( /version\/([0-9\.]*)/i )[1] || 10;
 210+ }
 211+
 212+ /* Caching */
 213+
 214+ client = {
 215+ 'browser': browser,
 216+ 'layout': layout,
 217+ 'os': os,
 218+ 'version': version,
 219+ 'versionBase': ( version !== x ? new String( version ).substr( 0, 1 ) : x ),
 220+ 'versionNumber': ( parseFloat( version, 10 ) || 0.0 )
 221+ };
 222+ }
 223+ return client;
 224+ };
 225+ /**
 226+ * Checks the current browser against a support map object to determine if the browser has been black-listed or
 227+ * not. If the browser was not configured specifically it is assumed to work. It is assumed that the body
 228+ * element is classified as either "ltr" or "rtl". If neither is set, "ltr" is assumed.
 229+ *
 230+ * A browser map is in the following format:
 231+ * {
 232+ * 'ltr': {
 233+ * // Multiple rules with configurable operators
 234+ * 'msie': [['>=', 7], ['!=', 9]],
 235+ * // Blocked entirely
 236+ * 'iphone': false
 237+ * },
 238+ * 'rtl': {
 239+ * // Test against a string
 240+ * 'msie': [['!==', '8.1.2.3']],
 241+ * // RTL rules do not fall through to LTR rules, you must explicity set each of them
 242+ * 'iphone': false
 243+ * }
 244+ * }
 245+ *
 246+ * @param Object of browser support map
 247+ *
 248+ * @return Boolean true if browser known or assumed to be supported, false if blacklisted
 249+ */
 250+ this.testClient = function( map ) {
 251+ var client = this.client();
 252+ // Check over each browser condition to determine if we are running in a compatible client
 253+ var browser = map[$( 'body' ).is( '.rtl' ) ? 'rtl' : 'ltr'][client.browser];
 254+ if ( typeof browser !== 'object' ) {
 255+ // Unknown, so we assume it's working
 256+ return true;
 257+ }
 258+ for ( var condition in browser ) {
 259+ var op = browser[condition][0];
 260+ var val = browser[condition][1];
 261+ if ( val === false ) {
 262+ return false;
 263+ } else if ( typeof val == 'string' ) {
 264+ if ( !( eval( 'client.version' + op + '"' + val + '"' ) ) ) {
 265+ return false;
 266+ }
 267+ } else if ( typeof val == 'number' ) {
 268+ if ( !( eval( 'client.versionNumber' + op + val ) ) ) {
 269+ return false;
 270+ }
 271+ }
 272+ }
 273+ return true;
 274+ };
 275+ /**
 276+ * Finds the highest tabindex in use.
 277+ *
 278+ * @return Integer of highest tabindex on the page
 279+ */
 280+ this.getMaxTabIndex = function() {
 281+ var maxTI = 0;
 282+ $( '[tabindex]' ).each( function() {
 283+ var ti = parseInt( $(this).attr( 'tabindex' ) );
 284+ if ( ti > maxTI ) {
 285+ maxTI = ti;
 286+ }
 287+ } );
 288+ return maxTI;
 289+ };
92290 } )(),
93291 /**
94292 * Configuration system
@@ -155,13 +353,19 @@
156354 messages[keys] = value;
157355 }
158356 };
159 - this.get = function( key, options ) {
160 - if ( typeof messages[key] === 'undefined' ) {
 357+ this.get = function( key, args ) {
 358+ if ( !( key in messages ) ) {
161359 return '<' + key + '>';
162 - } else {
163 - // TODO: Do something clever with options
164 - return messages[key];
165360 }
 361+ var msg = messages[key];
 362+ if ( typeof args == 'object' || typeof args == 'array' ) {
 363+ for ( var argKey in args ) {
 364+ msg = msg.replace( '\$' + ( parseInt( argKey ) + 1 ), args[argKey] );
 365+ }
 366+ } else if ( typeof args == 'string' || typeof args == 'number' ) {
 367+ msg = msg.replace( '$1', args );
 368+ }
 369+ return msg;
166370 };
167371 } )(),
168372 /**
@@ -204,6 +408,7 @@
205409 // Collect the names of pending modules
206410 var list = [];
207411 for ( var r = 0; r < requirements.length; r++ ) {
 412+ var requirement = requirements[r];
208413 if (
209414 typeof registry[requirements[r]] === 'undefined' ||
210415 typeof registry[requirements[r]].state === 'undefined' ||
@@ -349,10 +554,10 @@
350555 // Attach components
351556 registry[name].script = script;
352557 if ( typeof style === 'string' ) {
353 - implementations[name].style = style;
 558+ registry[name].style = style;
354559 }
355560 if ( typeof localization === 'object' ) {
356 - implementations[name].localization = localization;
 561+ registry[name].localization = localization;
357562 }
358563 // Collect requirements, if any
359564 var requirements = [];

Comments

#Comment by Nikerabbit (talk | contribs)   05:55, 24 June 2010
+ * JavaScript Backawrds Compatibility

Typo

+	var requirement = requirements[r];
 	if (
 		typeof registry[requirements[r]] === 'undefined' ||

Not used?

+	if ( ti > maxTI ) {
+		maxTI = ti;
+	}

maxTI = Math.Max( maxTI, ti ); ?

#Comment by Trevor Parscal (WMF) (talk | contribs)   17:14, 24 June 2010

> Typo

It happens, thanks for spotting it.

> Not used?

What's happening here is, if the module has not been registered (typeof registry[requirements[r]] === 'undefined'), it has been but there's no state information (typeof registry[requirements[r]].state === 'undefined'), or it has been but the state is something other than ready (registry[requirements[r]].state !== 'ready'), we consider it pending. The 2nd check also helps short circuit things before we get to the 3rd check, because if *.state is undefined, then checking the value of *.state would result in an error.

> maxTI = Math.Max( maxTI, ti );

Well, is function overhead + assignment really cheaper than evaluation + assignment? Also, in the existing version, assignment doesn't always happen, in yours it does, so even if calling Math.max indeed is the same speed as evaluating ti > maxTI, your implementation is still slower...

However, yours is much prettier...

Status & tagging log