r106991 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r106990‎ | r106991 | r106992 >
Date:22:06, 21 December 2011
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
[Core JS] Applying conventions to mw.util
- new recommended closure format
- fixing repeated var statements, or var statement inside blocks (there is no block scope in JavaScript) and moving them to the top of the function
- line breaking, indention, white space
- and more.. see also https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.util.js
@@ -1,7 +1,8 @@
22 /**
33 * Utilities
44 */
5 -( function( $ ) {
 5+( function ( $, mw ) {
 6+"use strict";
67
78 // Local cache and alias
89 var util = mw.util = {
@@ -11,6 +12,11 @@
1213 * (don't call before document ready)
1314 */
1415 'init' : function() {
 16+ var profile = $.client.profile(),
 17+ $tocContainer = $( '#toc' ),
 18+ $tocTitle = $( '#toctitle' ),
 19+ $tocToggleLink = $( '#togglelink' ),
 20+ hideTocCookie;
1521
1622 /* Set up $.messageBox */
1723 $.messageBoxNew( {
@@ -18,37 +24,34 @@
1925 'parent': '#content'
2026 } );
2127
22 - // Shortcut to client profile return
23 - var profile = $.client.profile();
24 -
2528 /* Set tooltipAccessKeyPrefix */
2629
2730 // Opera on any platform
28 - if ( profile.name == 'opera' ) {
 31+ if ( profile.name === 'opera' ) {
2932 util.tooltipAccessKeyPrefix = 'shift-esc-';
3033
3134 // Chrome on any platform
32 - } else if ( profile.name == 'chrome' ) {
 35+ } else if ( profile.name === 'chrome' ) {
3336 // Chrome on Mac or Chrome on other platform ?
34 - util.tooltipAccessKeyPrefix = ( profile.platform == 'mac'
 37+ util.tooltipAccessKeyPrefix = ( profile.platform === 'mac'
3538 ? 'ctrl-option-' : 'alt-' );
3639
3740 // Non-Windows Safari with webkit_version > 526
3841 } else if ( profile.platform !== 'win'
39 - && profile.name == 'safari'
 42+ && profile.name === 'safari'
4043 && profile.layoutVersion > 526 ) {
4144 util.tooltipAccessKeyPrefix = 'ctrl-alt-';
4245
4346 // Safari/Konqueror on any platform, or any browser on Mac
4447 // (but not Safari on Windows)
45 - } else if ( !( profile.platform == 'win' && profile.name == 'safari' )
46 - && ( profile.name == 'safari'
47 - || profile.platform == 'mac'
48 - || profile.name == 'konqueror' ) ) {
 48+ } else if ( !( profile.platform === 'win' && profile.name === 'safari' )
 49+ && ( profile.name === 'safari'
 50+ || profile.platform === 'mac'
 51+ || profile.name === 'konqueror' ) ) {
4952 util.tooltipAccessKeyPrefix = 'ctrl-';
5053
5154 // Firefox 2.x and later
52 - } else if ( profile.name == 'firefox' && profile.versionBase > '1' ) {
 55+ } else if ( profile.name === 'firefox' && profile.versionBase > '1' ) {
5356 util.tooltipAccessKeyPrefix = 'alt-shift-';
5457 }
5558
@@ -73,20 +76,23 @@
7477 util.$content = $( '#content' );
7578 }
7679
77 - /* Table of Contents toggle */
78 - var $tocContainer = $( '#toc' ),
79 - $tocTitle = $( '#toctitle' ),
80 - $tocToggleLink = $( '#togglelink' );
 80+ // Table of contents toggle
8181 // Only add it if there is a TOC and there is no toggle added already
8282 if ( $tocContainer.length && $tocTitle.length && !$tocToggleLink.length ) {
83 - var hideTocCookie = $.cookie( 'mw_hidetoc' );
 83+ hideTocCookie = $.cookie( 'mw_hidetoc' );
8484 $tocToggleLink = $( '<a href="#" class="internal" id="togglelink"></a>' )
8585 .text( mw.msg( 'hidetoc' ) )
8686 .click( function(e){
8787 e.preventDefault();
8888 util.toggleToc( $(this) );
8989 } );
90 - $tocTitle.append( $tocToggleLink.wrap( '<span class="toctoggle"></span>' ).parent().prepend( '&nbsp;[' ).append( ']&nbsp;' ) );
 90+ $tocTitle.append(
 91+ $tocToggleLink
 92+ .wrap( '<span class="toctoggle"></span>' )
 93+ .parent()
 94+ .prepend( '&nbsp;[' )
 95+ .append( ']&nbsp;' )
 96+ );
9197
9298 if ( hideTocCookie == '1' ) {
9399 // Cookie says user want toc hidden
@@ -140,7 +146,8 @@
141147 * @return string Address to script (eg. '/w/api.php' )
142148 */
143149 'wikiScript' : function( str ) {
144 - return mw.config.get( 'wgScriptPath' ) + '/' + ( str || 'index' ) + mw.config.get( 'wgScriptExtension' );
 150+ return mw.config.get( 'wgScriptPath' ) + '/' + ( str || 'index' ) +
 151+ mw.config.get( 'wgScriptExtension' );
145152 },
146153
147154 /**
@@ -156,7 +163,8 @@
157164 if ( s.styleSheet ) {
158165 s.styleSheet.cssText = text; // IE
159166 } else {
160 - s.appendChild( document.createTextNode( text + '' ) ); // Safari sometimes borks on null
 167+ // Safari sometimes borks on null
 168+ s.appendChild( document.createTextNode( text + '' ) );
161169 }
162170 document.getElementsByTagName('head')[0].appendChild( s );
163171 return s.sheet || s;
@@ -210,10 +218,10 @@
211219 * @return mixed Parameter value or null.
212220 */
213221 'getParamValue' : function( param, url ) {
214 - url = url ? url : document.location.href;
 222+ url = url || document.location.href;
215223 // Get last match, stop at hash
216 - var re = new RegExp( '^[^#]*[&?]' + $.escapeRE( param ) + '=([^&#]*)' );
217 - var m = re.exec( url );
 224+ var re = new RegExp( '^[^#]*[&?]' + $.escapeRE( param ) + '=([^&#]*)' ),
 225+ m = re.exec( url );
218226 if ( m && m.length > 1 ) {
219227 // Beware that decodeURIComponent is not required to understand '+'
220228 // by spec, as encodeURIComponent does not produce it.
@@ -241,7 +249,8 @@
242250 * otherwise, all the nodes that will probably have accesskeys by
243251 * default are updated.
244252 *
245 - * @param nodeList {Array|jQuery} (optional) A jQuery object, or array of elements to update.
 253+ * @param nodeList {Array|jQuery} [optional] A jQuery object, or array
 254+ * of elements to update.
246255 */
247256 'updateTooltipAccessKeys' : function( nodeList ) {
248257 var $nodes;
@@ -317,13 +326,14 @@
318327 * depending on the skin) or null if no element was added to the document.
319328 */
320329 'addPortletLink' : function( portlet, href, text, id, tooltip, accesskey, nextnode ) {
 330+ var $item, $link, $portlet, $ul;
321331
322332 // Check if there's atleast 3 arguments to prevent a TypeError
323333 if ( arguments.length < 3 ) {
324334 return null;
325335 }
326336 // Setup the anchor tag
327 - var $link = $( '<a></a>' ).attr( 'href', href ).text( text );
 337+ $link = $( '<a>' ).attr( 'href', href ).text( text );
328338 if ( tooltip ) {
329339 $link.attr( 'title', tooltip );
330340 }
@@ -341,12 +351,12 @@
342352 default : // Skins like chick, modern, monobook, myskin, simple, vector...
343353
344354 // Select the specified portlet
345 - var $portlet = $( '#' + portlet );
 355+ $portlet = $( '#' + portlet );
346356 if ( $portlet.length === 0 ) {
347357 return null;
348358 }
349359 // Select the first (most likely only) unordered list inside the portlet
350 - var $ul = $portlet.find( 'ul' );
 360+ $ul = $portlet.find( 'ul' );
351361
352362 // If it didn't have an unordered list yet, create it
353363 if ( $ul.length === 0 ) {
@@ -371,7 +381,6 @@
372382
373383 // Wrap the anchor tag in a list item (and a span if $portlet is a Vector tab)
374384 // and back up the selector to the list item
375 - var $item;
376385 if ( $portlet.hasClass( 'vectorTabs' ) ) {
377386 $item = $link.wrap( '<li><span></span></li>' ).parent().parent();
378387 } else {
@@ -392,12 +401,12 @@
393402 }
394403
395404 // Where to put our node ?
396 - // - nextnode is a DOM element (before MW 1.17, in wikibits.js, this was the only option)
397 - if ( nextnode && nextnode.parentNode == $ul[0] ) {
 405+ // - nextnode is a DOM element (was the only option before MW 1.17, in wikibits.js)
 406+ if ( nextnode && nextnode.parentNode === $ul[0] ) {
398407 $(nextnode).before( $item );
399408
400409 // - nextnode is a CSS selector for jQuery
401 - } else if ( typeof nextnode == 'string' && $ul.find( nextnode ).length !== 0 ) {
 410+ } else if ( typeof nextnode === 'string' && $ul.find( nextnode ).length !== 0 ) {
402411 $ul.find( nextnode ).eq( 0 ).before( $item );
403412
404413
@@ -436,7 +445,7 @@
437446 // an mw-js-message div to start with.
438447 var $messageDiv = $( '#mw-js-message' );
439448 if ( !$messageDiv.length ) {
440 - $messageDiv = $( '<div id="mw-js-message">' );
 449+ $messageDiv = $( '<div id="mw-js-message"></div>' );
441450 if ( util.$content.parent().length ) {
442451 util.$content.parent().prepend( $messageDiv );
443452 } else {
@@ -549,11 +558,14 @@
550559 if ( typeof address !== 'string' ) {
551560 return false;
552561 }
553 - var block = allowBlock ? '(?:\\/(?:3[0-2]|[12]?\\d))?' : '';
554 - var RE_IP_BYTE = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|0?[0-9]?[0-9])';
555 - var RE_IP_ADD = '(?:' + RE_IP_BYTE + '\\.){3}' + RE_IP_BYTE;
556 - return address.search( new RegExp( '^' + RE_IP_ADD + block + '$' ) ) != -1;
 562+
 563+ var block = allowBlock ? '(?:\\/(?:3[0-2]|[12]?\\d))?' : '',
 564+ RE_IP_BYTE = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|0?[0-9]?[0-9])',
 565+ RE_IP_ADD = '(?:' + RE_IP_BYTE + '\\.){3}' + RE_IP_BYTE;
 566+
 567+ return address.search( new RegExp( '^' + RE_IP_ADD + block + '$' ) ) !== -1;
557568 },
 569+
558570 /**
559571 * Note: borrows from IP::isIPv6
560572 *
@@ -565,8 +577,9 @@
566578 if ( typeof address !== 'string' ) {
567579 return false;
568580 }
569 - var block = allowBlock ? '(?:\\/(?:12[0-8]|1[01][0-9]|[1-9]?\\d))?' : '';
570 - var RE_IPV6_ADD =
 581+
 582+ var block = allowBlock ? '(?:\\/(?:12[0-8]|1[01][0-9]|[1-9]?\\d))?' : '',
 583+ RE_IPV6_ADD =
571584 '(?:' + // starts with "::" (including "::")
572585 ':(?::|(?::' + '[0-9A-Fa-f]{1,4}' + '){1,7})' +
573586 '|' + // ends with "::" (except "::")
@@ -574,15 +587,18 @@
575588 '|' + // contains no "::"
576589 '[0-9A-Fa-f]{1,4}' + '(?::' + '[0-9A-Fa-f]{1,4}' + '){7}' +
577590 ')';
578 - if ( address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) != -1 ) {
 591+
 592+ if ( address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) !== -1 ) {
579593 return true;
580594 }
 595+
581596 RE_IPV6_ADD = // contains one "::" in the middle (single '::' check below)
582597 '[0-9A-Fa-f]{1,4}' + '(?:::?' + '[0-9A-Fa-f]{1,4}' + '){1,6}';
583 - return address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) != -1
584 - && address.search( /::/ ) != -1 && address.search( /::.*::/ ) == -1;
 598+
 599+ return address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) !== -1
 600+ && address.search( /::/ ) !== -1 && address.search( /::.*::/ ) === -1;
585601 }
586602
587603 };
588604
589 -} )( jQuery );
 605+} )( jQuery, mediaWiki );

Comments

#Comment by Nikerabbit (talk | contribs)   07:24, 22 December 2011

Init is quite a long function. Considering that some variables are declared over 50 lines before used.

Status & tagging log