r107741 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r107740‎ | r107741 | r107742 >
Date:21:25, 31 December 2011
Author:krinkle
Status:ok
Tags:
Comment:
[Core JS] More fixing of global config variable usage
* mw.config is the new way, and global config variable lookups are deprecated

* Based on two phase3-wide quick searches:
-- of " wg": http://toolserver.org/~krinkle/wikimedia-svn-search/view.php?id=321&hash=81700bf7486e4fee3b7bc1f83eb9eba6
-- of "!wg": http://toolserver.org/~krinkle/wikimedia-svn-search/view.php?id=327&hash=47c9d54a7a1d5d58a724dd834585f40d

Related changes:
* Changed some php comments mentioning "wg" variables to include the dollar sign, and a typo when the wf function prefix was meant.
* Removed TODO comment in wikibits.js and made it use the JS equivalent of wfUrlencode, which we have now, mw.util.wikiUrlencode
* SpecialUpload.php: use OutputPage::addJsConfigVars instead of creating a new script tag through OutputPage::addScript(Skin::makeVariablesScript(..))
* Renamed wgUploadSetup in upload.js and made it local. Not used anywhere in ./trunk/phase3 and ./trunk/extensions
* Fix OutputPage::addJsConfigVars so that it can actually be called with an array instead of two arguments for key/value
* Some minor whitespace/convention stuff around the same line
Modified paths:
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/HTMLForm.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/logging/LogEventsList.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUpload.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/skins/common/ajax.js (modified) (history)
  • /trunk/phase3/skins/common/mwsuggest.js (modified) (history)
  • /trunk/phase3/skins/common/protect.js (modified) (history)
  • /trunk/phase3/skins/common/upload.js (modified) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/MWNamespaceTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/upload/UploadTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/skins/SideBarTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
@@ -1015,7 +1015,7 @@
10161016 'email',
10171017 'prefs-help-realname',
10181018
1019 - # 3 messages depending upon wgEmailConfirmToEdit and $wgEnableUserEmail
 1019+ # 3 messages depending upon $wgEmailConfirmToEdit and $wgEnableUserEmail
