r78172 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r78171‎ | r78172 | r78173 >
Date:11:33, 10 December 2010
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Followup r78117: information was still leaked through the caption of the Edit tab (which is "Create" for nonexistent pages) for users who had edit but not read rights. This may seem like a ridiculous case, but since we automatically disallow edit if you can't read, it's easy to forget to disallow editing as well as reading. Fixed this by not showing the edit tab at all if the user can't read (which makes sense since it's also hidden if the user is explicitly forbidden from editing).
Modified paths:
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/SkinTemplate.php
——@@ -754,7 +754,7 @@
755755 );
756756
757757 wfProfileIn( __METHOD__ . '-edit' );
758 - if ( $this->mTitle->quickUserCan( 'edit' ) && ( $this->mTitle->exists() || $this->mTitle->quickUserCan( 'create' ) ) ) {
 758+ if ( $userCanRead && $this->mTitle->quickUserCan( 'edit' ) && ( $this->mTitle->exists() || $this->mTitle->quickUserCan( 'create' ) ) ) {
759759 $istalk = $this->mTitle->isTalkPage();
760760 $istalkclass = $istalk?' istalk':'';
761761 $content_actions['edit'] = array(
Index: trunk/phase3/skins/Vector.php
——@@ -115,8 +115,8 @@
116116
117117 // Checks if user can...
118118 if (
119 - // edit the current page
120 - $this->mTitle->quickUserCan( 'edit' ) &&
 119+ // read and edit the current page
 120+ $userCanRead && $this->mTitle->quickUserCan( 'edit' ) &&
121121 (
122122 // if it exists
123123 $this->mTitle->exists() ||

Follow-up revisions

RevisionCommit summaryAuthorDate
r79129MFT r78011 r78014 r78015 r78016 r78099 r78117 r78161 r78170 r78172 r78199 r78......platonides19:58, 28 December 2010
r79341Follow-up r79340: merge r78117, r78170, r78172 and r78506 from trunk into ski...catrope15:48, 31 December 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r78117Do not reveal page existence in colour of links in tabs when a user cannot re...werdna04:54, 9 December 2010

Comments

#Comment by Platonides (talk | contribs)   22:32, 29 December 2010

I would change to

$userCanRead && ( ( $this->mTitle->exists() && $this->mTitle->quickUserCan( 'edit' ) ) || $this->mTitle->quickUserCan( 'create' ) ) ) {

#Comment by Catrope (talk | contribs)   22:35, 29 December 2010

Why?

#Comment by Platonides (talk | contribs)   22:37, 29 December 2010

You skip a quickUserCan if the article is missing. You could also then support creating pages if you can't edit, although that wouldn't make much sense.

Status & tagging log