r95957 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r95956‎ | r95957 | r95958 >
Date:10:24, 1 September 2011
Author:dantman
Status:ok
Tags:
Comment:
Kill off use of $wgRequest, $wgUser, and $wgLang in SkinTemplate, stop using $out args on our protected methods, deprecate the $out on outputPage and for now make it temporarily override the context of the skin (or should we throw a fatal if someone passes an OutputPage that doesn't match context)
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/OutputPage.php
——@@ -1864,7 +1864,7 @@
18651865 wfRunHooks( 'BeforePageDisplay', array( &$this, &$sk ) );
18661866
18671867 wfProfileIn( 'Output-skin' );
1868 - $sk->outputPage( $this );
 1868+ $sk->outputPage();
18691869 wfProfileOut( 'Output-skin' );
18701870 }
18711871
Index: trunk/phase3/includes/SkinTemplate.php
——@@ -131,10 +131,10 @@
132132 *
133133 * @param $out OutputPage
134134 */
135 - function outputPage( OutputPage $out ) {
136 - global $wgUser, $wgLang, $wgContLang;
 135+ function outputPage( OutputPage $out=null ) {
 136+ global $wgContLang;
137137 global $wgScript, $wgStylePath;
138 - global $wgMimeType, $wgJsMimeType, $wgRequest;
 138+ global $wgMimeType, $wgJsMimeType;
139139 global $wgXhtmlDefaultNamespace, $wgXhtmlNamespaces, $wgHtml5Version;
140140 global $wgDisableCounters, $wgLogo, $wgHideInterlanguageLinks;
141141 global $wgMaxCredits, $wgShowCreditsIfMax;
——@@ -145,6 +145,17 @@
146146 wfProfileIn( __METHOD__ );
147147 Profiler::instance()->setTemplated( true );
148148
 149+ $oldContext = null;
 150+ if ( $out !== null ) {
 151+ // @todo Add wfDeprecated in 1.20
 152+ $oldContext = $this->getContext();
 153+ $this->setContext( $out->getContext() );
 154+ }
 155+
 156+ $out = $this->getOutput();
 157+ $request = $this->getRequest();
 158+ $user = $this->getUser();
 159+
149160 wfProfileIn( __METHOD__ . '-init' );
150161 $this->initPage( $out );
151162
——@@ -153,19 +164,19 @@
154165
155166 wfProfileIn( __METHOD__ . '-stuff' );
156167 $this->thispage = $this->getTitle()->getPrefixedDBkey();
157 - $this->userpage = $this->getUser()->getUserPage()->getPrefixedText();
 168+ $this->userpage = $user->getUserPage()->getPrefixedText();
158169 $query = array();
159 - if ( !$wgRequest->wasPosted() ) {
160 - $query = $wgRequest->getValues();
 170+ if ( !$request->wasPosted() ) {
 171+ $query = $request->getValues();
161172 unset( $query['title'] );
162173 unset( $query['returnto'] );
163174 unset( $query['returntoquery'] );
164175 }
165176 $this->thisquery = wfArrayToCGI( $query );
166 - $this->loggedin = $wgUser->isLoggedIn();
167 - $this->username = $wgUser->getName();
 177+ $this->loggedin = $user->isLoggedIn();
 178+ $this->username = $user->getName();
168179
169 - if ( $wgUser->isLoggedIn() || $this->showIPinHeader() ) {
 180+ if ( $user->isLoggedIn() || $this->showIPinHeader() ) {
170181 $this->userpageUrlDetails = self::makeUrlDetails( $this->userpage );
171182 } else {
172183 # This won't be used in the standard skins, but we define it to preserve the interface
——@@ -262,7 +273,7 @@
263274 $tpl->set( 'skinclass', get_class( $this ) );
264275 $tpl->setRef( 'stylename', $this->stylename );
265276 $tpl->set( 'printable', $out->isPrintable() );
266 - $tpl->set( 'handheld', $wgRequest->getBool( 'handheld' ) );
 277+ $tpl->set( 'handheld', $request->getBool( 'handheld' ) );
267278 $tpl->setRef( 'loggedin', $this->loggedin );
268279 $tpl->set( 'notspecialpage', $this->getTitle()->getNamespace() != NS_SPECIAL );
269280 /* XXX currently unused, might get useful later
——@@ -274,7 +285,7 @@
275286 */
276287 $tpl->set( 'searchaction', $this->escapeSearchLink() );
277288 $tpl->set( 'searchtitle', SpecialPage::getTitleFor( 'Search' )->getPrefixedDBKey() );
278 - $tpl->set( 'search', trim( $wgRequest->getVal( 'search' ) ) );
 289+ $tpl->set( 'search', trim( $request->getVal( 'search' ) ) );
279290 $tpl->setRef( 'stylepath', $wgStylePath );
280291 $tpl->setRef( 'articlepath', $wgArticlePath );
281292 $tpl->setRef( 'scriptpath', $wgScriptPath );
——@@ -283,16 +294,16 @@
284295
285296 $contentlang = $wgContLang->getCode();
286297 $contentdir = $wgContLang->getDir();
287 - $userlang = $wgLang->getCode();
288 - $userdir = $wgLang->getDir();
 298+ $userlang = $this->getLang()->getCode();
 299+ $userdir = $this->getLang()->getDir();
289300
290301 $tpl->set( 'lang', $userlang );
291302 $tpl->set( 'dir', $userdir );
292 - $tpl->set( 'rtl', $wgLang->isRTL() );
 303+ $tpl->set( 'rtl', $this->getLang()->isRTL() );
293304
294 - $tpl->set( 'capitalizeallnouns', $wgLang->capitalizeAllNouns() ? ' capitalize-all-nouns' : '' );
295 - $tpl->set( 'showjumplinks', $wgUser->getOption( 'showjumplinks' ) );
296 - $tpl->set( 'username', $wgUser->isAnon() ? null : $this->username );
 305+ $tpl->set( 'capitalizeallnouns', $this->getLang()->capitalizeAllNouns() ? ' capitalize-all-nouns' : '' );
 306+ $tpl->set( 'showjumplinks', $user->getOption( 'showjumplinks' ) );
 307+ $tpl->set( 'username', $user->isAnon() ? null : $this->username );
297308 $tpl->setRef( 'userpage', $this->userpage );
298309 $tpl->setRef( 'userpageurl', $this->userpageUrlDetails['href'] );
299310 $tpl->set( 'userlang', $userlang );
——@@ -324,7 +335,7 @@
325336 if ( $this->isRevisionCurrent() ) {
326337 $article = new Article( $this->getTitle(), 0 );
327338 if ( !$wgDisableCounters ) {
328 - $viewcount = $wgLang->formatNum( $article->getCount() );
 339+ $viewcount = $this->getLang()->formatNum( $article->getCount() );
329340 if ( $viewcount ) {
330341 $tpl->set( 'viewcount', wfMsgExt( 'viewcount', array( 'parseinline' ), $viewcount ) );
331342 }
——@@ -339,7 +350,7 @@
340351 if( $num > 0 ) {
341352 $tpl->set( 'numberofwatchingusers',
342353 wfMsgExt( 'number_of_watching_users_pageview', array( 'parseinline' ),
343 - $wgLang->formatNum( $num ) )
 354+ $this->getLang()->formatNum( $num ) )
344355 );
345356 }
346357 }
——@@ -410,7 +421,7 @@
411422 # not for special pages or file pages AND only when viewing AND if the page exists
412423 # (or is in MW namespace, because that has default content)
413424 if( !in_array( $this->getTitle()->getNamespace(), array( NS_SPECIAL, NS_FILE ) ) &&
414 - in_array( $wgRequest->getVal( 'action', 'view' ), array( 'view', 'historysubmit' ) ) &&
 425+ in_array( $request->getVal( 'action', 'view' ), array( 'view', 'historysubmit' ) ) &&
415426 ( $this->getTitle()->exists() || $this->getTitle()->getNamespace() == NS_MEDIAWIKI ) ) {
416427 $pageLang = $this->getTitle()->getPageLanguage();
417428 $realBodyAttribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(),
——@@ -449,14 +460,14 @@
450461
451462 wfProfileIn( __METHOD__ . '-stuff5' );
452463 # Personal toolbar
453 - $tpl->set( 'personal_urls', $this->buildPersonalUrls( $out ) );
454 - $content_navigation = $this->buildContentNavigationUrls( $out );
 464+ $tpl->set( 'personal_urls', $this->buildPersonalUrls() );
 465+ $content_navigation = $this->buildContentNavigationUrls();
455466 $content_actions = $this->buildContentActionUrls( $content_navigation );
456467 $tpl->setRef( 'content_navigation', $content_navigation );
457468 $tpl->setRef( 'content_actions', $content_actions );
458469
459470 $tpl->set( 'sidebar', $this->buildSidebar() );
460 - $tpl->set( 'nav_urls', $this->buildNavUrls( $out ) );
 471+ $tpl->set( 'nav_urls', $this->buildNavUrls() );
461472
462473 // Set the head scripts near the end, in case the above actions resulted in added scripts
463474 if ( $this->useHeadElement ) {
——@@ -484,6 +495,10 @@
485496
486497 // result may be an error
487498 $this->printOrError( $res );
 499+
 500+ if ( $oldContext ) {
 501+ $this->setContext( $oldContext );
 502+ }
488503 wfProfileOut( __METHOD__ );
489504 }
490505
——@@ -516,18 +531,17 @@
517532 * build array of urls for personal toolbar
518533 * @return array
519534 */
520 - protected function buildPersonalUrls( OutputPage $out ) {
521 - global $wgRequest;
522 -
523 - $title = $out->getTitle();
 535+ protected function buildPersonalUrls() {
 536+ $title = $this->getTitle();
 537+ $request = $this->getRequest();
524538 $pageurl = $title->getLocalURL();
525539 wfProfileIn( __METHOD__ );
526540
527541 /* set up the default links for the personal toolbar */
528542 $personal_urls = array();
529543
530 - $page = $wgRequest->getVal( 'returnto', $this->thispage );
531 - $query = $wgRequest->getVal( 'returntoquery', $this->thisquery );
 544+ $page = $request->getVal( 'returnto', $this->thispage );
 545+ $query = $request->getVal( 'returntoquery', $this->thisquery );
532546 $a = array( 'returnto' => $page );
533547 if( $query != '' ) {
534548 $a['returntoquery'] = $query;
——@@ -565,12 +579,12 @@
566580 # from request values or be specified in "sub page" form. The plot
567581 # thickens, because $wgTitle is altered for special pages, so doesn't
568582 # contain the original alias-with-subpage.
569 - $origTitle = Title::newFromText( $wgRequest->getText( 'title' ) );
 583+ $origTitle = Title::newFromText( $request->getText( 'title' ) );
570584 if( $origTitle instanceof Title && $origTitle->getNamespace() == NS_SPECIAL ) {
571585 list( $spName, $spPar ) = SpecialPageFactory::resolveAlias( $origTitle->getText() );
572586 $active = $spName == 'Contributions'
573587 && ( ( $spPar && $spPar == $this->username )
574 - || $wgRequest->getText( 'target' ) == $this->username );
 588+ || $request->getText( 'target' ) == $this->username );
575589 } else {
576590 $active = false;
577591 }
——@@ -591,12 +605,11 @@
592606 'active' => false
593607 );
594608 } else {
595 - global $wgUser;
596609 $useCombinedLoginLink = $this->useCombinedLoginLink();
597 - $loginlink = $wgUser->isAllowed( 'createaccount' ) && $useCombinedLoginLink
 610+ $loginlink = $this->getUser()->isAllowed( 'createaccount' ) && $useCombinedLoginLink
598611 ? 'nav-login-createaccount'
599612 : 'login';
600 - $is_signup = $wgRequest->getText('type') == "signup";
 613+ $is_signup = $request->getText('type') == "signup";
601614
602615 # anonlogin & login are the same
603616 $login_url = array(
——@@ -604,7 +617,7 @@
605618 'href' => self::makeSpecialUrl( 'Userlogin', $returnto ),
606619 'active' => $title->isSpecial( 'Userlogin' ) && ( $loginlink == "nav-login-createaccount" || !$is_signup )
607620 );
608 - if ( $wgUser->isAllowed( 'createaccount' ) && !$useCombinedLoginLink ) {
 621+ if ( $this->getUser()->isAllowed( 'createaccount' ) && !$useCombinedLoginLink ) {
609622 $createaccount_url = array(
610623 'text' => wfMsg( 'createaccount' ),
611624 'href' => self::makeSpecialUrl( 'Userlogin', "$returnto&type=signup" ),
——@@ -762,8 +775,8 @@
763776 *
764777 * @return array
765778 */
766 - protected function buildContentNavigationUrls( OutputPage $out ) {
767 - global $wgContLang, $wgLang, $wgUser, $wgRequest;
 779+ protected function buildContentNavigationUrls() {
 780+ global $wgContLang;
768781 global $wgDisableLangConversion;
769782
770783 wfProfileIn( __METHOD__ );
——@@ -771,6 +784,10 @@
772785 $title = $this->getRelevantTitle(); // Display tabs for the relevant title rather than always the title itself
773786 $onPage = $title->equals($this->getTitle());
774787
 788+ $out = $this->getOutput();
 789+ $request = $this->getRequest();
 790+ $user = $this->getUser();
 791+
775792 $content_navigation = array(
776793 'namespaces' => array(),
777794 'views' => array(),
——@@ -779,8 +796,8 @@
780797 );
781798
782799 // parameters
783 - $action = $wgRequest->getVal( 'action', 'view' );
784 - $section = $wgRequest->getVal( 'section' );
 800+ $action = $request->getVal( 'action', 'view' );
 801+ $section = $request->getVal( 'section' );
785802
786803 $userCanRead = $title->userCanRead();
787804 $skname = $this->skinname;
——@@ -898,7 +915,7 @@
899916 'rel' => 'archives',
900917 );
901918
902 - if( $wgUser->isAllowed( 'delete' ) ) {
 919+ if( $user->isAllowed( 'delete' ) ) {
903920 $content_navigation['actions']['delete'] = array(
904921 'class' => ( $onPage && $action == 'delete' ) ? 'selected' : false,
905922 'text' => wfMessageFallback( "$skname-action-delete", 'delete' )->text(),
——@@ -914,7 +931,7 @@
915932 );
916933 }
917934
918 - if ( $title->getNamespace() !== NS_MEDIAWIKI && $wgUser->isAllowed( 'protect' ) ) {
 935+ if ( $title->getNamespace() !== NS_MEDIAWIKI && $user->isAllowed( 'protect' ) ) {
919936 $mode = !$title->isProtected() ? 'protect' : 'unprotect';
920937 $content_navigation['actions'][$mode] = array(
921938 'class' => ( $onPage && $action == $mode ) ? 'selected' : false,
——@@ -924,23 +941,23 @@
925942 }
926943 } else {
927944 // article doesn't exist or is deleted
928 - if ( $wgUser->isAllowed( 'deletedhistory' ) && !$wgUser->isBlocked() ) {
929 - $includeSuppressed = $wgUser->isAllowed( 'suppressrevision' );
 945+ if ( $user->isAllowed( 'deletedhistory' ) && !$user->isBlocked() ) {
 946+ $includeSuppressed = $user->isAllowed( 'suppressrevision' );
930947 $n = $title->isDeleted( $includeSuppressed );
931948 if( $n ) {
932949 $undelTitle = SpecialPage::getTitleFor( 'Undelete' );
933950 // If the user can't undelete but can view deleted history show them a "View .. deleted" tab instead
934 - $msgKey = $wgUser->isAllowed( 'undelete' ) ? 'undelete' : 'viewdeleted';
 951+ $msgKey = $user->isAllowed( 'undelete' ) ? 'undelete' : 'viewdeleted';
935952 $content_navigation['actions']['undelete'] = array(
936953 'class' => $this->getTitle()->isSpecial( 'Undelete' ) ? 'selected' : false,
937954 'text' => wfMessageFallback( "$skname-action-$msgKey", "{$msgKey}_short" )
938 - ->params( $wgLang->formatNum( $n ) )->text(),
 955+ ->params( $this->getLang()->formatNum( $n ) )->text(),
939956 'href' => $undelTitle->getLocalURL( array( 'target' => $title->getPrefixedDBkey() ) )
940957 );
941958 }
942959 }
943960
944 - if ( $title->getNamespace() !== NS_MEDIAWIKI && $wgUser->isAllowed( 'protect' ) ) {
 961+ if ( $title->getNamespace() !== NS_MEDIAWIKI && $user->isAllowed( 'protect' ) ) {
945962 $mode = !$title->getRestrictions( 'create' ) ? 'protect' : 'unprotect';
946963 $content_navigation['actions'][$mode] = array(
947964 'class' => ( $onPage && $action == $mode ) ? 'selected' : false,
——@@ -963,7 +980,7 @@
964981 * the global versions.
965982 */
966983 $mode = $title->userIsWatching() ? 'unwatch' : 'watch';
967 - $token = WatchAction::getWatchToken( $title, $wgUser, $mode );
 984+ $token = WatchAction::getWatchToken( $title, $user, $mode );
968985 $content_navigation['actions'][$mode] = array(
969986 'class' => $onPage && ( $action == 'watch' || $action == 'unwatch' ) ? 'selected' : false,
970987 'text' => wfMsg( $mode ), // uses 'watch' or 'unwatch' message
——@@ -977,7 +994,7 @@
978995 $content_navigation['namespaces']['special'] = array(
979996 'class' => 'selected',
980997 'text' => wfMsg( 'nstab-special' ),
981 - 'href' => $wgRequest->getRequestURL(), // @bug 2457, 2510
 998+ 'href' => $request->getRequestURL(), // @bug 2457, 2510
982999 'context' => 'subject'
9831000 );
9841001
——@@ -1105,17 +1122,20 @@
11061123 * @return array
11071124 * @private
11081125 */
1109 - protected function buildNavUrls( OutputPage $out ) {
1110 - global $wgUseTrackbacks, $wgUser, $wgRequest;
 1126+ protected function buildNavUrls() {
 1127+ global $wgUseTrackbacks;
11111128 global $wgUploadNavigationUrl;
11121129
11131130 wfProfileIn( __METHOD__ );
11141131
 1132+ $out = $this->getOutput();
 1133+ $request = $this->getRequest();
 1134+
11151135 $nav_urls = array();
11161136 $nav_urls['mainpage'] = array( 'href' => self::makeMainPageUrl() );
11171137 if( $wgUploadNavigationUrl ) {
11181138 $nav_urls['upload'] = array( 'href' => $wgUploadNavigationUrl );
1119 - } elseif( UploadBase::isEnabled() && UploadBase::isAllowed( $wgUser ) === true ) {
 1139+ } elseif( UploadBase::isEnabled() && UploadBase::isAllowed( $this->getUser() ) === true ) {
11201140 $nav_urls['upload'] = array( 'href' => self::makeSpecialUrl( 'Upload' ) );
11211141 } else {
11221142 $nav_urls['upload'] = false;
——@@ -1132,7 +1152,7 @@
11331153 $nav_urls['print'] = array(
11341154 'text' => wfMsg( 'printableversion' ),
11351155 'href' => $this->getTitle()->getLocalURL(
1136 - $wgRequest->appendQueryValue( 'printable', 'yes', true ) )
 1156+ $request->appendQueryValue( 'printable', 'yes', true ) )
11371157 );
11381158 }
11391159
——@@ -1198,7 +1218,7 @@
11991219 $nav_urls['log'] = false;
12001220 }
12011221
1202 - if ( $wgUser->isAllowed( 'block' ) ) {
 1222+ if ( $this->getUser()->isAllowed( 'block' ) ) {
12031223 $nav_urls['blockip'] = array(
12041224 'href' => self::makeSpecialUrlSubpage( 'Block', $rootUser )
12051225 );
——@@ -1234,13 +1254,13 @@
12351255 * @todo FIXME: Why is this duplicated in/from OutputPage::getHeadScripts()??
12361256 */
12371257 function setupUserJs( $allowUserJs ) {
1238 - global $wgRequest, $wgJsMimeType;
 1258+ global $wgJsMimeType;
12391259 wfProfileIn( __METHOD__ );
12401260
12411261 if( $allowUserJs && $this->loggedin ) {
12421262 if( $this->getTitle()->isJsSubpage() and $this->getOutput()->userCanPreview() ) {
12431263 # XXX: additional security check/prompt?
1244 - $this->userjsprev = '/*<![CDATA[*/ ' . $wgRequest->getText( 'wpTextbox1' ) . ' /*]]>*/';
 1264+ $this->userjsprev = '/*<![CDATA[*/ ' . $this->getRequest()->getText( 'wpTextbox1' ) . ' /*]]>*/';
12451265 } else {
12461266 $this->userjs = self::makeUrl( $this->userpage . '/' . $this->skinname . '.js', 'action=raw&ctype=' . $wgJsMimeType );
12471267 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r95959Followup r95957, add =null to Skin::outputPage toodantman10:35, 1 September 2011
r112296Updates for r95957/r95959, committing for test on servertstarling03:53, 24 February 2012

Status & tagging log