r101491 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101490‎ | r101491 | r101492 >
Date:16:09, 1 November 2011
Author:ialex
Status:resolved (Comments)
Tags:
Comment:
* Create all items in $nav_urls at the beginning so that we don't need to bother if they are set or not after that
* Don't show 'whatlinkshere', 'recentchangeslinked' and 'trackbacklink' items if OutputPage::isArticleRelated() is false
* Simplify the code to check if we show show user related items since the actual code is just to check if we have an User object
* Moved the showEmailUser() check to the same location as other related user checks
* Made Skin::showEmailUser() accept an User object instead of doing object -> int -> object conversion
* Check if the items are present in BaseTemplate::getToolbox() instead of 'notspecialpage' and removed some empty() or isset() checks since all of these items are now set
Modified paths:
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/SkinTemplate.php
——@@ -1154,8 +1154,15 @@
11551155 }
11561156 $nav_urls['specialpages'] = array( 'href' => self::makeSpecialUrl( 'Specialpages' ) );
11571157
1158 - // default permalink to being off, will override it as required below.
 1158+ $nav_urls['print'] = false;
11591159 $nav_urls['permalink'] = false;
 1160+ $nav_urls['whatlinkshere'] = false;
 1161+ $nav_urls['recentchangeslinked'] = false;
 1162+ $nav_urls['trackbacklink'] = false;
 1163+ $nav_urls['contributions'] = false;
 1164+ $nav_urls['log'] = false;
 1165+ $nav_urls['blockip'] = false;
 1166+ $nav_urls['emailuser'] = false;
