r96958 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96957‎ | r96958 | r96959 >
Date:12:15, 13 September 2011
Author:ialex
Status:ok (Comments)
Tags:
Comment:
* Use local context instead of global variables
* Pass the context to the HTMLForm object
Modified paths:
  • /trunk/phase3/includes/specials/SpecialUploadStash.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialUploadStash.php
@@ -53,11 +53,11 @@
5454 return;
5555 }
5656
57 - if ( !isset( $subPage ) || $subPage === '' ) {
 57+ if ( $subPage === null || $subPage === '' ) {
5858 return $this->showUploads();
 59+ } else {
 60+ return $this->showUpload( $subPage );
5961 }
60 -
61 - return $this->showUpload( $subPage );
6262 }
6363
6464
@@ -68,10 +68,8 @@
6969 * @param $key String: the key of a particular requested file
7070 */
7171 public function showUpload( $key ) {
72 - global $wgOut;
73 -
7472 // prevent callers from doing standard HTML output -- we'll take it from here
75 - $wgOut->disable();
 73+ $this->getOutput()->disable();
7674
7775 try {
7876 $params = $this->parseKey( $key );
@@ -317,7 +315,6 @@
318316 * @param Status : $status - the result of processRequest
319317 */
320318 private function showUploads( $status = null ) {
321 - global $wgOut;
322319 if ( $status === null ) {
323320 $status = Status::newGood();
324321 }
@@ -335,7 +332,7 @@
336333 'default' => true,
337334 'name' => 'clear',
338335 )
339 - ), 'clearStashedUploads' );
 336+ ), $this->getContext(), 'clearStashedUploads' );
340337 $form->setSubmitCallback( array( __CLASS__ , 'tryClearStashedUploads' ) );
341338 $form->setTitle( $this->getTitle() );
342339 $form->setSubmitText( wfMsg( 'uploadstash-clear' ) );
@@ -358,11 +355,11 @@
359356 $this->getTitle( "file/$file" )->getLocalURL() ), $file )
360357 );
361358 }
362 - $wgOut->addHtml( Html::rawElement( 'ul', array(), $fileListItemsHtml ) );
 359+ $this->getOutput()->addHtml( Html::rawElement( 'ul', array(), $fileListItemsHtml ) );
363360 $form->displayForm( $formResult );
364 - $wgOut->addHtml( Html::rawElement( 'p', array(), $refreshHtml ) );
 361+ $this->getOutput()->addHtml( Html::rawElement( 'p', array(), $refreshHtml ) );
365362 } else {
366 - $wgOut->addHtml( Html::rawElement( 'p', array(),
 363+ $this->getOutput()->addHtml( Html::rawElement( 'p', array(),
367364 Html::element( 'span', array(), wfMsg( 'uploadstash-nofiles' ) )
368365 . ' '
369366 . $refreshHtml

Follow-up revisions

RevisionCommit summaryAuthorDate
r98062Follow-up r96958: forgot one use of $wgUserialex10:14, 25 September 2011

Comments

#Comment by Nikerabbit (talk | contribs)   15:29, 13 September 2011
-		), 'clearStashedUploads' );
+		), $this->getContext(), 'clearStashedUploads' );

Was that a bug you fixed or what happened with the old code?

#Comment by Brion VIBBER (talk | contribs)   21:18, 14 September 2011

Seems to be a non-required update for an internal API change in 1.18 (r85957), which isn't clearly marked in the doc comments for the HTMLForm constructor.

If a string is passed as the second parameter (which used to be $messagePrefix but is now $context), then the function internally rearranges to the new order of parameters.

IMO this isn't a super great kind of API change as it looks confusing; optional parameters in the middle of a list are dangerous!

#Comment by Brion VIBBER (talk | contribs)   21:19, 14 September 2011

Hm, also note that when passing the context through, there's no need to call $form->setTitle() as it'll get the title out of the context.

Status & tagging log