r55741 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r55740‎ | r55741 | r55742 >
Date:01:59, 2 September 2009
Author:tstarling
Status:resolved (Comments)
Tags:
Comment:
* Fixed CR r48837. Although using ids[]=x in the query string is wrong and broken, ids[x]=1 works just fine and is a convenient interface.
* Fixed CR r49408: removed JS dependency for multiple revision delete/undelete. Using <button type=submit> we can submit the parameter "action=revisiondelete". The custom action is then distributed to Special:Revisiondelete, which can understand this kind of entry point with only minor changes.
* Also used <button type=submit> to avoid submitting the htmldiff button caption back to the server
* Show the same buttons at the top and bottom of the history page, refactored duplicated code
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/HistoryPage.php (modified) (history)
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialRevisiondelete.php (modified) (history)
  • /trunk/phase3/skins/common/history.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/history.js
——@@ -9,16 +9,6 @@
1010 return radios;
1111 }
1212
13 -function deleteCheck(parent) {
14 - var inputs = parent.getElementsByTagName('input');
15 - for (var i = 0; i < inputs.length; i++) {
16 - if (inputs[i].name == "showhiderevisions") {
17 - return inputs[i];
18 - }
19 - }
20 - return null;
21 -}
22 -
2313 // check selection and tweak visibility/class onclick
2414 function diffcheck() {
2515 var dli = false; // the li where the diff radio is checked
——@@ -90,8 +80,6 @@
9181 function histrowinit() {
9282 var hf = document.getElementById('pagehistory');
9383 if (!hf) return;
94 - var df = document.getElementById('mw-history-revdeleteform');
95 - if( df ) df.style.visibility = 'visible'; // Enable JS form
9684 var lis = hf.getElementsByTagName('li');
9785 for (var i = 0; i < lis.length; i++) {
9886 var inputs = historyRadios(lis[i]);
——@@ -99,28 +87,8 @@
10088 inputs[0].onclick = diffcheck;
10189 inputs[1].onclick = diffcheck;
10290 }
103 - var check = deleteCheck(lis[i]);
104 - if( df && check ) {
105 - check.style.display = 'inline'; // Enable JS form
106 - }
10791 }
10892 diffcheck();
10993 }
11094
111 -// Multi-item revision delete. 'checked' is the *previous* state.
112 -function updateShowHideForm( oldid, checked ) {
113 - var formOldids = document.getElementById('revdel-oldid');
114 - if( !formOldids ) return;
115 - if( checked ) { // add on oldid if checked
116 - if( formOldids.value ) {
117 - formOldids.value += ',' + oldid;
118 - } else {
119 - formOldids.value = oldid;
120 - }
121 - } else if( formOldids.value ) { // remove oldid if unchecked
122 - var reg = new RegExp( '(^|,)'+oldid+'($|,)' );
123 - formOldids.value = formOldids.value.replace( reg, '' );
124 - }
125 -}
126 -
12795 hookEvent("load", histrowinit);
Index: trunk/phase3/includes/Article.php
——@@ -937,7 +937,7 @@
938938 $rcid = $wgRequest->getVal( 'rcid' );
939939 $diffOnly = $wgRequest->getBool( 'diffonly', $wgUser->getOption( 'diffonly' ) );
940940 $purge = $wgRequest->getVal( 'action' ) == 'purge';
941 - $htmldiff = $wgRequest->getVal( 'htmldiff' , false);
 941+ $htmldiff = $wgRequest->getBool( 'htmldiff' );
