r41222 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r41221‎ | r41222 | r41223 >
Date:09:44, 24 September 2008
Author:tstarling
Status:old
Tags:
Comment:
Fixes for r41154 and r41155:
* Boolean parameters are widely accepted to reduce readability. Replaced the new boolean parameters with class constant parameters instead.
* Re-added Revision::revText(), for backwards compatibility
* The getUser()/getUserText() changes near line 1223 of SpecialUndelete.php were incorrect, $file is an ArchivedFile not a Revision, and doesn't have any $isPublic parameters.
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)
  • /trunk/phase3/includes/Revision.php (modified) (history)
  • /trunk/phase3/includes/api/ApiParse.php (modified) (history)
  • /trunk/phase3/includes/diff/DifferenceEngine.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUndelete.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/diff/DifferenceEngine.php
——@@ -176,7 +176,7 @@
177177 $change = RecentChange::newFromConds(
178178 array(
179179 // Add redundant user,timestamp condition so we can use the existing index
180 - 'rc_user_text' => $this->mNewRev->getUserText(false),
 180+ 'rc_user_text' => $this->mNewRev->getUserText( Revision::FOR_THIS_USER ),
181181 'rc_timestamp' => $db->timestamp( $this->mNewRev->getTimestamp() ),
182182 'rc_this_oldid' => $this->mNewid,
183183 'rc_last_oldid' => $this->mOldid,
——@@ -847,13 +847,13 @@
848848 return false;
849849 }
850850 if ( $this->mOldRev ) {
851 - $this->mOldtext = $this->mOldRev->getText( false );
 851+ $this->mOldtext = $this->mOldRev->getText( Revision::FOR_THIS_USER );
852852 if ( $this->mOldtext === false ) {
853853 return false;
854854 }
855855 }
856856 if ( $this->mNewRev ) {
857 - $this->mNewtext = $this->mNewRev->getText( false );
 857+ $this->mNewtext = $this->mNewRev->getText( Revision::FOR_THIS_USER );
858858 if ( $this->mNewtext === false ) {
859859 return false;
860860 }
——@@ -2114,4 +2114,4 @@
21152115 }
21162116 wfProfileOut( __METHOD__ );
21172117 }
2118 -}
\ No newline at end of file
 2118+}
Index: trunk/phase3/includes/Article.php
——@@ -448,7 +448,7 @@
449449
450450 // FIXME: Horrible, horrible! This content-loading interface just plain sucks.
451451 // We should instead work with the Revision object when we need it...
452 - $this->mContent = $revision->getText( false ); // Loads if user is allowed
 452+ $this->mContent = $revision->getText( Revision::FOR_THIS_USER ); // Loads if user is allowed
453453
454454 $this->mUser = $revision->getUser();
455455 $this->mUserText = $revision->getUserText();
Index: trunk/phase3/includes/Linker.php
——@@ -1113,7 +1113,8 @@
11141114 if( $rev->isDeleted( Revision::DELETED_USER ) && $isPublic ) {
11151115 $link = wfMsgHtml( 'rev-deleted-user' );
11161116 } else if( $rev->userCan( Revision::DELETED_USER ) ) {
1117 - $link = $this->userLink( $rev->getUser(false), $rev->getUserText(false) );
 1117+ $link = $this->userLink( $rev->getUser( Revision::FOR_THIS_USER ),
 1118+ $rev->getUserText( Revision::FOR_THIS_USER ) );
11181119 } else {
11191120 $link = wfMsgHtml( 'rev-deleted-user' );
11201121 }
——@@ -1133,8 +1134,10 @@
11341135 if( $rev->isDeleted( Revision::DELETED_USER ) && $isPublic ) {
11351136 $link = wfMsgHtml( 'rev-deleted-user' );
11361137 } else if( $rev->userCan( Revision::DELETED_USER ) ) {
1137 - $link = $this->userLink( $rev->getUser(false), $rev->getUserText(false) ) .
1138 - ' ' . $this->userToolLinks( $rev->getUser(false), $rev->getUserText(false) );
 1138+ $userId = $rev->getUser( Revision::FOR_THIS_USER );
 1139+ $userText = $rev->getUserText( Revision::FOR_THIS_USER );
 1140+ $link = $this->userLink( $userId, $userText ) .
 1141+ ' ' . $this->userToolLinks( $userId, $userText );
11391142 } else {
11401143 $link = wfMsgHtml( 'rev-deleted-user' );
11411144 }
——@@ -1340,7 +1343,8 @@
13411344 if( $rev->isDeleted( Revision::DELETED_COMMENT ) && $isPublic ) {
13421345 $block = " <span class=\"comment\">" . wfMsgHtml( 'rev-deleted-comment' ) . "</span>";
13431346 } else if( $rev->userCan( Revision::DELETED_COMMENT ) ) {
1344 - $block = $this->commentBlock( $rev->getComment(false), $rev->getTitle(), $local );
 1347+ $block = $this->commentBlock( $rev->getComment( Revision::FOR_THIS_USER ),
 1348+ $rev->getTitle(), $local );
13451349 } else {
13461350 $block = " <span class=\"comment\">" . wfMsgHtml( 'rev-deleted-comment' ) . "</span>";
13471351 }
Index: trunk/phase3/includes/api/ApiParse.php
——@@ -63,7 +63,7 @@
6464 $this->dieUsage("There is no revision ID $oldid", 'missingrev');
6565 if(!$rev->userCan(Revision::DELETED_TEXT))
6666 $this->dieUsage("You don't have permission to view deleted revisions", 'permissiondenied');
67 - $text = $rev->getText(false);
 67+ $text = $rev->getText( Revision::FOR_THIS_USER );
