r92933 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r92932‎ | r92933 | r92934 >
Date:09:09, 23 July 2011
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
Minor mw.loader fixes
- No functional changes
- Whitespace correction
- Comment fixes
- Combine var statements
- A few [[JSPERF]]ies
- Using mw instead of mediaWiki
- Putting global alias on top, to avoid a ReferenceError in case the immediately invoked "new function(){}" refers to mw during the construction
- "class" -> "constructor"
- caching "typeof dependencies" in mw.loader.using
- fix var "group" collision in mw.loader.work
- move var statements to top of functions
- fixing some occurrences of var statements in for-loops by declaring them before the loop
- Remove space before slash in html-fragement for jQuery html regex in <link /> and <br />
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.util.js
@@ -342,7 +342,7 @@
343343 switch ( mw.config.get( 'skin' ) ) {
344344 case 'standard' :
345345 case 'cologneblue' :
346 - $( '#quickbar' ).append( $link.after( '<br />' ) );
 346+ $( '#quickbar' ).append( $link.after( '<br/>' ) );
347347 return $link[0];
348348 case 'nostalgia' :
349349 $( '#searchform' ).before( $link).before( ' &#124; ' );
Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -2,8 +2,8 @@
33 * Core MediaWiki JavaScript Library
44 */
55
6 -// Attach to window
7 -window.mediaWiki = new ( function( $ ) {
 6+// Attach to window and globally alias
 7+window.mw = window.mediaWiki = new ( function( $ ) {
88
99 /* Private Members */
1010
@@ -21,7 +21,7 @@
2222 * that allow both single and multiple variables at once.
2323 *
2424 * @param global boolean Whether to store the values in the global window
25 - * object or a exclusively in the object property 'values'.
 25+ * object or a exclusively in the object property 'values'.
2626 * @return Map
2727 */
2828 function Map( global ) {
@@ -37,9 +37,9 @@
3838 * @param selection mixed String key or array of keys to get values for.
3939 * @param fallback mixed Value to use in case key(s) do not exist (optional).
4040 * @return mixed If selection was a string returns the value or null,
41 - * If selection was an array, returns an object of key/values (value is null if not found),
42 - * If selection was not passed or invalid, will return the 'values' object member (be careful as
43 - * objects are always passed by reference in JavaScript!).
 41+ * If selection was an array, returns an object of key/values (value is null if not found),
 42+ * If selection was not passed or invalid, will return the 'values' object member (be careful as
 43+ * objects are always passed by reference in JavaScript!).
4444 * @return Values as a string or object, null if invalid/inexistant.
4545 */
4646 Map.prototype.get = function( selection, fallback ) {
@@ -147,8 +147,9 @@
148148 // Return <key> if key does not exist
149149 return '<' + this.key + '>';
150150 }
151 - var text = this.map.get( this.key );
152 - var parameters = this.parameters;
 151+ var text = this.map.get( this.key );
 152+ parameters = this.parameters;
 153+
153154 text = text.replace( /\$(\d+)/g, function( string, match ) {
154155 var index = parseInt( match, 10 ) - 1;
155156 return index in parameters ? parameters[index] : '$' + match;
@@ -164,7 +165,7 @@
165166 }
166167
167168 /* This should be fixed up when we have a parser
168 - if ( this.format === 'parse' && 'language' in mediaWiki ) {
 169+ if ( this.format === 'parse' && 'language' in mw ) {
169170 text = mw.language.parse( text );
170171 }
171172 */
@@ -219,7 +220,7 @@
220221 this.log = function() { };
221222
222223 /**
223 - * @var constructor Make the Map-class publicly available.
 224+ * @var constructor Make the Map constructor publicly available.
224225 */
225226 this.Map = Map;
226227
@@ -251,7 +252,7 @@
252253 *
253254 * @param key string Key of message to get
254255 * @param parameter_1 mixed First argument in a list of variadic arguments,
255 - * each a parameter for $N replacement in messages.
 256+ * each a parameter for $N replacement in messages.
256257 * @return Message
257258 */
258259 this.message = function( key, parameter_1 /* [, parameter_2] */ ) {
@@ -271,7 +272,7 @@
272273 *
273274 * @param key string Key of message to get
274275 * @param parameters mixed First argument in a list of variadic arguments,
275 - * each a parameter for $N replacement in messages.
 276+ * each a parameter for $N replacement in messages.
276277 * @return String.
277278 */
278279 this.msg = function( key, parameters ) {
@@ -295,28 +296,27 @@
296297 * mediawiki.
297298 *
298299 * Format:
299 - * {
300 - * 'moduleName': {
301 - * 'dependencies': ['required module', 'required module', ...], (or) function() {}
302 - * 'state': 'registered', 'loading', 'loaded', 'ready', or 'error'
303 - * 'script': function() {},
304 - * 'style': 'css code string',
305 - * 'messages': { 'key': 'value' },
306 - * 'version': ############## (unix timestamp)
307 - * }
308 - * }
 300+ * {
 301+ * 'moduleName': {
 302+ * 'dependencies': ['required module', 'required module', ...], (or) function() {}
 303+ * 'state': 'registered', 'loading', 'loaded', 'ready', or 'error'
 304+ * 'script': function() {},
 305+ * 'style': 'css code string',
 306+ * 'messages': { 'key': 'value' },
 307+ * 'version': ############## (unix timestamp)
 308+ * }
309309 */
310 - var registry = {};
311 - // List of modules which will be loaded as when ready
312 - var batch = [];
313 - // List of modules to be loaded
314 - var queue = [];
315 - // List of callback functions waiting for modules to be ready to be called
316 - var jobs = [];
317 - // Flag inidicating that document ready has occured
318 - var ready = false;
319 - // Selector cache for the marker element. Use getMarker() to get/use the marker!
320 - var $marker = null;
 310+ var registry = {},
 311+ // List of modules which will be loaded as when ready
 312+ batch = [],
 313+ // List of modules to be loaded
 314+ queue = [],
 315+ // List of callback functions waiting for modules to be ready to be called
 316+ jobs = [],
 317+ // Flag inidicating that document ready has occured
 318+ ready = false,
 319+ // Selector cache for the marker element. Use getMarker() to get/use the marker!
 320+ $marker = null;
321321
322322 /* Private Methods */
323323
@@ -336,7 +336,7 @@
337337 }
338338
339339 function compare( a, b ) {
340 - if ( a.length != b.length ) {
 340+ if ( a.length !== b.length ) {
341341 return false;
342342 }
343343 for ( var i = 0; i < b.length; i++ ) {
@@ -356,11 +356,10 @@
357357 * Generates an ISO8601 "basic" string from a UNIX timestamp
358358 */
359359 function formatVersionNumber( timestamp ) {
360 - function pad( a, b, c ) {
361 - return [a < 10 ? '0' + a : a, b < 10 ? '0' + b : b, c < 10 ? '0' + c : c].join( '' );
362 - }
363 - var d = new Date();
364 - d.setTime( timestamp * 1000 );
 360+ var pad = function( a, b, c ) {
 361+ return [a < 10 ? '0' + a : a, b < 10 ? '0' + b : b, c < 10 ? '0' + c : c].join( '' );
 362+ },
 363+ d = new Date().setTime( timestamp * 1000 );
365364 return [
366365 pad( d.getUTCFullYear(), d.getUTCMonth() + 1, d.getUTCDate() ), 'T',
367366 pad( d.getUTCHours(), d.getUTCMinutes(), d.getUTCSeconds() ), 'Z'
@@ -435,7 +434,7 @@
436435 *
437436 * @param states string or array of strings of module states to filter by
438437 * @param modules array list of module names to filter (optional, all modules
439 - * will be used by default)
 438+ * will be used by default)
440439 * @return array list of filtered module names
441440 */
442441 function filter( states, modules ) {
@@ -456,7 +455,7 @@
457456 for ( var m = 0; m < modules.length; m++ ) {
458457 if ( registry[modules[m]] === undefined ) {
459458 // Module does not exist
460 - if ( states[s] == 'undefined' ) {
 459+ if ( states[s] === 'undefined' ) {
461460 // OK, undefined
462461 list[list.length] = modules[m];
463462 }
@@ -478,7 +477,6 @@
479478 * @param module string module name to execute
480479 */
481480 function execute( module, callback ) {
482 - var _fn = 'mw.loader::execute> ';
483481 if ( registry[module] === undefined ) {
484482 throw new Error( 'Module has not been registered yet: ' + module );
485483 } else if ( registry[module].state === 'registered' ) {
@@ -489,9 +487,10 @@
490488 throw new Error( 'Module has already been loaded: ' + module );
491489 }
492490 // Add styles
 491+ var style;
493492 if ( $.isPlainObject( registry[module].style ) ) {
494493 for ( var media in registry[module].style ) {
495 - var style = registry[module].style[media];
 494+ style = registry[module].style[media];
496495 if ( $.isArray( style ) ) {
497496 for ( var i = 0; i < style.length; i++ ) {
498497 getMarker().before( mw.html.element( 'link', {
@@ -515,14 +514,15 @@
516515 }
517516 // Execute script
518517 try {
519 - var script = registry[module].script;
520 - var markModuleReady = function() {
521 - registry[module].state = 'ready';
522 - handlePending( module );
523 - if ( $.isFunction( callback ) ) {
524 - callback();
525 - }
526 - };
 518+ var script = registry[module].script,
 519+ markModuleReady = function() {
 520+ registry[module].state = 'ready';
 521+ handlePending( module );
 522+ if ( $.isFunction( callback ) ) {
 523+ callback();
 524+ }
 525+ };
 526+
527527 if ( $.isArray( script ) ) {
528528 var done = 0;
529529 if ( script.length === 0 ) {
@@ -532,7 +532,7 @@
533533 for ( var i = 0; i < script.length; i++ ) {
534534 registry[module].state = 'loading';
535535 addScript( script[i], function() {
536 - if ( ++done == script.length ) {
 536+ if ( ++done === script.length ) {
537537 markModuleReady();
538538 }
539539 } );
@@ -545,7 +545,7 @@
546546 // This needs to NOT use mw.log because these errors are common in production mode
547547 // and not in debug mode, such as when a symbol that should be global isn't exported
548548 if ( window.console && typeof window.console.log === 'function' ) {
549 - console.log( _fn + 'Exception thrown by ' + module + ': ' + e.message );
 549+ console.log( 'mw.loader::execute> Exception thrown by ' + module + ': ' + e.message );
550550 }
551551 registry[module].state = 'error';
552552 throw e;
@@ -574,7 +574,7 @@
575575 }
576576 // Execute modules who's dependencies have just been met
577577 for ( var r in registry ) {
578 - if ( registry[r].state == 'loaded' ) {
 578+ if ( registry[r].state === 'loaded' ) {
579579 if ( compare(
580580 filter( ['ready'], registry[r].dependencies ),
581581 registry[r].dependencies ) )
@@ -611,8 +611,7 @@
612612 dependencies = [dependencies];
613613 if ( dependencies[0] in registry ) {
614614 for ( var n = 0; n < registry[dependencies[0]].dependencies.length; n++ ) {
615 - dependencies[dependencies.length] =
616 - registry[dependencies[0]].dependencies[n];
 615+ dependencies[dependencies.length] = registry[dependencies[0]].dependencies[n];
617616 }
618617 }
619618 }
@@ -621,7 +620,8 @@
622621 jobs[jobs.length] = {
623622 'dependencies': filter(
624623 ['undefined', 'registered', 'loading', 'loaded'],
625 - dependencies ),
 624+ dependencies
 625+ ),
626626 'ready': ready,
627627 'error': error
628628 };
@@ -656,9 +656,9 @@
657657 * to a query string of the form foo.bar,baz|bar.baz,quux
658658 */
659659 function buildModulesString( moduleMap ) {
660 - var arr = [];
 660+ var arr = [], p;
661661 for ( var prefix in moduleMap ) {
662 - var p = prefix === '' ? '' : prefix + '.';
 662+ p = prefix === '' ? '' : prefix + '.';
663663 arr.push( p + moduleMap[prefix].join( ',' ) );
664664 }
665665 return arr.join( '|' );
@@ -676,11 +676,11 @@
677677 // jQuery's getScript method is NOT better than doing this the old-fassioned way
678678 // because jQuery will eval the script's code, and errors will not have sane
679679 // line numbers.
680 - var script = document.createElement( 'script' );
 680+ var done = false,
 681+ script = document.createElement( 'script' );
681682 script.setAttribute( 'src', src );
682683 script.setAttribute( 'type', 'text/javascript' );
683684 if ( $.isFunction( callback ) ) {
684 - var done = false;
685685 // Attach handlers for all browsers -- this is based on jQuery.getScript
686686 script.onload = script.onreadystatechange = function() {
687687 if (
@@ -719,10 +719,21 @@
720720 * Requests dependencies from server, loading and executing when things when ready.
721721 */
722722 this.work = function() {
 723+ // Build a list of request parameters
 724+ var base = {
 725+ 'skin': mw.config.get( 'skin' ),
 726+ 'lang': mw.config.get( 'wgUserLanguage' ),
 727+ 'debug': mw.config.get( 'debug' )
 728+ },
 729+ // Extend request parameters with a list of modules in the batch
 730+ requests = [],
 731+ // Split into groups
 732+ groups = {};
 733+
723734 // Appends a list of modules to the batch
724735 for ( var q = 0; q < queue.length; q++ ) {
725736 // Only request modules which are undefined or registered
726 - if ( !( queue[q] in registry ) || registry[queue[q]].state == 'registered' ) {
 737+ if ( !( queue[q] in registry ) || registry[queue[q]].state === 'registered' ) {
727738 // Prevent duplicate entries
728739 if ( $.inArray( queue[q], batch ) === -1 ) {
729740 batch[batch.length] = queue[q];
@@ -742,22 +753,12 @@
743754 // Always order modules alphabetically to help reduce cache
744755 // misses for otherwise identical content
745756 batch.sort();
746 - // Build a list of request parameters
747 - var base = {
748 - 'skin': mw.config.get( 'skin' ),
749 - 'lang': mw.config.get( 'wgUserLanguage' ),
750 - 'debug': mw.config.get( 'debug' )
751 - };
752 - // Extend request parameters with a list of modules in the batch
753 - var requests = [];
754 - // Split into groups
755 - var groups = {};
756757 for ( var b = 0; b < batch.length; b++ ) {
757 - var group = registry[batch[b]].group;
758 - if ( !( group in groups ) ) {
759 - groups[group] = [];
 758+ var bGroup = registry[batch[b]].group;
 759+ if ( !( bGroup in groups ) ) {
 760+ groups[bGroup] = [];
760761 }
761 - groups[group][groups[group].length] = batch[b];
 762+ groups[bGroup][groups[bGroup].length] = batch[b];
762763 }
763764 for ( var group in groups ) {
764765 // Calculate the highest timestamp
@@ -767,24 +768,27 @@
768769 version = registry[groups[group][g]].version;
769770 }
770771 }
771 - var reqBase = $.extend( { 'version': formatVersionNumber( version ) }, base );
772 - var reqBaseLength = $.param( reqBase ).length;
773 - var reqs = [];
774 - var limit = mw.config.get( 'wgResourceLoaderMaxQueryLength', -1 );
775 - // We may need to split up the request to honor the query string length limit
776 - // So build it piece by piece
777 - var l = reqBaseLength + 9; // '&modules='.length == 9
778 - var r = 0;
 772+
 773+ var reqBase = $.extend( { 'version': formatVersionNumber( version ) }, base ),
 774+ reqBaseLength = $.param( reqBase ).length,
 775+ reqs = [],
 776+ limit = mw.config.get( 'wgResourceLoaderMaxQueryLength', -1 ),
 777+ // We may need to split up the request to honor the query string length limit,
 778+ // so build it piece by piece.
 779+ l = reqBaseLength + 9, // '&modules='.length == 9
 780+ r = 0;
 781+
779782 reqs[0] = {}; // { prefix: [ suffixes ] }
 783+
780784 for ( var i = 0; i < groups[group].length; i++ ) {
781 - // Determine how many bytes this module would add to the query string
782 - var lastDotIndex = groups[group][i].lastIndexOf( '.' );
783 - // Note that these substr() calls work even if lastDotIndex == -1
784 - var prefix = groups[group][i].substr( 0, lastDotIndex );
785 - var suffix = groups[group][i].substr( lastDotIndex + 1 );
786 - var bytesAdded = prefix in reqs[r] ?
787 - suffix.length + 3 : // '%2C'.length == 3
788 - groups[group][i].length + 3; // '%7C'.length == 3
 785+ // Determine how many bytes this module would add to the query string
 786+ var lastDotIndex = groups[group][i].lastIndexOf( '.' ),
 787+ // Note that these substr() calls work even if lastDotIndex == -1
 788+ prefix = groups[group][i].substr( 0, lastDotIndex ),
 789+ suffix = groups[group][i].substr( lastDotIndex + 1 ),
 790+ bytesAdded = prefix in reqs[r]
 791+ ? suffix.length + 3 // '%2C'.length == 3
 792+ : groups[group][i].length + 3; // '%7C'.length == 3
789793
790794 // If the request would become too long, create a new one,
791795 // but don't create empty requests
@@ -870,9 +874,9 @@
871875 *
872876 * @param module String: Name of module
873877 * @param script Mixed: Function of module code or String of URL to be used as the src
874 - * attribute when adding a script element to the body
 878+ * attribute when adding a script element to the body
875879 * @param style Object: Object of CSS strings keyed by media-type or Object of lists of URLs
876 - * keyed by media-type
 880+ * keyed by media-type
877881 * @param msgs Object: List of key/value pairs to be passed through mw.messages.set
878882 */
879883 this.implement = function( module, script, style, msgs ) {
@@ -918,19 +922,18 @@
919923 * Executes a function as soon as one or more required modules are ready
920924 *
921925 * @param dependencies string or array of strings of modules names the callback
922 - * dependencies to be ready before
923 - * executing
 926+ * dependencies to be ready before executing
924927 * @param ready function callback to execute when all dependencies are ready (optional)
925928 * @param error function callback to execute when if dependencies have a errors (optional)
926929 */
927930 this.using = function( dependencies, ready, error ) {
 931+ var tod = typeof dependencies;
928932 // Validate input
929 - if ( typeof dependencies !== 'object' && typeof dependencies !== 'string' ) {
930 - throw new Error( 'dependencies must be a string or an array, not a ' +
931 - typeof dependencies );
 933+ if ( tod !== 'object' && tod !== 'string' ) {
 934+ throw new Error( 'dependencies must be a string or an array, not a ' + tod );
932935 }
933936 // Allow calling with a single dependency as a string
934 - if ( typeof dependencies === 'string' ) {
 937+ if ( tod === 'string' ) {
935938 dependencies = [dependencies];
936939 }
937940 // Resolve entire dependency map
@@ -957,24 +960,22 @@
958961 * Loads an external script or one or more modules for future use
959962 *
960963 * @param modules mixed either the name of a module, array of modules,
961 - * or a URL of an external script or style
 964+ * or a URL of an external script or style
962965 * @param type string mime-type to use if calling with a URL of an
963 - * external script or style; acceptable values are "text/css" and
964 - * "text/javascript"; if no type is provided, text/javascript is
965 - * assumed
 966+ * external script or style; acceptable values are "text/css" and
 967+ * "text/javascript"; if no type is provided, text/javascript is assumed.
966968 */
967969 this.load = function( modules, type ) {
968970 // Validate input
969971 if ( typeof modules !== 'object' && typeof modules !== 'string' ) {
970 - throw new Error( 'modules must be a string or an array, not a ' +
971 - typeof modules );
 972+ throw new Error( 'modules must be a string or an array, not a ' + typeof modules );
972973 }
973974 // Allow calling with an external script or single dependency as a string
974975 if ( typeof modules === 'string' ) {
975976 // Support adding arbitrary external scripts
976 - if ( modules.substr( 0, 7 ) == 'http://' || modules.substr( 0, 8 ) == 'https://' ) {
 977+ if ( modules.substr( 0, 7 ) === 'http://' || modules.substr( 0, 8 ) === 'https://' ) {
977978 if ( type === 'text/css' ) {
978 - $( 'head' ).append( $( '<link />', {
 979+ $( 'head' ).append( $( '<link/>', {
979980 rel: 'stylesheet',
980981 type: 'text/css',
981982 href: modules
@@ -1037,11 +1038,12 @@
10381039 }
10391040 return null;
10401041 };
 1042+
10411043 /**
10421044 * @deprecated use mw.loader.getVersion() instead
10431045 */
10441046 this.version = function() {
1045 - return mediaWiki.loader.getVersion.apply( mediaWiki.loader, arguments );
 1047+ return mw.loader.getVersion.apply( mw.loader, arguments );
10461048 };
10471049
10481050 /**
@@ -1106,17 +1108,17 @@
11071109 * @param name The tag name.
11081110 * @param attrs An object with members mapping element names to values
11091111 * @param contents The contents of the element. May be either:
1110 - * - string: The string is escaped.
1111 - * - null or undefined: The short closing form is used, e.g. <br/>.
1112 - * - this.Raw: The value attribute is included without escaping.
1113 - * - this.Cdata: The value attribute is included, and an exception is
1114 - * thrown if it contains an illegal ETAGO delimiter.
1115 - * See http://www.w3.org/TR/1999/REC-html401-19991224/appendix/notes.html#h-B.3.2
 1112+ * - string: The string is escaped.
 1113+ * - null or undefined: The short closing form is used, e.g. <br/>.
 1114+ * - this.Raw: The value attribute is included without escaping.
 1115+ * - this.Cdata: The value attribute is included, and an exception is
 1116+ * thrown if it contains an illegal ETAGO delimiter.
 1117+ * See http://www.w3.org/TR/1999/REC-html401-19991224/appendix/notes.html#h-B.3.2
11161118 *
11171119 * Example:
1118 - * var h = mw.html;
1119 - * return h.element( 'div', {},
1120 - * new h.Raw( h.element( 'img', {src: '<'} ) ) );
 1120+ * var h = mw.html;
 1121+ * return h.element( 'div', {},
 1122+ * new h.Raw( h.element( 'img', {src: '<'} ) ) );
11211123 * Returns <div><img src="&lt;"/></div>
11221124 */
11231125 this.element = function( name, attrs, contents ) {
@@ -1160,9 +1162,6 @@
11611163 // Alias $j to jQuery for backwards compatibility
11621164 window.$j = jQuery;
11631165
1164 -// Global alias
1165 -window.mw = mediaWiki;
1166 -
11671166 /* Auto-register from pre-loaded startup scripts */
11681167
11691168 if ( jQuery.isFunction( startUp ) ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r92934Fix typo in r92933 that resulted in global variable leakagecatrope09:19, 23 July 2011
r92964fu r92933 - fix breakagejeroendedauw20:56, 23 July 2011
r93012More mediawiki.js cleanup...krinkle20:22, 24 July 2011
r93013More mediawiki.js cleanup (addScript AJAX)...krinkle20:38, 24 July 2011

Comments

#Comment by Nikerabbit (talk | contribs)   19:05, 23 July 2011

Liar:

Uncaught TypeError: Cannot read property 'options' of undefined
Uncaught TypeError: Object 1311447359000 has no method 'getUTCFullYear'
Uncaught TypeError: Object 1309808662000 has no method 'getUTCFullYear'
#Comment by Jeroen De Dauw (talk | contribs)   20:57, 23 July 2011

I think I fixed it in r92964.

Status & tagging log