10201020 'prefs-help-email',
10211021 'prefs-help-email-others',
10221022 'prefs-help-email-required',
Index: trunk/phase3/skins/common/protect.js
@@ -77,16 +77,20 @@
7878 },
7979
8080 /**
81 - * Is this protection level cascadeable?
82 - * @param level String
83 - *
84 - * @return boolean
85 - *
 81+ * Checks if a cerain protection level is cascadeable.
 82+ * @param level {String}
 83+ * @return {Boolean}
8684 */
8785 'isCascadeableLevel': function( level ) {
88 - for (var k = 0; k < wgCascadeableLevels.length; k++) {
89 - if ( wgCascadeableLevels[k] == level ) {
90 - return true;
 86+ var cascadeLevels, len, i;
 87+
 88+ cascadeLevels = mw.config.get( 'wgCascadeableLevels' );
 89+ // cascadeLevels isn't defined on all pages
 90+ if ( cascadeLevels ) {
 91+ for ( i = 0, len = cascadeLevels.length; i < len; i += 1 ) {
 92+ if ( cascadeLevels[i] === level ) {
 93+ return true;
 94+ }
9195 }
9296 }
9397 return false;
Index: trunk/phase3/skins/common/ajax.js
@@ -84,9 +84,7 @@
8585 var i, x, n;
8686 var uri;
8787 var post_data;
88 - uri = wgServer +
89 - ( ( wgScript == null ) ? ( wgScriptPath + '/index.php' ) : wgScript ) +
90 - '?action=ajax';
 88+ uri = mw.util.wikiScript() + '?action=ajax';
9189 if ( sajax_request_type == 'GET' ) {
9290 if ( uri.indexOf( '?' ) == -1 ) {
9391 uri = uri + '?rs=' + encodeURIComponent( func_name );
Index: trunk/phase3/skins/common/upload.js
@@ -1,3 +1,7 @@
 2+( function () {
 3+var ajaxUploadDestCheck = mw.config.get( 'wgAjaxUploadDestCheck' ),
 4+ fileExtensions = mw.config.get( 'wgFileExtensions' );
 5+
26 window.licenseSelectorCheck = function() {
37 var selector = document.getElementById( "wpLicense" );
48 var selection = selector.options[selector.selectedIndex].value;
@@ -11,7 +15,7 @@
1216 wgUploadLicenseObj.fetchPreview( selection );
1317 };
1418
15 -window.wgUploadSetup = function() {
 19+function uploadSetup() {
1620 // Disable URL box if the URL copy upload source type is not selected
1721 var e = document.getElementById( 'wpSourceTypeurl' );
1822 if( e ) {
@@ -36,7 +40,7 @@
3741 }
3842
3943 // AJAX wpDestFile warnings
40 - if ( wgAjaxUploadDestCheck ) {
 44+ if ( ajaxUploadDestCheck ) {
4145 // Insert an event handler that fetches upload warnings when wpDestFile
4246 // has been changed
4347 document.getElementById( 'wpDestFile' ).onchange = function ( e ) {
@@ -54,7 +58,7 @@
5559 }
5660
5761 var wpLicense = document.getElementById( 'wpLicense' );
58 - if ( wgAjaxLicensePreview && wpLicense ) {
 62+ if ( mw.config.get( 'wgAjaxLicensePreview' ) && wpLicense ) {
5963 // License selector check
6064 wpLicense.onchange = licenseSelectorCheck;
6165
@@ -74,8 +78,11 @@
7579
7680
7781 // fillDestFile setup
78 - for ( var i = 0; i < wgUploadSourceIds.length; i++ )
79 - document.getElementById( wgUploadSourceIds[i] ).onchange = function (e) {
 82+ var i,
 83+ uploadSourceIds = mw.config.get( 'wgUploadSourceIds' ),
 84+ len = uploadSourceIds.length;
 85+ for ( i = 0; i < len; i += 1 )
 86+ document.getElementById( uploadSourceIds[i] ).onchange = function (e) {
8087 fillDestFilename( this.id );
8188 };
8289 };
@@ -89,7 +96,7 @@
9097 'timeoutID': false,
9198
9299 'keypress': function () {
93 - if ( !wgAjaxUploadDestCheck || !sajax_init_object() ) return;
 100+ if ( !ajaxUploadDestCheck || !sajax_init_object() ) return;
94101
95102 // Find file to upload
96103 var destFile = document.getElementById('wpDestFile');
@@ -114,7 +121,7 @@
115122 },
116123
117124 'checkNow': function (fname) {
118 - if ( !wgAjaxUploadDestCheck || !sajax_init_object() ) return;
 125+ if ( !ajaxUploadDestCheck || !sajax_init_object() ) return;
119126 if ( this.timeoutID ) {
120127 window.clearTimeout( this.timeoutID );
121128 }
@@ -123,7 +130,7 @@
124131 },
125132
126133 'timeout' : function() {
127 - if ( !wgAjaxUploadDestCheck || !sajax_init_object() ) return;
 134+ if ( !ajaxUploadDestCheck || !sajax_init_object() ) return;
128135 injectSpinner( document.getElementById( 'wpDestFile' ), 'destcheck' );
129136
130137 // Get variables into local scope so that they will be preserved for the
@@ -168,7 +175,7 @@
169176 };
170177
171178 window.fillDestFilename = function(id) {
172 - if (!wgUploadAutoFill) {
 179+ if ( !mw.config.get( 'wgUploadAutoFill' ) ) {
173180 return;
174181 }
175182 if (!document.getElementById) {
@@ -197,12 +204,12 @@
198205 // Clear the filename if it does not have a valid extension.
199206 // URLs are less likely to have a useful extension, so don't include them in the
200207 // extension check.
201 - if( wgStrictFileExtensions && wgFileExtensions && id != 'wpUploadFileURL' ) {
 208+ if ( mw.config.get( 'wgStrictFileExtensions' ) && fileExtensions && id !== 'wpUploadFileURL' ) {
202209 var found = false;
203 - if( fname.lastIndexOf( '.' ) != -1 ) {
 210+ if ( fname.lastIndexOf( '.' ) !== -1 ) {
204211 var ext = fname.substr( fname.lastIndexOf( '.' ) + 1 );
205 - for( var i = 0; i < wgFileExtensions.length; i++ ) {
206 - if( wgFileExtensions[i].toLowerCase() == ext.toLowerCase() ) {
 212+ for ( var i = 0; i < fileExtensions.length; i += 1 ) {
 213+ if ( fileExtensions[i].toLowerCase() === ext.toLowerCase() ) {
207214 found = true;
208215 break;
209216 }
@@ -229,7 +236,7 @@
230237 // Replace spaces by underscores
231238 fname = fname.replace( / /g, '_' );
232239 // Capitalise first letter if needed
233 - if ( wgCapitalizeUploads ) {
 240+ if ( mw.config.get( 'wgCapitalizeUploads' ) ) {
234241 fname = fname.charAt( 0 ).toUpperCase().concat( fname.substring( 1, 10000 ) );
235242 }
236243
@@ -253,7 +260,7 @@
254261 'responseCache' : { '' : '' },
255262
256263 'fetchPreview': function( license ) {
257 - if( !wgAjaxLicensePreview ) return;
 264+ if ( !mw.config.get( 'wgAjaxLicensePreview' ) ) return;
258265 for (cached in this.responseCache) {
259266 if (cached == license) {
260267 this.showPreview( this.responseCache[license] );
@@ -265,7 +272,7 @@
266273 var title = document.getElementById('wpDestFile').value;
267274 if ( !title ) title = 'File:Sample.jpg';
268275
269 - var url = wgScriptPath + '/api' + wgScriptExtension
 276+ var url = mw.util.wikiScript( 'api' )
270277 + '?action=parse&text={{' + encodeURIComponent( license ) + '}}'
271278 + '&title=' + encodeURIComponent( title )
272279 + '&prop=text&pst&format=json';
@@ -293,4 +300,6 @@
294301
295302 };
296303
297 -addOnloadHook( wgUploadSetup );
 304+$( document ).ready( uploadSetup );
 305+
 306+}() );
Index: trunk/phase3/skins/common/wikibits.js
@@ -69,9 +69,8 @@
7070 };
7171
7272 window.importScript = function( page ) {
73 - // TODO: might want to introduce a utility function to match wfUrlencode() in PHP
74 - var uri = wgScript + '?title=' +
75 - encodeURIComponent(page.replace(/ /g,'_')).replace(/%2F/ig,'/').replace(/%3A/ig,':') +
 73+ var uri = mw.config.get( 'wgScript' ) + '?title=' +
 74+ mw.util.wikiUrlencode( page ) +
7675 '&action=raw&ctype=text/javascript';
7776 return importScriptURI( uri );
7877 };
@@ -90,7 +89,7 @@
9190 };
9291
9392 window.importStylesheet = function( page ) {
94 - return importStylesheetURI( wgScript + '?action=raw&ctype=text/css&title=' + encodeURIComponent( page.replace(/ /g,'_') ) );
 93+ return importStylesheetURI( mw.config.get( 'wgScript' ) + '?action=raw&ctype=text/css&title=' + mw.util.wikiUrlencode( page ) );
9594 };
9695
9796 window.importStylesheetURI = function( url, media ) {
Index: trunk/phase3/skins/common/mwsuggest.js
@@ -478,7 +478,7 @@
479479 var query = os_timer.query;
480480 os_timer = null;
481481 var path = mw.config.get( 'wgMWSuggestTemplate' ).replace( "{namespaces}", os_getNamespaces( r ) )
482 - .replace( "{dbname}", wgDBname )
 482+ .replace( "{dbname}", mw.config.get( 'wgDBname' ) )
483483 .replace( "{searchTerms}", os_encodeQuery( query ) );
484484
485485 // try to get from cache, if not fetch using ajax
Index: trunk/phase3/docs/hooks.txt
@@ -1221,7 +1221,7 @@
12221222 - lim Integer Limit of items to show, default is 50
12231223 - conds Array Extra conditions for the query (e.g. "log_action != 'revision'")
12241224 - showIfEmpty boolean Set to false if you don't want any output in case the loglist is empty if set to true (default), "No matching items in log" is displayed if loglist is empty
1225 - - msgKey Array If you want a nice box with a message, set this to the key of the message. First element is the message key, additional optional elements are parameters for the key that are processed with wgMsgExt and option 'parse'
 1225+ - msgKey Array If you want a nice box with a message, set this to the key of the message. First element is the message key, additional optional elements are parameters for the key that are processed with wfMsgExt and option 'parse'
12261226 - offset Set to overwrite offset parameter in $wgRequest set to '' to unset offset
12271227 - wrap String Wrap the message in html (usually something like "&lt;div ...>$1&lt;/div>").
12281228 - flags Integer display flags (NO_ACTION_LINK,NO_EXTRA_USER_LINKS)
Index: trunk/phase3/tests/phpunit/skins/SideBarTest.php
@@ -136,7 +136,7 @@
137137 }
138138
139139 /**
140 - * Test wgNoFollowLinks in sidebar
 140+ * Test $wgNoFollowLinks in sidebar
141141 */
142142 function testRespectWgnofollowlinks() {
143143 global $wgNoFollowLinks;
@@ -145,7 +145,7 @@
146146
147147 $attribs = $this->getAttribs();
148148 $this->assertArrayNotHasKey( 'rel', $attribs,
149 - 'External URL in sidebar do not have rel=nofollow when wgNoFollowLinks = false'
 149+ 'External URL in sidebar do not have rel=nofollow when $wgNoFollowLinks = false'
150150 );
151151
152152 // Restore global
@@ -153,7 +153,7 @@
154154 }
155155
156156 /**
157 - * Test wgExternaLinkTarget in sidebar
 157+ * Test $wgExternaLinkTarget in sidebar
158158 */
159159 function testRespectExternallinktarget() {
160160 global $wgExternalLinkTarget;
Index: trunk/phase3/tests/phpunit/includes/upload/UploadTest.php
@@ -100,7 +100,7 @@
101101 }
102102
103103 /**
104 - * test uploading a 100 bytes file with wgMaxUploadSize = 100
 104+ * test uploading a 100 bytes file with $wgMaxUploadSize = 100
105105 *
106106 * This method should be abstracted so we can test different settings.
107107 */
Index: trunk/phase3/tests/phpunit/includes/MWNamespaceTest.php
@@ -422,7 +422,7 @@
423423 $this->assertEquals(
424424 array( NS_MAIN, NS_USER, NS_CATEGORY ),
425425 MWNamespace::getcontentNamespaces(),
426 - 'NS_MAIN is forced in wgContentNamespaces even if unwanted'
 426+ 'NS_MAIN is forced in $wgContentNamespaces even if unwanted'
427427 );
428428
429429 # test other cases, return $wgcontentNamespaces as is
Index: trunk/phase3/includes/HTMLForm.php
@@ -423,7 +423,7 @@
424424 }
425425
426426 /**
427 - * Display the form (sending to wgOut), with an appropriate error
 427+ * Display the form (sending to $wgOut), with an appropriate error
428428 * message or stack of messages, and any validation errors, etc.
429429 * @param $submitResult Mixed output from HTMLForm::trySubmit()
430430 */
Index: trunk/phase3/includes/OutputPage.php
@@ -2750,9 +2750,9 @@
27512751 * Add one or more variables to be set in mw.config in JavaScript.
27522752 *
27532753 * @param $key {String|Array} Key or array of key/value pars.
2754 - * @param $value {Mixed} Value of the configuration variable.
 2754+ * @param $value {Mixed} [optional] Value of the configuration variable.
27552755 */
2756 - public function addJsConfigVars( $keys, $value ) {
 2756+ public function addJsConfigVars( $keys, $value = null ) {
27572757 if ( is_array( $keys ) ) {
27582758 foreach ( $keys as $key => $value ) {
27592759 $this->mJsConfigVars[$key] = $value;
Index: trunk/phase3/includes/logging/LogEventsList.php
@@ -614,7 +614,7 @@
615615 * if set to true (default), "No matching items in log" is displayed if loglist is empty
616616 * - msgKey Array If you want a nice box with a message, set this to the key of the message.
617617 * First element is the message key, additional optional elements are parameters for the key
618 - * that are processed with wgMsgExt and option 'parse'
 618+ * that are processed with wfMsgExt and option 'parse'
619619 * - offset Set to overwrite offset parameter in $wgRequest
620620 * set to '' to unset offset
621621 * - wrap String Wrap the message in html (usually something like "<div ...>$1</div>").
Index: trunk/phase3/includes/specials/SpecialUpload.php
@@ -1084,7 +1084,7 @@
10851085 );
10861086
10871087 $out = $this->getOutput();
1088 - $out->addScript( Skin::makeVariablesScript( $scriptVars ) );
 1088+ $out->addJsConfigVars( $scriptVars );
10891089
10901090
10911091 $out->addModules( array(
Index: trunk/phase3/resources/Resources.php
@@ -765,7 +765,7 @@
766766 'size-gigabytes',
767767 'largefileserver',
768768 ),
769 - 'dependencies' => array( 'mediawiki.libs.jpegmeta' ),
 769+ 'dependencies' => array( 'mediawiki.libs.jpegmeta', 'mediawiki.util' ),
770770 ),
771771
772772 /* MediaWiki Legacy */
@@ -774,7 +774,7 @@
775775 'scripts' => 'common/ajax.js',
776776 'remoteBasePath' => $GLOBALS['wgStylePath'],
777777 'localBasePath' => $GLOBALS['wgStyleDirectory'],
778 - 'dependencies' => 'mediawiki.legacy.wikibits',
 778+ 'dependencies' => array( 'mediawiki.util', 'mediawiki.legacy.wikibits' ),
779779 ),
780780 'mediawiki.legacy.commonPrint' => array(
781781 'styles' => array( 'common/commonPrint.css' => array( 'media' => 'print' ) ),

Status & tagging log