6868 $titleObj = $rev->getTitle();
6969 $p_result = $wgParser->parse($text, $titleObj, $popts);
7070 }
Index: trunk/phase3/includes/Revision.php
——@@ -13,6 +13,11 @@
1414 const DELETED_USER = 4;
1515 const DELETED_RESTRICTED = 8;
1616
 17+ // Audience options for Revision::getText()
 18+ const FOR_PUBLIC = 1;
 19+ const FOR_THIS_USER = 2;
 20+ const RAW = 3;
 21+
1722 /**
1823 * Load a page revision from a given revision ID number.
1924 * Returns null if no such revision can be found.
——@@ -427,13 +432,22 @@
428433 }
429434
430435 /**
431 - * Fetch revision's user id if it's available to all users
 436+ * Fetch revision's user id if it's available to the specified audience.
 437+ * If the specified audience does not have access to it, zero will be
 438+ * returned.
 439+ *
 440+ * @param integer $audience One of:
 441+ * Revision::FOR_PUBLIC to be displayed to all users
 442+ * Revision::FOR_THIS_USER to be displayed to $wgUser
 443+ * Revision::RAW get the ID regardless of permissions
 444+ *
 445+ *
432446 * @return int
433447 */
434 - public function getUser( $isPublic = true ) {
435 - if( $isPublic && $this->isDeleted( self::DELETED_USER ) ) {
 448+ public function getUser( $audience = self::FOR_PUBLIC ) {
 449+ if( $audience == self::FOR_PUBLIC && $this->isDeleted( self::DELETED_USER ) ) {
436450 return 0;
437 - } else if( !$this->userCan( self::DELETED_USER ) ) {
 451+ } elseif( $audience == self::FOR_THIS_USER && !$this->userCan( self::DELETED_USER ) ) {
438452 return 0;
439453 } else {
440454 return $this->mUser;
——@@ -449,13 +463,21 @@
450464 }
451465
452466 /**
453 - * Fetch revision's username if it's available to all users
 467+ * Fetch revision's username if it's available to the specified audience.
 468+ * If the specified audience does not have access to the username, an
 469+ * empty string will be returned.
 470+ *
 471+ * @param integer $audience One of:
 472+ * Revision::FOR_PUBLIC to be displayed to all users
 473+ * Revision::FOR_THIS_USER to be displayed to $wgUser
 474+ * Revision::RAW get the text regardless of permissions
 475+ *
454476 * @return string
455477 */
456 - public function getUserText( $isPublic = true ) {
457 - if( $isPublic && $this->isDeleted( self::DELETED_USER ) ) {
 478+ public function getUserText( $audience = self::FOR_PUBLIC ) {
 479+ if( $audience == self::FOR_PUBLIC && $this->isDeleted( self::DELETED_USER ) ) {
458480 return "";
459 - } else if( !$this->userCan( self::DELETED_USER ) ) {
 481+ } elseif( $audience == self::FOR_THIS_USER && !$this->userCan( self::DELETED_USER ) ) {
460482 return "";
461483 } else {
462484 return $this->mUserText;
——@@ -471,13 +493,21 @@
472494 }
473495
474496 /**
475 - * Fetch revision comment if it's available to all users
 497+ * Fetch revision comment if it's available to the specified audience.
 498+ * If the specified audience does not have access to the comment, an
 499+ * empty string will be returned.
 500+ *
 501+ * @param integer $audience One of:
 502+ * Revision::FOR_PUBLIC to be displayed to all users
 503+ * Revision::FOR_THIS_USER to be displayed to $wgUser
 504+ * Revision::RAW get the text regardless of permissions
 505+ *
476506 * @return string
477507 */
478 - function getComment( $isPublic = true ) {
479 - if( $isPublic && $this->isDeleted( self::DELETED_COMMENT ) ) {
 508+ function getComment( $audience = self::FOR_PUBLIC ) {
 509+ if( $audience == self::FOR_PUBLIC && $this->isDeleted( self::DELETED_COMMENT ) ) {
480510 return "";
481 - } else if( !$this->userCan( self::DELETED_COMMENT ) ) {
 511+ } elseif( $audience == self::FOR_THIS_USER && !$this->userCan( self::DELETED_COMMENT ) ) {
482512 return "";
483513 } else {
484514 return $this->mComment;
——@@ -508,13 +538,22 @@
509539 }
510540
511541 /**
512 - * Fetch revision text if it's available to all users
 542+ * Fetch revision text if it's available to the specified audience.
 543+ * If the specified audience does not have the ability to view this
 544+ * revision, an empty string will be returned.
 545+ *
 546+ * @param integer $audience One of:
 547+ * Revision::FOR_PUBLIC to be displayed to all users
 548+ * Revision::FOR_THIS_USER to be displayed to $wgUser
 549+ * Revision::RAW get the text regardless of permissions
 550+ *
 551+ *
513552 * @return string
514553 */
515 - public function getText( $isPublic = true ) {
516 - if( $isPublic && $this->isDeleted( self::DELETED_TEXT ) ) {
 554+ public function getText( $audience = self::FOR_PUBLIC ) {
 555+ if( $audience == self::FOR_PUBLIC && $this->isDeleted( self::DELETED_TEXT ) ) {
517556 return "";
518 - } else if( !$this->userCan( self::DELETED_TEXT ) ) {
 557+ } elseif( $audience == self::FOR_THIS_USER && !$this->userCan( self::DELETED_TEXT ) ) {
519558 return "";
520559 } else {
521560 return $this->getRawText();
——@@ -522,6 +561,13 @@
523562 }
524563
525564 /**
 565+ * Alias for getText(Revision::FOR_THIS_USER)
 566+ */
 567+ public function revText() {
 568+ return $this->getText( self::FOR_THIS_USER );
 569+ }
 570+
 571+ /**
526572 * Fetch revision text without regard for view restrictions
527573 * @return string
528574 */
Index: trunk/phase3/includes/specials/SpecialUndelete.php
——@@ -787,7 +787,7 @@
788788 $popts = $wgOut->parserOptions();
789789 $popts->setEditSection( false );
790790 $wgOut->parserOptions( $popts );
791 - $wgOut->addWikiTextTitleTidy( $rev->getText( false ), $this->mTargetObj, true );
 791+ $wgOut->addWikiTextTitleTidy( $rev->getText( Revision::FOR_THIS_USER ), $this->mTargetObj, true );
792792 }
793793
794794 $wgOut->addHtml(
——@@ -795,7 +795,7 @@
796796 'readonly' => 'readonly',
797797 'cols' => intval( $wgUser->getOption( 'cols' ) ),
798798 'rows' => intval( $wgUser->getOption( 'rows' ) ) ),
799 - $rev->getText( false ) . "\n" ) .
 799+ $rev->getText( Revision::FOR_THIS_USER ) . "\n" ) .
800800 wfOpenElement( 'div' ) .
801801 wfOpenElement( 'form', array(
802802 'method' => 'post',
——@@ -1223,8 +1223,8 @@
12241224 if( !$file->userCan(File::DELETED_USER) ) {
12251225 return '<span class="history-deleted">' . wfMsgHtml( 'rev-deleted-user' ) . '</span>';
12261226 } else {
1227 - $link = $sk->userLink( $file->getUser(false), $file->getUserText(false) ) .
1228 - $sk->userToolLinks( $file->getUser(false), $file->getUserText(false) );
 1227+ $link = $sk->userLink( $file->getUser(), $file->getUserText() ) .
 1228+ $sk->userToolLinks( $file->getUser(), $file->getUserText() );
12291229 if( $file->isDeleted(File::DELETED_USER) )
12301230 $link = '<span class="history-deleted">' . $link . '</span>';
12311231 return $link;

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r41154Remove Revision::revText() and move functionality to getText()aaron14:24, 22 September 2008
r41155rev_deleted security improvements as well as fix for rawpagesaaron14:37, 22 September 2008