r57530 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r57529‎ | r57530 | r57531 >
Date:19:40, 8 October 2009
Author:werdna
Status:ok (Comments)
Tags:
Comment:
Replace broken nested terneries (causing a nasty bug where all admins could view fully suppressed revisions) with clear, easy to read if/then/elseif/else structure.
Modified paths:
  • /trunk/phase3/includes/Revision.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Revision.php
——@@ -979,9 +979,16 @@
980980 public function userCan( $field ) {
981981 if( $this->mDeleted & $field ) {
982982 global $wgUser;
983 - $permission = ( $this->mDeleted & self::DELETED_RESTRICTED )
984 - ? 'suppressrevision'
985 - : ( $field & self::DELETED_TEXT ) ? 'deletedtext' : 'deletedhistory';
 983+
 984+ $permission = '';
 985+ if ( $this->mDeleted & self::DELETED_RESTRICTED ) {
 986+ $permission = 'suppressrevision';
 987+ } elseif ( $field & self::DELETED_TEXT ) {
 988+ $permission = 'deletedtext';
 989+ } else {
 990+ $permission = 'deletedhistory';
 991+ }
 992+
986993 wfDebug( "Checking for $permission due to $field match on $this->mDeleted\n" );
987994 return $wgUser->isAllowed( $permission );
988995 } else {

Follow-up revisions

RevisionCommit summaryAuthorDate
r57531Merge r57530werdna19:57, 8 October 2009
r57579Follow up r57530 elsewhereaaron19:01, 9 October 2009
r57594Same fix as r57530, except done correctlywerdna21:33, 9 October 2009
r57602Fill in class names for r57530aaron02:12, 10 October 2009

Comments

#Comment by Aaron Schulz (talk | contribs)   04:42, 9 October 2009

nested terneries don't work?

#Comment by Werdna (talk | contribs)   09:28, 9 October 2009

It's an order of operations issue.

This is what you meant: 

-			$permission = ( $this->mDeleted & self::DELETED_RESTRICTED )
-				? 'suppressrevision'
-				: ( ( $field & self::DELETED_TEXT ) ? 'deletedtext' : 'deletedhistory' );

And this is what PHP saw: 

-			$permission = ( ( $this->mDeleted & self::DELETED_RESTRICTED )
-				? 'suppressrevision'
-				: ( $field & self::DELETED_TEXT ) ) ? 'deletedtext' : 'deletedhistory';

Status & tagging log