r59949 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r59948‎ | r59949 | r59950 >
Date:06:57, 11 December 2009
Author:aaron
Status:resolved (Comments)
Tags:
Comment:
(bug 20928) Added tri-state form for RevisionDelete when there are multiple items.
Modified paths:
  • /trunk/phase3/includes/specials/SpecialRevisiondelete.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialRevisiondelete.php
——@@ -181,6 +181,7 @@
182182 } else {
183183 $this->showForm();
184184 }
 185+
185186 $qc = $this->getLogQueryCond();
186187 # Show relevant lines from the deletion log
187188 $wgOut->addHTML( "<h2>" . htmlspecialchars( LogPage::logName( 'delete' ) ) . "</h2>\n" );
——@@ -330,7 +331,6 @@
331332 $this->targetObj->getPrefixedText(), count( $this->ids ) );
332333 }
333334
334 - $bitfields = 0;
335335 $wgOut->addHTML( "<ul>" );
336336
337337 $where = $revObjs = array();
——@@ -348,7 +348,6 @@
349349 $UserAllowed = false;
350350 }
351351 $numRevisions++;
352 - $bitfields |= $item->getBits();
353352 $wgOut->addHTML( $item->getHTML() );
354353 }
355354
——@@ -370,7 +369,7 @@
371370 'action' => $this->getTitle()->getLocalUrl( array( 'action' => 'submit' ) ),
372371 'id' => 'mw-revdel-form-revisions' ) ) .
373372 Xml::fieldset( wfMsg( 'revdelete-legend' ) ) .
374 - $this->buildCheckBoxes( $bitfields ) .
 373+ $this->buildCheckBoxes() .
