r81675 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81674‎ | r81675 | r81676 >
Date:01:08, 8 February 2011
Author:demon
Status:resolved (Comments)
Tags:
Comment:
Cleanup to r70900, r72481: don't construct new skin objects just because the Title is passed. Use the skin object we already have if the titles are the same
Modified paths:
  • /trunk/phase3/includes/User.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/User.php
——@@ -2247,21 +2247,16 @@
22482248 * @todo: FIXME : need to check the old failback system [AV]
22492249 */
22502250 function getSkin( $t = null ) {
2251 - if ( $t ) {
 2251+ if( !$this->mSkin ) {
 2252+ global $wgOut;
 2253+ $this->mSkin = $this->createSkinObject();
 2254+ $this->mSkin->setTitle( $wgOut->getTitle() );
 2255+ }
 2256+ if ( $t && !$t->equals( $this->mSkin->getTitle() ) ) {
22522257 $skin = $this->createSkinObject();
22532258 $skin->setTitle( $t );
22542259 return $skin;
22552260 } else {
2256 - if ( !$this->mSkin ) {
2257 - $this->mSkin = $this->createSkinObject();
2258 - }
2259 -
2260 - if ( !$this->mSkin->getTitle() ) {
2261 - global $wgOut;
2262 - $t = $wgOut->getTitle();
2263 - $this->mSkin->setTitle($t);
2264 - }
2265 -
22662261 return $this->mSkin;
22672262 }
22682263 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r81684MFT r81675: avoid creating unneeded skin objects due to changes in r70900, r7...brion02:48, 8 February 2011
r81685MFT from r81675 via REL1_17 r81684: cleanup for r70900, r72481brion02:50, 8 February 2011
r81827Fix for r81675: Skin::getTitle() will return null when $wgTitle is null and $...ialex17:05, 9 February 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r70900getSkin() should get a Title as parameter.platonides17:11, 11 August 2010
r72481Rewrite User::getSkin, broken in r49493 because requesting the skin for a par...werdna12:11, 6 September 2010

Comments

#Comment by 😂 (talk | contribs)   02:50, 8 February 2011

This is not a followup to r70900, I'm just crazy.

#Comment by Brion VIBBER (talk | contribs)   02:51, 8 February 2011

it's a related issue, we'll allow it ;)

#Comment by Brion VIBBER (talk | contribs)   02:51, 8 February 2011

Looks ok per IRC discussion -- merged to 1.17 and 1.17wmf1

#Comment by Nikerabbit (talk | contribs)   17:03, 9 February 2011

Catchable fatal error: Argument 1 passed to Title::equals() must be an instance of Title, null given, called in /www/sandwiki/includes/User.php on line 2255 and defined in /www/sandwiki/includes/Title.php on line 3689

Fallout of that CLI scripts don't have $wgTitle set, OutputPage returns $wgTitle which I think is null in this case.

#Comment by IAlex (talk | contribs)   17:06, 9 February 2011

Fixed in r81827.

Status & tagging log