11601167
11611168 // A print stylesheet is attached to all pages, but nobody ever
11621169 // figures that out. :) Add a link...
——@@ -1182,72 +1189,50 @@
11831190 array( &$this, &$nav_urls, &$revid, &$revid ) );
11841191 }
11851192
1186 - if( $this->getTitle()->getNamespace() != NS_SPECIAL ) {
1187 - $wlhTitle = SpecialPage::getTitleFor( 'Whatlinkshere', $this->thispage );
 1193+ if ( $out->isArticleRelated() ) {
11881194 $nav_urls['whatlinkshere'] = array(
1189 - 'href' => $wlhTitle->getLocalUrl()
 1195+ 'href' => SpecialPage::getTitleFor( 'Whatlinkshere', $this->thispage )->getLocalUrl()
11901196 );
1191 - if( $this->getTitle()->getArticleId() ) {
1192 - $rclTitle = SpecialPage::getTitleFor( 'Recentchangeslinked', $this->thispage );
 1197+ if ( $this->getTitle()->getArticleId() ) {
11931198 $nav_urls['recentchangeslinked'] = array(
1194 - 'href' => $rclTitle->getLocalUrl()
 1199+ 'href' => SpecialPage::getTitleFor( 'Recentchangeslinked', $this->thispage )->getLocalUrl()
11951200 );
1196 - } else {
1197 - $nav_urls['recentchangeslinked'] = false;
11981201 }
1199 - if( $wgUseTrackbacks )
 1202+ if ( $wgUseTrackbacks ) {
12001203 $nav_urls['trackbacklink'] = array(
12011204 'href' => $out->getTitle()->trackbackURL()
12021205 );
 1206+ }
12031207 }
12041208
12051209 $user = $this->getRelevantUser();
12061210 if ( $user ) {
1207 - $id = $user->getID();
1208 - $ip = $user->isAnon();
12091211 $rootUser = $user->getName();
1210 - } else {
1211 - $id = 0;
1212 - $ip = false;
1213 - $rootUser = null;
1214 - }
12151212
1216 - if( $id || $ip ) { # both anons and non-anons have contribs list
12171213 $nav_urls['contributions'] = array(
12181214 'href' => self::makeSpecialUrlSubpage( 'Contributions', $rootUser )
12191215 );
12201216
1221 - if( $id ) {
 1217+ if ( $user->isLoggedIn() ) {
12221218 $logPage = SpecialPage::getTitleFor( 'Log' );
12231219 $nav_urls['log'] = array(
1224 - 'href' => $logPage->getLocalUrl(
1225 - array(
1226 - 'user' => $rootUser
1227 - )
1228 - )
 1220+ 'href' => $logPage->getLocalUrl( array( 'user' => $rootUser ) )
12291221 );
1230 - } else {
1231 - $nav_urls['log'] = false;
12321222 }
12331223
12341224 if ( $this->getUser()->isAllowed( 'block' ) ) {
12351225 $nav_urls['blockip'] = array(
12361226 'href' => self::makeSpecialUrlSubpage( 'Block', $rootUser )
12371227 );
1238 - } else {
1239 - $nav_urls['blockip'] = false;
12401228 }
1241 - } else {
1242 - $nav_urls['contributions'] = false;
1243 - $nav_urls['log'] = false;
1244 - $nav_urls['blockip'] = false;
 1229+
 1230+ if ( $this->showEmailUser( $user ) ) {
 1231+ $nav_urls['emailuser'] = array(
 1232+ 'href' => self::makeSpecialUrlSubpage( 'Emailuser', $rootUser )
 1233+ );
 1234+ }
12451235 }
1246 - $nav_urls['emailuser'] = false;
1247 - if( $this->showEmailUser( $id ) ) {
1248 - $nav_urls['emailuser'] = array(
1249 - 'href' => self::makeSpecialUrlSubpage( 'Emailuser', $rootUser )
1250 - );
1251 - }
 1236+
12521237 wfProfileOut( __METHOD__ );
12531238 return $nav_urls;
12541239 }
——@@ -1441,16 +1426,16 @@
14421427 wfProfileIn( __METHOD__ );
14431428
14441429 $toolbox = array();
1445 - if ( $this->data['notspecialpage'] ) {
 1430+ if ( $this->data['nav_urls']['whatlinkshere'] ) {
14461431 $toolbox['whatlinkshere'] = $this->data['nav_urls']['whatlinkshere'];
14471432 $toolbox['whatlinkshere']['id'] = 't-whatlinkshere';
1448 - if ( $this->data['nav_urls']['recentchangeslinked'] ) {
1449 - $toolbox['recentchangeslinked'] = $this->data['nav_urls']['recentchangeslinked'];
1450 - $toolbox['recentchangeslinked']['msg'] = 'recentchangeslinked-toolbox';
1451 - $toolbox['recentchangeslinked']['id'] = 't-recentchangeslinked';
1452 - }
14531433 }
1454 - if( isset( $this->data['nav_urls']['trackbacklink'] ) && $this->data['nav_urls']['trackbacklink'] ) {
 1434+ if ( $this->data['nav_urls']['recentchangeslinked'] ) {
 1435+ $toolbox['recentchangeslinked'] = $this->data['nav_urls']['recentchangeslinked'];
 1436+ $toolbox['recentchangeslinked']['msg'] = 'recentchangeslinked-toolbox';
 1437+ $toolbox['recentchangeslinked']['id'] = 't-recentchangeslinked';
 1438+ }
 1439+ if ( $this->data['nav_urls']['trackbacklink'] ) {
14551440 $toolbox['trackbacklink'] = $this->data['nav_urls']['trackbacklink'];
14561441 $toolbox['trackbacklink']['id'] = 't-trackbacklink';
14571442 }
——@@ -1471,7 +1456,7 @@
14721457 $toolbox[$special]['id'] = "t-$special";
14731458 }
14741459 }
1475 - if ( !empty( $this->data['nav_urls']['print']['href'] ) ) {
 1460+ if ( $this->data['nav_urls']['print'] ) {
14761461 $toolbox['print'] = $this->data['nav_urls']['print'];
14771462 $toolbox['print']['rel'] = 'alternate';
14781463 $toolbox['print']['msg'] = 'printableversion';
Index: trunk/phase3/includes/Skin.php
——@@ -929,7 +929,11 @@
930930 }
931931
932932 function showEmailUser( $id ) {
933 - $targetUser = User::newFromId( $id );
 933+ if ( $id instanceof User ) {
 934+ $targetUser = $id;
 935+ } else {
 936+ $targetUser = User::newFromId( $id );
 937+ }
934938 return $this->getUser()->canSendEmail() && # the sending user must have a confirmed email address
935939 $targetUser->canReceiveEmail(); # the target user must have a confirmed email address and allow emails from users
936940 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r101609Per Roan, fix for r101491: set to false instead of unsetting to avoid noticeialex10:26, 2 November 2011

Comments

#Comment by Catrope (talk | contribs)   10:14, 2 November 2011 
Notice: Undefined index: print in /home/catrope/mediawiki/trunk/phase3/includes/SkinTemplate.php on line 1459

Call Stack:
    0.0001     657656   1. {main}() /home/catrope/mediawiki/trunk/phase3/index.php:0
    0.0592   14151696   2. MediaWiki->run() /home/catrope/mediawiki/trunk/phase3/index.php:58
    0.0593   14151696   3. MediaWiki->main() /home/catrope/mediawiki/trunk/phase3/includes/Wiki.php:519
    0.1469   30854768   4. MediaWiki->finalCleanup() /home/catrope/mediawiki/trunk/phase3/includes/Wiki.php:601
    0.1470   30854848   5. OutputPage->output() /home/catrope/mediawiki/trunk/phase3/includes/Wiki.php:381
    0.1701   33328800   6. SkinTemplate->outputPage() /home/catrope/mediawiki/trunk/phase3/includes/OutputPage.php:1901
    0.2831   41035824   7. VectorTemplate->execute() /home/catrope/mediawiki/trunk/phase3/includes/SkinTemplate.php:502
    0.2948   41084264   8. VectorTemplate->renderPortals() /home/catrope/mediawiki/trunk/phase3/skins/Vector.php:211
    0.2978   41090328   9. BaseTemplate->getToolbox() /home/catrope/mediawiki/trunk/phase3/skins/Vector.php:275

Reverting this revision makes the notice go away.

#Comment by IAlex (talk | contribs)   10:28, 2 November 2011

Fixed in r101609.

Status & tagging log