r47059 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r47058‎ | r47059 | r47060 >
Date:22:27, 9 February 2009
Author:aaron
Status:deferred (Comments)
Tags:
Comment:
Tweaks for bug 17060
* Don't show links to deleted revs in history for anyone (consistency)
* Always hide deleted content from all users; it can be viewed via (show/hide) links
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/PageHistory.php (modified) (history)
  • /trunk/phase3/includes/diff/DifferenceEngine.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialRevisiondelete.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/diff/DifferenceEngine.php
——@@ -28,6 +28,8 @@
2929 var $mRevisionsLoaded = false; // Have the revisions been loaded
3030 var $mTextLoaded = 0; // How many text blobs have been loaded, 0, 1 or 2?
3131 var $htmldiff;
 32+
 33+ protected $unhide = false;
3234 /**#@-*/
3335
3436 /**
——@@ -38,8 +40,9 @@
3941 * @param $rcid Integer: ??? FIXME (default 0)
4042 * @param $refreshCache boolean If set, refreshes the diff cache
4143 * @param $htmldiff boolean If set, output using HTMLDiff instead of raw wikicode diff
 44+ * @param $unhide boolean If set, allow viewing deleted revs
4245 */
43 - function __construct( $titleObj = null, $old = 0, $new = 0, $rcid = 0, $refreshCache = false , $htmldiff = false) {
 46+ function __construct( $titleObj = null, $old = 0, $new = 0, $rcid = 0, $refreshCache = false , $htmldiff = false, $unhide = false ) {
4447 $this->mTitle = $titleObj;
4548 wfDebug("DifferenceEngine old '$old' new '$new' rcid '$rcid'\n");
4649
——@@ -67,6 +70,7 @@
6871 $this->mRcidMarkPatrolled = intval($rcid); # force it to be an integer
6972 $this->mRefreshCache = $refreshCache;
7073 $this->htmldiff = $htmldiff;
 74+ $this->unhide = $unhide;
7175 }
7276
7377 function getTitle() {
——@@ -254,16 +258,18 @@
255259 }
256260
257261 $oldHeader = '<div id="mw-diff-otitle1"><strong>'.$this->mOldtitle.'</strong></div>' .
258 - '<div id="mw-diff-otitle2">' . $sk->revUserTools( $this->mOldRev, true ) . "</div>" .
259 - '<div id="mw-diff-otitle3">' . $oldminor . $sk->revComment( $this->mOldRev, !$diffOnly, true ) . $ldel . "</div>" .
 262+ '<div id="mw-diff-otitle2">' . $sk->revUserTools( $this->mOldRev, !$this->unhide ) . "</div>" .
 263+ '<div id="mw-diff-otitle3">' . $oldminor . $sk->revComment( $this->mOldRev, !$diffOnly, !$this->unhide ).$ldel."</div>" .
260264 '<div id="mw-diff-otitle4">' . $prevlink .'</div>';
261265 $newHeader = '<div id="mw-diff-ntitle1"><strong>'.$this->mNewtitle.'</strong></div>' .
262 - '<div id="mw-diff-ntitle2">' . $sk->revUserTools( $this->mNewRev, true ) . " $rollback</div>" .
263 - '<div id="mw-diff-ntitle3">' . $newminor . $sk->revComment( $this->mNewRev, !$diffOnly, true ) . $rdel . "</div>" .
 266+ '<div id="mw-diff-ntitle2">' . $sk->revUserTools( $this->mNewRev, !$this->unhide ) . " $rollback</div>" .
 267+ '<div id="mw-diff-ntitle3">' . $newminor . $sk->revComment( $this->mNewRev, !$diffOnly, !$this->unhide ).$rdel."</div>" .
264268 '<div id="mw-diff-ntitle4">' . $nextlink . $patrol . '</div>';
265269
266 - # Output the diff
267 - if( !$this->mOldRev->userCan(Revision::DELETED_TEXT) || !$this->mNewRev->userCan(Revision::DELETED_TEXT) ) {
 270+ # Output the diff if allowed
 271+ $allowed = $this->mOldRev->userCan(Revision::DELETED_TEXT) && $this->mNewRev->userCan(Revision::DELETED_TEXT);
 272+ $deleted = $this->mOldRev->isDeleted(Revision::DELETED_TEXT) || $this->mNewRev->isDeleted(Revision::DELETED_TEXT);
 273+ if( $deleted && (!$this->unhide || !$allowed) ) {
268274 $this->showDiffStyle();
269275 $multi = $this->getMultiNotice();
270276 $wgOut->addHTML( $this->addHeader( '', $oldHeader, $newHeader, $multi ) );
Index: trunk/phase3/includes/Article.php
——@@ -780,15 +780,17 @@
781781 }
782782 $wgOut->setRobotPolicy( $policy );
783783
 784+ # Allow admins to see deleted content if explicitly requested
 785+ $delId = $diff ? $diff : $oldid;
 786+ $unhide = $wgRequest->getInt('unhide') == 1 && $wgUser->matchEditToken( $wgRequest->getVal('token'), $delId );
784787 # If we got diff and oldid in the query, we want to see a
785788 # diff page instead of the article.
786789
787790 if( !is_null( $diff ) ) {
788791 $wgOut->setPageTitle( $this->mTitle->getPrefixedText() );
789792
790 - $diff = $wgRequest->getVal( 'diff' );
791793 $htmldiff = $wgRequest->getVal( 'htmldiff' , false);
792 - $de = new DifferenceEngine( $this->mTitle, $oldid, $diff, $rcid, $purge, $htmldiff);
 794+ $de = new DifferenceEngine( $this->mTitle, $oldid, $diff, $rcid, $purge, $htmldiff, $unhide );
793795 // DifferenceEngine directly fetched the revision:
794796 $this->mRevIdFetched = $de->mNewid;
795797 $de->showDiffPage( $diffOnly );
——@@ -913,8 +915,9 @@
914916 // FIXME: This would be a nice place to load the 'no such page' text.
915917 } else {
916918 $this->setOldSubtitle( isset($this->mOldId) ? $this->mOldId : $oldid );
 919+ # Allow admins to see deleted content if explicitly requested
917920 if( $this->mRevision->isDeleted( Revision::DELETED_TEXT ) ) {
918 - if( !$this->mRevision->userCan( Revision::DELETED_TEXT ) ) {
 921+ if( !$unhide || !$this->mRevision->userCan(Revision::DELETED_TEXT) ) {
919922 $wgOut->addWikiMsg( 'rev-deleted-text-permission' );
920923 $wgOut->setPageTitle( $this->mTitle->getPrefixedText() );
921924 wfProfileOut( __METHOD__ );
——@@ -2971,7 +2974,7 @@
29722975 * @param $oldid String: revision ID of this article revision
29732976 */
29742977 public function setOldSubtitle( $oldid = 0 ) {
2975 - global $wgLang, $wgOut, $wgUser;
 2978+ global $wgLang, $wgOut, $wgUser, $wgRequest;
29762979
29772980 if( !wfRunHooks( 'DisplayOldSubtitle', array( &$this, &$oldid ) ) ) {
29782981 return;
——@@ -3022,16 +3025,17 @@
30233026 }
30243027 $cdel = "(<small>$cdel</small>) ";
30253028 }
3026 - # Show user links if allowed to see them. Normally they
3027 - # are hidden regardless, but since we can already see the text here...
3028 - $userlinks = $sk->revUserTools( $revision, false );
 3029+ $unhide = $wgRequest->getInt('unhide') == 1 && $wgUser->matchEditToken( $wgRequest->getVal('token'), $oldid );
 3030+ # Show user links if allowed to see them. If hidden, then show them only if requested...
 3031+ $userlinks = $sk->revUserTools( $revision, !$unhide );
30293032
30303033 $m = wfMsg( 'revision-info-current' );
30313034 $infomsg = $current && !wfEmptyMsg( 'revision-info-current', $m ) && $m != '-'
30323035 ? 'revision-info-current'
30333036 : 'revision-info';
30343037
3035 - $r = "\n\t\t\t\t<div id=\"mw-{$infomsg}\">" . wfMsgExt( $infomsg, array( 'parseinline', 'replaceafter' ), $td, $userlinks, $revision->getID() ) . "</div>\n" .
 3038+ $r = "\n\t\t\t\t<div id=\"mw-{$infomsg}\">" . wfMsgExt( $infomsg, array( 'parseinline', 'replaceafter' ),
 3039+ $td, $userlinks, $revision->getID() ) . "</div>\n" .
30363040
30373041 "\n\t\t\t\t<div id=\"mw-revision-nav\">" . $cdel . wfMsgHtml( 'revision-nav', $prevdiff,
30383042 $prevlink, $lnk, $curdiff, $nextlink, $nextdiff ) . "</div>\n\t\t\t";
Index: trunk/phase3/includes/specials/SpecialRevisiondelete.php
——@@ -542,15 +542,16 @@
543543 * @returns string
544544 */
545545 private function historyLine( $rev ) {
546 - global $wgLang;
 546+ global $wgLang, $wgUser;
547547
548548 $date = $wgLang->timeanddate( $rev->getTimestamp() );
549549 $difflink = $del = '';
550550 // Live revisions
551551 if( $this->deleteKey=='oldid' ) {
552 - $revlink = $this->skin->makeLinkObj( $this->page, $date, 'oldid=' . $rev->getId() );
 552+ $tokenParams = '&unhide=1&token='.urlencode( $wgUser->editToken( $rev->getId() ) );
 553+ $revlink = $this->skin->makeLinkObj( $this->page, $date, 'oldid='.$rev->getId() . $tokenParams );
553554 $difflink = '(' . $this->skin->makeKnownLinkObj( $this->page, wfMsgHtml('diff'),
554 - 'diff=' . $rev->getId() . '&oldid=prev' ) . ')';
 555+ 'diff=' . $rev->getId() . '&oldid=prev' . $tokenParams ) . ')';
555556 // Archived revisions
556557 } else {
557558 $undelete = SpecialPage::getTitleFor( 'Undelete' );
——@@ -560,7 +561,7 @@
561562 $difflink = '(' . $this->skin->makeKnownLinkObj( $undelete, wfMsgHtml('diff'),
562563 "target=$target&diff=prev&timestamp=" . $rev->getTimestamp() ) . ')';
563564 }
564 -
 565+ // Check permissions; items may be "suppressed"
565566 if( $rev->isDeleted(Revision::DELETED_TEXT) ) {
566567 $revlink = '<span class="history-deleted">'.$revlink.'</span>';
567568 $del = ' <tt>' . wfMsgHtml( 'deletedrev' ) . '</tt>';
——@@ -569,8 +570,10 @@
570571 $difflink = '(' . wfMsgHtml('diff') . ')';
571572 }
572573 }
 574+ $userlink = $this->skin->revUserLink( $rev );
 575+ $comment = $this->skin->revComment( $rev );
573576
574 - return "<li> $difflink $revlink ".$this->skin->revUserLink( $rev )." ".$this->skin->revComment( $rev )."$del</li>";
 577+ return "<li> $difflink $revlink $userlink $comment{$del}</li>";
575578 }
576579
577580 /**
Index: trunk/phase3/includes/PageHistory.php
——@@ -315,11 +315,11 @@
316316 $s .= " $link";
317317 $s .= " <span class='history-user'>" . $this->mSkin->revUserTools( $rev, true ) . "</span>";
318318
319 - if( $row->rev_minor_edit ) {
 319+ if( $rev->isMinor() ) {
320320 $s .= ' ' . Xml::element( 'span', array( 'class' => 'minor' ), wfMsg( 'minoreditletter') );
321321 }
322322
323 - if( !is_null( $size = $rev->getSize() ) && $rev->userCan( Revision::DELETED_TEXT ) ) {
 323+ if( !is_null( $size = $rev->getSize() ) && !$rev->isDeleted( Revision::DELETED_TEXT ) ) {
324324 $s .= ' ' . $this->mSkin->formatRevisionSize( $size );
325325 }
326326
——@@ -381,15 +381,11 @@
382382 function revLink( $rev ) {
383383 global $wgLang;
384384 $date = $wgLang->timeanddate( wfTimestamp(TS_MW, $rev->getTimestamp()), true );
385 - if( $rev->userCan( Revision::DELETED_TEXT ) ) {
386 - $link = $this->mSkin->makeKnownLinkObj(
387 - $this->mTitle, $date, "oldid=" . $rev->getId() );
 385+ if( !$rev->isDeleted( Revision::DELETED_TEXT ) ) {
 386+ $link = $this->mSkin->makeKnownLinkObj( $this->mTitle, $date, "oldid=" . $rev->getId() );
388387 } else {
389 - $link = $date;
 388+ $link = '<span class="history-deleted">' . $date . '</span>';
390389 }
391 - if( $rev->isDeleted( Revision::DELETED_TEXT ) ) {
392 - return '<span class="history-deleted">' . $link . '</span>';
393 - }
394390 return $link;
395391 }
396392
——@@ -401,7 +397,7 @@
402398 */
403399 function curLink( $rev, $latest ) {
404400 $cur = $this->message['cur'];
405 - if( $latest || !$rev->userCan( Revision::DELETED_TEXT ) ) {
 401+ if( $latest || $rev->isDeleted( Revision::DELETED_TEXT ) ) {
406402 return $cur;
407403 } else {
408404 return $this->mSkin->makeKnownLinkObj( $this->mTitle, $cur,
——@@ -427,7 +423,7 @@
428424 # Next row probably exists but is unknown, use an oldid=prev link
429425 return $this->mSkin->makeKnownLinkObj( $this->mTitle, $last,
430426 "diff=" . $prevRev->getId() . "&oldid=prev" );
431 - } elseif( !$prevRev->userCan(Revision::DELETED_TEXT) || !$nextRev->userCan(Revision::DELETED_TEXT) ) {
 427+ } elseif( $prevRev->isDeleted(Revision::DELETED_TEXT) || $nextRev->isDeleted(Revision::DELETED_TEXT) ) {
432428 return $last;
433429 } else {
434430 return $this->mSkin->makeKnownLinkObj( $this->mTitle, $last,
——@@ -454,7 +450,7 @@
455451 $checkmark = array( 'checked' => 'checked' );
456452 } else {
457453 # Check visibility of old revisions
458 - if( !$rev->userCan( Revision::DELETED_TEXT ) ) {
 454+ if( $rev->isDeleted( Revision::DELETED_TEXT ) ) {
459455 $radio['disabled'] = 'disabled';
460456 $checkmark = array(); // We will check the next possible one
461457 } else if( $counter == 2 || !$this->mOldIdChecked ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r50788(bug 17060) Renders edit comments the same for all users wrt to revdeleteaaron19:19, 19 May 2009

Comments

#Comment by Werdna (talk | contribs)   22:55, 12 February 2009

Looks okay in implementation, but are we sure that we don't want admins being able to view this stuff in the ordinary diff screen? It seems inconvenient and annoying to restrict them from it, for no discernible purpose.

Also, while this isn't 'authentication' (just UI), if you're going to do authentication at all, it should be stronger than checking an edit token. 

+		$unhide = $wgRequest->getInt('unhide') == 1 && $wgUser->matchEditToken( $wgRequest->getVal('token'), $delId );

Status & tagging log