375374 Xml::openElement( 'table' ) .
376375 "<tr>\n" .
377376 '<td class="mw-label">' .
——@@ -438,20 +437,55 @@
439438 }
440439
441440 /**
442 - * @param $bitfields Interger: aggregate bitfield of all the bitfields
443441 * @return String: HTML
444442 */
445 - protected function buildCheckBoxes( $bitfields ) {
 443+ protected function buildCheckBoxes() {
 444+ global $wgRequest;
 445+
446446 $html = '<table>';
447 - // FIXME: all items checked for just one rev are checked, even if not set for the others
448 - foreach( $this->checks as $item ) {
449 - list( $message, $name, $field ) = $item;
450 - $innerHTML = Xml::checkLabel( wfMsg($message), $name, $name, $bitfields & $field );
451 - if( $field == Revision::DELETED_RESTRICTED )
452 - $innerHTML = "<b>$innerHTML</b>";
453 - $line = Xml::tags( 'td', array( 'class' => 'mw-input' ), $innerHTML );
454 - $html .= '<tr>' . $line . "</tr>\n";
 447+ // If there is just one item, use checkboxes
 448+ $list = $this->getList();
 449+ if( $list->length() == 1 ) {
 450+ $list->reset();
 451+ $bitfield = $list->current()->getBits(); // existing field
 452+ if( $this->submitClicked ) {
 453+ $bitfield = $this->extractBitfield( $this->extractBitParams($wgRequest), $bitfield );
 454+ }
 455+ foreach( $this->checks as $item ) {
 456+ list( $message, $name, $field ) = $item;
 457+ $innerHTML = Xml::checkLabel( wfMsg($message), $name, $name, $bitfield & $field );
 458+ if( $field == Revision::DELETED_RESTRICTED )
 459+ $innerHTML = "<b>$innerHTML</b>";
 460+ $line = Xml::tags( 'td', array( 'class' => 'mw-input' ), $innerHTML );
 461+ $html .= "<tr>$line</tr>\n";
 462+ }
 463+ // Otherwise, use tri-state radios
 464+ } else {
 465+ $html .= '<tr>';
 466+ $html .= '<th>'.wfMsgHtml('revdelete-radio-same').'</th><th>&nbsp;</th>';
 467+ $html .= '<th>'.wfMsgHtml('revdelete-radio-unset').'</th><th>&nbsp;</th>';
 468+ $html .= '<th>'.wfMsgHtml('revdelete-radio-set').'</th><th>&nbsp;</th>';
 469+ $html .= '<th></th></tr>';
 470+ foreach( $this->checks as $item ) {
 471+ list( $message, $name, $field ) = $item;
 472+ // If there are several items, use third state by default...
 473+ if( $this->submitClicked ) {
 474+ $selected = $wgRequest->getInt( $name, 0 /* unchecked */ );
 475+ } else {
 476+ $selected = -1; // use existing field
 477+ }
 478+ $line = '<td>' . Xml::radio( $name, -1, $selected == -1 ) . '</td><td>&nbsp;</td>';
 479+ $line .= '<td>' . Xml::radio( $name, 0, $selected == 0 ) . '</td><td>&nbsp;</td>';
 480+ $line .= '<td>' . Xml::radio( $name, 1, $selected == 1 ) . '</td><td>&nbsp;</td>';
 481+ $label = wfMsgHtml($message);
 482+ if( $field == Revision::DELETED_RESTRICTED ) {
 483+ $label = "<b>$label</b>";
 484+ }
 485+ $line .= "<td>$label</td>";
 486+ $html .= "<tr>$line</tr>\n";
 487+ }
455488 }
 489+
456490 $html .= '</table>';
457491 return $html;
458492 }
——@@ -467,7 +501,7 @@
468502 $wgOut->addWikiMsg( 'sessionfailure' );
469503 return false;
470504 }
471 - $bitfield = $this->extractBitfield( $request );
 505+ $bitParams = $this->extractBitParams( $request );
472506 $listReason = $request->getText( 'wpRevDeleteReasonList', 'other' ); // from dropdown
473507 $comment = $listReason;
474508 if( $comment != 'other' && $this->otherReason != '' ) {
——@@ -477,12 +511,12 @@
478512 $comment = $this->otherReason;
479513 }
480514 # Can the user set this field?
481 - if( $bitfield & Revision::DELETED_RESTRICTED && !$wgUser->isAllowed('suppressrevision') ) {
 515+ if( $bitParams[Revision::DELETED_RESTRICTED]==1 && !$wgUser->isAllowed('suppressrevision') ) {
482516 $wgOut->permissionRequired( 'suppressrevision' );
483517 return false;
484518 }
485519 # If the save went through, go to success message...
486 - $status = $this->save( $bitfield, $comment, $this->targetObj );
 520+ $status = $this->save( $bitParams, $comment, $this->targetObj );
487521 if ( $status->isGood() ) {
488522 $this->success();
489523 return true;
——@@ -515,32 +549,52 @@
516550 }
517551
518552 /**
519 - * Put together a rev_deleted bitfield from the submitted checkboxes
 553+ * Put together an array that contains -1, 0, or the *_deleted const for each bit
520554 * @param $request WebRequest
521 - * @return Integer
 555+ * @return array
522556 */
523 - protected function extractBitfield( $request ) {
524 - $bitfield = 0;
 557+ protected function extractBitParams( $request ) {
 558+ $bitfield = array();
525559 foreach( $this->checks as $item ) {
526560 list( /* message */ , $name, $field ) = $item;
527 - if( $request->getCheck( $name ) ) {
528 - $bitfield |= $field;
 561+ $val = $request->getInt( $name, 0 /* unchecked */ );
 562+ if( $val < -1 || $val > 1) {
 563+ $val = -1; // -1 for existing value
529564 }
 565+ $bitfield[$field] = $val;
530566 }
 567+ if( !isset($bitfield[Revision::DELETED_RESTRICTED]) ) {
 568+ $bitfield[Revision::DELETED_RESTRICTED] = 0;
 569+ }
531570 return $bitfield;
532571 }
 572+
 573+ /**
 574+ * Put together a rev_deleted bitfield
 575+ * @param $bitPars array extractBitParams() params
 576+ * @param $oldfield int current bitfield
 577+ * @return array
 578+ */
 579+ public static function extractBitfield( $bitPars, $oldfield ) {
 580+ // Build the actual new rev_deleted bitfield
 581+ $newBits = 0;
 582+ foreach( $bitPars as $const => $val ) {
 583+ if( $val == 1 ) {
 584+ $newBits |= $const; // $const is the *_deleted const
 585+ } else if( $val == -1 ) {
 586+ $newBits |= ($oldfield & $const); // use existing
 587+ }
 588+ }
 589+ return $newBits;
 590+ }
533591
534592 /**
535593 * Do the write operations. Simple wrapper for RevDel_*List::setVisibility().
536594 */
537595 protected function save( $bitfield, $reason, $title ) {
538 - // Don't allow simply locking the interface for no reason
539 - if( $bitfield == Revision::DELETED_RESTRICTED ) {
540 - return Status::newFatal( 'revdelete-only-restricted' );
541 - }
542 - return $this->getList()->setVisibility( array(
543 - 'value' => $bitfield,
544 - 'comment' => $reason ) );
 596+ return $this->getList()->setVisibility(
 597+ array( 'value' => $bitfield, 'comment' => $reason )
 598+ );
545599 }
546600 }
547601
——@@ -711,7 +765,7 @@
712766 * @return Status
713767 */
714768 public function setVisibility( $params ) {
715 - $newBits = $params['value'];
 769+ $bitPars = $params['value'];
716770 $comment = $params['comment'];
717771
718772 $this->res = false;
——@@ -728,8 +782,10 @@
729783 $item = $this->current();
730784 unset( $missing[ $item->getId() ] );
731785
732 - // Make error messages less vague
733786 $oldBits = $item->getBits();
 787+ // Build the actual new rev_deleted bitfield
 788+ $newBits = SpecialRevisionDelete::extractBitfield( $bitPars, $oldBits );
 789+
734790 if ( $oldBits == $newBits ) {
735791 $status->warning( 'revdelete-no-change', $item->formatDate(), $item->formatTime() );
736792 $status->failCount++;
——@@ -750,11 +806,18 @@
751807 }
752808 if ( !$item->canView() ) {
753809 // Cannot access this revision
754 - $msg = $opType == 'show' ? 'revdelete-show-no-access' : 'revdelete-modify-no-access';
 810+ $msg = ($opType == 'show') ?
 811+ 'revdelete-show-no-access' : 'revdelete-modify-no-access';
755812 $status->error( $msg, $item->formatDate(), $item->formatTime() );
756813 $status->failCount++;
757814 continue;
758815 }
 816+ // Cannot just "hide from Sysops" without hiding any fields
 817+ if( $newBits == Revision::DELETED_RESTRICTED ) {
 818+ $status->warning( 'revdelete-only-restricted', $item->formatDate(), $item->formatTime() );
 819+ $status->failCount++;
 820+ continue;
 821+ }
759822
760823 // Update the revision
761824 $ok = $item->setBits( $newBits );
——@@ -920,6 +983,17 @@
921984 $this->initCurrent();
922985 return $this->current;
923986 }
 987+
 988+ /**
 989+ * Get the number of items in the list.
 990+ */
 991+ public function length() {
 992+ if( !$this->res ) {
 993+ return 0;
 994+ } else {
 995+ return $this->res->numRows();
 996+ }
 997+ }
924998
925999 /**
9261000 * Clear any data structures needed for doPreCommitUpdates() and doPostCommitUpdates()
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -1477,12 +1477,15 @@
14781478 *: ''home addresses and telephone numbers, social security numbers, etc.''",
14791479 'revdelete-legend' => 'Set visibility restrictions',
14801480 'revdelete-hide-text' => 'Hide revision text',
 1481+'revdelete-hide-image' => 'Hide file content',
14811482 'revdelete-hide-name' => 'Hide action and target',
14821483 'revdelete-hide-comment' => 'Hide edit comment',
14831484 'revdelete-hide-user' => "Hide editor's username/IP address",
14841485 'revdelete-hide-restricted' => 'Suppress data from administrators as well as others',
 1486+'revdelete-radio-same' => '(leave)',
 1487+'revdelete-radio-set' => 'Yes',
 1488+'revdelete-radio-unset' => 'No',
14851489 'revdelete-suppress' => 'Suppress data from administrators as well as others',
1486 -'revdelete-hide-image' => 'Hide file content',
14871490 'revdelete-unsuppress' => 'Remove restrictions on restored revisions',
14881491 'revdelete-log' => 'Reason for deletion:',
14891492 'revdelete-submit' => 'Apply to selected {{PLURAL:$1|revision|revisions}}',
——@@ -1516,7 +1519,7 @@
15171520 'revdelete-no-change' => "'''Warning:''' the item dated $2, $1 already had the requested visibility settings.",
15181521 'revdelete-concurrent-change' => 'Error modifying the item dated $2, $1: its status appears to have been changed by someone else while you attempted to modify it.
15191522 Please check the logs.',
1520 -'revdelete-only-restricted' => 'You cannot suppress items from view by administrators without also selecting one of the other suppression options.',
 1523+'revdelete-only-restricted' => 'Error hiding the item dated $2, $1: you cannot suppress items from view by administrators without also selecting one of the other visibility options.',
15211524 'revdelete-reason-dropdown' => '*Common delete reasons
15221525 ** Copyright violation
15231526 ** Inappropriate personal information

Follow-up revisions

RevisionCommit summaryAuthorDate
r60252Clearer (if slightly longer) text to indicate that the property is not to be ...tstarling04:58, 21 December 2009
r61990Per Tim, fix r59949: use CSS padding (margin doesn't seem to work on Firefox ...ialex21:07, 4 February 2010

Comments

#Comment by Tim Starling (talk | contribs)   04:53, 21 December 2009

Please style that table properly, with CSS margins instead of fake empty columns. If the radio button columns had align:center they'd look nicer. Otherwise, it's a reasonable solution to bug 20928, I suppose.

#Comment by IAlex (talk | contribs)   21:08, 4 February 2010

Done in r61990.

Status & tagging log