r108372 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r108371‎ | r108372 | r108373 >
Date:22:32, 8 January 2012
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
[HistoryAction] Clean up
* Don't build $s as well as $this->buttons for re-use later in getEndBody(), simply use it in both cases.
* Adding a css class for compareselectedversions button just like there is for the revisiondelete button
Modified paths:
  • /trunk/phase3/includes/actions/HistoryAction.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/actions/HistoryAction.php
@@ -409,20 +409,20 @@
410410 $s .= Html::hidden( 'title', $this->getTitle()->getPrefixedDbKey() ) . "\n";
411411 $s .= Html::hidden( 'action', 'historysubmit' ) . "\n";
412412
413 - $s .= '<div>' . $this->submitButton( $this->msg( 'compareselectedversions' )->text(),
414 - array( 'class' => 'historysubmit' ) ) . "\n";
415 -
 413+ // Button container stored in $this->buttons for re-use in getEndBody()
416414 $this->buttons = '<div>';
417415 $this->buttons .= $this->submitButton( $this->msg( 'compareselectedversions' )->text(),
418 - array( 'class' => 'historysubmit' )
 416+ array( 'class' => 'historysubmit mw-history-compareselectedversions-button' )
419417 + Linker::tooltipAndAccesskeyAttribs( 'compareselectedversions' )
420418 ) . "\n";
421419
422420 if ( $this->getUser()->isAllowed( 'deleterevision' ) ) {
423 - $s .= $this->getRevisionButton( 'revisiondelete', 'showhideselectedversions' );
 421+ $this->buttons .= $this->getRevisionButton( 'revisiondelete', 'showhideselectedversions' );
424422 }
425423 $this->buttons .= '</div>';
426 - $s .= '</div><ul id="pagehistory">' . "\n";
 424+
 425+ $s .= $this->buttons;
 426+ $s .= '<ul id="pagehistory">' . "\n";
427427 return $s;
428428 }
429429
@@ -438,7 +438,6 @@
439439 ),
440440 $this->msg( $msg )->text()
441441 ) . "\n";
442 - $this->buttons .= $element;
443442 return $element;
444443 }
445444

Comments

#Comment by EnDumEn (talk | contribs)   15:41, 16 March 2012

I think this broke the fix for bug 24977.

#Comment by Krinkle (talk | contribs)   18:06, 21 March 2012

Hm.. the fix for that bug (r72424) left the code in a counter-intuitive state (having a central buttons string but not using it). Should have a comment explaining it why. Will try to confirm later.

Status & tagging log