942942 $unhide = $wgRequest->getInt('unhide') == 1;
943943 $oldid = $this->getOldID();
944944
Index: trunk/phase3/includes/HistoryPage.php
——@@ -260,7 +260,7 @@
261261 * @ingroup Pager
262262 */
263263 class HistoryPager extends ReverseChronologicalPager {
264 - public $lastRow = false, $counter, $historyPage, $title;
 264+ public $lastRow = false, $counter, $historyPage, $title, $buttons;
265265 protected $oldIdChecked;
266266
267267 function __construct( $historyPage, $year='', $month='', $tagFilter = '' ) {
——@@ -323,58 +323,56 @@
324324 $this->oldIdChecked = 0;
325325
326326 $wgOut->wrapWikiMsg( "<div class='mw-history-legend'>\n$1</div>", 'histlegend' );
327 - $s = '';
328 - if( $this->getNumRows() > 1 && $wgUser->isAllowed('deleterevision') ) {
329 - $revdel = SpecialPage::getTitleFor( 'Revisiondelete' );
330 - $s .= Xml::openElement( 'form',
 327+ $s = Xml::openElement( 'form', array( 'action' => $wgScript,
 328+ 'id' => 'mw-history-compare' ) ) . "\n";
 329+ $s .= Xml::hidden( 'title', $this->title->getPrefixedDbKey() ) . "\n";
 330+
 331+ $this->buttons = '<div>';
 332+ if( $wgUser->isAllowed('deleterevision') ) {
 333+ $this->buttons .= Xml::element( 'button',
331334 array(
332 - 'action' => $revdel->getLocalURL( array(
333 - 'type' => 'revision',
334 - 'target' => $this->title->getPrefixedDbKey()
335 - ) ),
336 - 'method' => 'post',
337 - 'id' => 'mw-history-revdeleteform',
338 - 'style' => 'visibility:hidden;float:right;'
339 - )
340 - );
341 - $s .= Xml::hidden( 'ids', '', array('id'=>'revdel-oldid') );
342 - $s .= Xml::submitButton( wfMsg( 'showhideselectedversions' ) );
343 - $s .= Xml::closeElement( 'form' );
 335+ 'type' => 'submit',
 336+ 'name' => 'action',
 337+ 'value' => 'revisiondelete',
 338+ 'style' => 'float: right',
 339+ ),
 340+ wfMsg( 'showhideselectedversions' )
 341+ ) . "\n";
344342 }
345 - $s .= Xml::openElement( 'form', array( 'action' => $wgScript,
346 - 'id' => 'mw-history-compare' ) );
347 - $s .= Xml::hidden( 'title', $this->title->getPrefixedDbKey() );
348343 if( $wgEnableHtmlDiff ) {
349 - $s .= $this->submitButton( wfMsg( 'visualcomparison'),
 344+ $this->buttons .= Xml::element( 'button',
350345 array(
351 - 'name' => 'htmldiff',
 346+ 'type' => 'submit',
 347+ 'name' => 'htmldiff',
 348+ 'value' => '1',
352349 'class' => 'historysubmit',
353350 'accesskey' => wfMsg( 'accesskey-visualcomparison' ),
354351 'title' => wfMsg( 'tooltip-compareselectedversions' ),
355 - )
356 - );
357 - $s .= $this->submitButton( wfMsg( 'wikicodecomparison'),
 352+ ),
 353+ wfMsg( 'visualcomparison')
 354+ ) . "\n";
 355+ $this->buttons .= $this->submitButton( wfMsg( 'wikicodecomparison'),
358356 array(
359357 'class' => 'historysubmit',
360358 'accesskey' => wfMsg( 'accesskey-compareselectedversions' ),
361359 'title' => wfMsg( 'tooltip-compareselectedversions' ),
362360 )
363 - );
 361+ ) . "\n";
364362 } else {
365 - $s .= $this->submitButton( wfMsg( 'compareselectedversions'),
 363+ $this->buttons .= $this->submitButton( wfMsg( 'compareselectedversions'),
366364 array(
367365 'class' => 'historysubmit',
368366 'accesskey' => wfMsg( 'accesskey-compareselectedversions' ),
369367 'title' => wfMsg( 'tooltip-compareselectedversions' ),
370368 )
371 - );
 369+ ) . "\n";
372370 }
373 - $s .= '<ul id="pagehistory">' . "\n";
 371+ $this->buttons .= '</div>';
 372+ $s .= $this->buttons . '<ul id="pagehistory">' . "\n";
374373 return $s;
375374 }
376375
377376 function getEndBody() {
378 -
379377 if( $this->lastRow ) {
380378 $latest = $this->counter == 1 && $this->mIsFirst;
381379 $firstInList = $this->counter == 1;
——@@ -394,34 +392,8 @@
395393 } else {
396394 $s = '';
397395 }
398 -
399 - global $wgEnableHtmlDiff;
400 - $s .= '</ul>';
401 - if( $wgEnableHtmlDiff ) {
402 - $s .= $this->submitButton( wfMsg( 'visualcomparison'),
403 - array(
404 - 'name' => 'htmldiff',
405 - 'class' => 'historysubmit',
406 - 'accesskey' => wfMsg( 'accesskey-visualcomparison' ),
407 - 'title' => wfMsg( 'tooltip-compareselectedversions' ),
408 - )
409 - );
410 - $s .= $this->submitButton( wfMsg( 'wikicodecomparison'),
411 - array(
412 - 'class' => 'historysubmit',
413 - 'accesskey' => wfMsg( 'accesskey-compareselectedversions' ),
414 - 'title' => wfMsg( 'tooltip-compareselectedversions' ),
415 - )
416 - );
417 - } else {
418 - $s .= $this->submitButton( wfMsg( 'compareselectedversions'),
419 - array(
420 - 'class' => 'historysubmit',
421 - 'accesskey' => wfMsg( 'accesskey-compareselectedversions' ),
422 - 'title' => wfMsg( 'tooltip-compareselectedversions' ),
423 - )
424 - );
425 - }
 396+ $s .= "</ul>\n";
 397+ $s .= $this->buttons;
426398 $s .= '</form>';
427399 return $s;
428400 }
——@@ -470,26 +442,19 @@
471443 $s = "($curlink) ($lastlink) $diffButtons";
472444
473445 if( $wgUser->isAllowed( 'deleterevision' ) ) {
474 - // Hide JS by default for non-JS browsing
475 - $hidden = array( 'style' => 'display:none' );
476446 // If revision was hidden from sysops
477447 if( !$rev->userCan( Revision::DELETED_RESTRICTED ) ) {
478 - $del = Xml::check( 'deleterevisions', false,
479 - $hidden + array('disabled' => 'disabled') );
 448+ $del = Xml::check( 'deleterevisions', false, array('disabled' => 'disabled') );
480449 $del .= Xml::tags( 'span', array( 'class'=>'mw-revdelundel-link' ),
481450 '(' . $this->historyPage->message['rev-delundel'] . ')' );
482451 // Otherwise, show the link...
483452 } else {
484453 $id = $rev->getId();
485 - $jsCall = "updateShowHideForm($id,this.checked)";
486 - $del = Xml::check( 'showhiderevisions', false,
487 - $hidden + array(
488 - 'onchange' => $jsCall,
489 - 'id' => "mw-revdel-$id" ) );
 454+ $del = Xml::check( 'showhiderevisions', false, array( 'name' => "ids[$id]" ) );
490455 $query = array(
491456 'type' => 'revision',
492457 'target' => $this->title->getPrefixedDbkey(),
493 - 'ids' => $rev->getId() );
 458+ 'ids' => $id );
494459 $del .= $this->getSkin()->revDeleteLink( $query,
495460 $rev->isDeleted( Revision::DELETED_RESTRICTED ) );
496461 }
Index: trunk/phase3/includes/Wiki.php
——@@ -535,6 +535,11 @@
536536 $history = new HistoryPage( $article );
537537 $history->history();
538538 break;
 539+ case 'revisiondelete':
 540+ # For show/hide submission from history page
 541+ $special = SpecialPage::getPage( 'Revisiondelete' );
 542+ $special->execute( '' );
 543+ break;
539544 default:
540545 if( wfRunHooks( 'UnknownAction', array( $action, $article ) ) ) {
541546 $output->showErrorPage( 'nosuchaction', 'nosuchactiontext' );
Index: trunk/phase3/includes/specials/SpecialRevisiondelete.php
——@@ -107,11 +107,26 @@
108108 $this->outputHeader();
109109 $this->submitClicked = $wgRequest->wasPosted() && $wgRequest->getBool( 'wpSubmit' );
110110 # Handle our many different possible input types.
111 - # Use CSV, since the cgi handling will break on arrays.
112 - $this->ids = explode( ',', $wgRequest->getVal('ids') );
 111+ $ids = $wgRequest->getVal( 'ids' );
 112+ if ( !is_null( $ids ) ) {
 113+ # Allow CSV, for backwards compatibility, or a single ID for show/hide links
 114+ $this->ids = explode( ',', $ids );
 115+ } else {
 116+ # Array input
 117+ $this->ids = array_keys( $wgRequest->getArray( 'ids' ) );
 118+ }
 119+ $this->ids = array_map( 'intval', $this->ids );
113120 $this->ids = array_unique( array_filter( $this->ids ) );
114 - $this->targetObj = Title::newFromText( $wgRequest->getText( 'target' ) );
115121
 122+ if ( $wgRequest->getVal( 'action' ) == 'revisiondelete' ) {
 123+ # For show/hide form submission from history page
 124+ $this->targetObj = $GLOBALS['wgTitle'];
 125+ $this->typeName = 'revision';
 126+ } else {
 127+ $this->typeName = $wgRequest->getVal( 'type' );
 128+ $this->targetObj = Title::newFromText( $wgRequest->getText( 'target' ) );
 129+ }
 130+
116131 # For reviewing deleted files...
117132 $this->archiveName = $wgRequest->getVal( 'file' );
118133 $this->token = $wgRequest->getVal( 'token' );
——@@ -120,7 +135,6 @@
121136 return;
122137 }
123138
124 - $this->typeName = $wgRequest->getVal( 'type' );
125139 if ( isset( self::$deprecatedTypeMap[$this->typeName] ) ) {
126140 $this->typeName = self::$deprecatedTypeMap[$this->typeName];
127141 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r55762* Fix for r55741 - selecting no revisions made PHP notices...aaron21:46, 2 September 2009

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r48837Fix url params for multi-item deletion by converting to CVSaaron17:52, 25 March 2009
r49408(bug 16607) Added convenience checkboxes for revisiondelete to history pagesaaron17:38, 11 April 2009

Comments

#Comment by Aaron Schulz (talk | contribs)   03:15, 2 September 2009

"<button type="submit" name="action" value="revisiondelete" ..."

Didn't know that worked, nice :)

#Comment by Mr.Z-man (talk | contribs)   01:49, 3 October 2009

This doesn't work correctly in IE - bugzilla:20966. IE uses the contents of the tag as the value, rather than the value attrib.

#Comment by Tim Starling (talk | contribs)   06:31, 6 October 2009

Fixed in r57415.

Status & tagging log