Some misc fixes to Skin and SkinTemplate:
authorAlexandre Emsenhuber <ialex@users.mediawiki.org>
Sun, 24 Jul 2011 17:05:42 +0000 (17:05 +0000)
committerAlexandre Emsenhuber <ialex@users.mediawiki.org>
Sun, 24 Jul 2011 17:05:42 +0000 (17:05 +0000)
* Removed Skin::getStylesheet(), not called from anywhere
* Removed Skin::setMembers() and moved the definition of the $userpage member to SkinTemplate::outputPage() since it's only used in that class
* Removed useless $out parameter of Skin::bottomScripts()
* Call Linker methods statically
* Use Linker::linkKnown() instead of Linker::link() if possible
* Escape 'mainpage' message when used for description
* Removed some variables from SkinTemplate::outputPage(): nscanonical, nsnumber, currevisionid that were used when the JavaScript global variables were defined in the template. Also changed pageclass and skinnameclass to be defined only when $useHeadElement is false
* Removed some unused members of SkinTemplate: $iscontent and $iseditable
* Refactored the part of SkinTemplate::outputPage() that sets copyright, viewcount, lastmod, credits and numberofwatchingusers to always define them to false first and set them when needed. Also modified the check to display the copyright; it will now only be displayed when $out->isArticle() is true and the page exists, since it is currently possible to display it on any page by adding the oldid parameter to the url.

includes/Skin.php
includes/SkinTemplate.php

index ae98951..4ac8bbb 100644 (file)
@@ -160,11 +160,6 @@ abstract class Skin extends ContextSource {
                return $skin;
        }
 
-       /** @return string path to the skin stylesheet */
-       function getStylesheet() {
-               return 'common/wikistandard.css';
-       }
-
        /** @return string skin name */
        public function getSkinName() {
                return $this->skinname;
@@ -174,7 +169,6 @@ abstract class Skin extends ContextSource {
                wfProfileIn( __METHOD__ );
 
                $this->preloadExistence();
-               $this->setMembers();
 
                wfProfileOut( __METHOD__ );
        }
@@ -202,13 +196,6 @@ abstract class Skin extends ContextSource {
                $lb->execute();
        }
 
-       /**
-        * Set some local variables
-        */
-       protected function setMembers() {
-               $this->userpage = $this->getUser()->getUserPage()->getPrefixedText();
-       }
-
        /**
         * Get the current revision ID
         *
@@ -417,15 +404,15 @@ abstract class Skin extends ContextSource {
                global $wgUseCategoryBrowser;
 
                $out = $this->getOutput();
+               $allCats = $out->getCategoryLinks();
 
-               if ( count( $out->mCategoryLinks ) == 0 ) {
+               if ( !count( $allCats ) ) {
                        return '';
                }
 
                $embed = "<li>";
                $pop = "</li>";
 
-               $allCats = $out->getCategoryLinks();
                $s = '';
                $colon = wfMsgExt( 'colon-separator', 'escapenoentities' );
 
@@ -493,7 +480,7 @@ abstract class Skin extends ContextSource {
 
                        # add our current element to the list
                        $eltitle = Title::newFromText( $element );
-                       $return .=  Linker::link( $eltitle, $eltitle->getText() );
+                       $return .=  Linker::link( $eltitle, htmlspecialchars( $eltitle->getText() ) );
                }
 
                return $return;
@@ -622,14 +609,14 @@ abstract class Skin extends ContextSource {
 
        /**
         * This gets called shortly before the </body> tag.
-        * @param $out OutputPage object
+        *
         * @return String HTML-wrapped JS code to be put before </body>
         */
-       function bottomScripts( $out ) {
+       function bottomScripts() {
                // TODO and the suckage continues. This function is really just a wrapper around
                // OutputPage::getBottomScripts() which takes a Skin param. This should be cleaned
                // up at some point
-               $bottomScriptText = $out->getBottomScripts();
+               $bottomScriptText = $this->getOutput()->getBottomScripts();
                wfRunHooks( 'SkinAfterBottomScripts', array( $this, &$bottomScriptText ) );
 
                return $bottomScriptText;
@@ -659,12 +646,9 @@ abstract class Skin extends ContextSource {
 
                                return wfMsg(
                                        $msg,
-                                       Linker::link(
+                                       Linker::linkKnown(
                                                SpecialPage::getTitleFor( 'Undelete', $this->getTitle()->getPrefixedDBkey() ),
-                                               wfMsgExt( 'restorelink', array( 'parsemag', 'escape' ), $this->getLang()->formatNum( $n ) ),
-                                               array(),
-                                               array(),
-                                               array( 'known', 'noclasses' )
+                                               wfMsgExt( 'restorelink', array( 'parsemag', 'escape' ), $this->getLang()->formatNum( $n ) )
                                        )
                                );
                        }
@@ -696,12 +680,9 @@ abstract class Skin extends ContextSource {
                                        $linkObj = Title::newFromText( $growinglink );
 
                                        if ( is_object( $linkObj ) && $linkObj->exists() ) {
-                                               $getlink = $this->link(
+                                               $getlink = Linker::linkKnown(
                                                        $linkObj,
-                                                       htmlspecialchars( $display ),
-                                                       array(),
-                                                       array(),
-                                                       array( 'known', 'noclasses' )
+                                                       htmlspecialchars( $display )
                                                );
 
                                                $c++;
@@ -864,7 +845,7 @@ abstract class Skin extends ContextSource {
                        $a = '';
                }
 
-               $mp = wfMsg( 'mainpage' );
+               $mp = wfMsgHtml( 'mainpage' );
                $mptitle = Title::newMainPage();
                $url = ( is_object( $mptitle ) ? $mptitle->escapeLocalURL() : '' );
 
@@ -903,12 +884,9 @@ abstract class Skin extends ContextSource {
         * @return string
         */
        function mainPageLink() {
-               $s = Linker::link(
+               $s = Linker::linkKown(
                        Title::newMainPage(),
-                       wfMsg( 'mainpage' ),
-                       array(),
-                       array(),
-                       array( 'known', 'noclasses' )
+                       wfMsgHtml( 'mainpage' )
                );
 
                return $s;
@@ -1274,20 +1252,18 @@ abstract class Skin extends ContextSource {
                        $userTalkTitle = $userTitle->getTalkPage();
 
                        if ( !$userTalkTitle->equals( $out->getTitle() ) ) {
-                               $newMessagesLink = $this->link(
+                               $newMessagesLink = Linker::linkKown(
                                        $userTalkTitle,
                                        wfMsgHtml( 'newmessageslink' ),
                                        array(),
-                                       array( 'redirect' => 'no' ),
-                                       array( 'known', 'noclasses' )
+                                       array( 'redirect' => 'no' )
                                );
 
-                               $newMessagesDiffLink = $this->link(
+                               $newMessagesDiffLink = Linker::linkKown(
                                        $userTalkTitle,
                                        wfMsgHtml( 'newmessagesdifflink' ),
                                        array(),
-                                       array( 'diff' => 'cur' ),
-                                       array( 'known', 'noclasses' )
+                                       array( 'diff' => 'cur' )
                                );
 
                                $ntl = wfMsg(
index 18fab0c..4cef194 100644 (file)
@@ -145,10 +145,6 @@ class SkinTemplate extends Skin {
                wfProfileIn( __METHOD__ );
                Profiler::instance()->setTemplated( true );
 
-               $oldid = $wgRequest->getVal( 'oldid' );
-               $diff = $wgRequest->getVal( 'diff' );
-               $action = $wgRequest->getVal( 'action', 'view' );
-
                wfProfileIn( __METHOD__ . '-init' );
                $this->initPage( $out );
 
@@ -157,6 +153,7 @@ class SkinTemplate extends Skin {
 
                wfProfileIn( __METHOD__ . '-stuff' );
                $this->thispage = $this->getTitle()->getPrefixedDBkey();
+               $this->userpage = $this->getUser()->getUserPage()->getPrefixedText();
                $query = array();
                if ( !$wgRequest->wasPosted() ) {
                        $query = $wgRequest->getValues();
@@ -166,8 +163,6 @@ class SkinTemplate extends Skin {
                }
                $this->thisquery = wfArrayToCGI( $query );
                $this->loggedin = $wgUser->isLoggedIn();
-               $this->iscontent = ( $this->getTitle()->getNamespace() != NS_SPECIAL );
-               $this->iseditable = ( $this->iscontent and !( $action == 'edit' or $action == 'submit' ) );
                $this->username = $wgUser->getName();
 
                if ( $wgUser->isLoggedIn() || $this->showIPinHeader() ) {
@@ -183,8 +178,6 @@ class SkinTemplate extends Skin {
 
                wfProfileIn( __METHOD__ . '-stuff-head' );
                if ( !$this->useHeadElement ) {
-                       $this->setupUserCss( $out );
-
                        $tpl->set( 'pagecss', false );
                        $tpl->set( 'usercss', false );
 
@@ -216,6 +209,9 @@ class SkinTemplate extends Skin {
                        } else {
                                $tpl->set( 'trackbackhtml', null );
                        }
+
+                       $tpl->set( 'pageclass', $this->getPageClasses( $this->getTitle() ) );
+                       $tpl->set( 'skinnameclass', ( 'skin-' . Sanitizer::escapeClass( $this->getSkinName() ) ) );
                }
                wfProfileOut( __METHOD__ . '-stuff-head' );
 
@@ -223,19 +219,10 @@ class SkinTemplate extends Skin {
                $tpl->set( 'title', $out->getPageTitle() );
                $tpl->set( 'pagetitle', $out->getHTMLTitle() );
                $tpl->set( 'displaytitle', $out->mPageLinkTitle );
-               $tpl->set( 'pageclass', $this->getPageClasses( $this->getTitle() ) );
-               $tpl->set( 'skinnameclass', ( 'skin-' . Sanitizer::escapeClass( $this->getSkinName() ) ) );
 
-               $nsname = MWNamespace::exists( $this->getTitle()->getNamespace() ) ?
-                                       MWNamespace::getCanonicalName( $this->getTitle()->getNamespace() ) :
-                                       $this->getTitle()->getNsText();
-
-               $tpl->set( 'nscanonical', $nsname );
-               $tpl->set( 'nsnumber', $this->getTitle()->getNamespace() );
                $tpl->set( 'titleprefixeddbkey', $this->getTitle()->getPrefixedDBKey() );
                $tpl->set( 'titletext', $this->getTitle()->getText() );
                $tpl->set( 'articleid', $this->getTitle()->getArticleId() );
-               $tpl->set( 'currevisionid', $this->getTitle()->getLatestRevID() );
 
                $tpl->set( 'isarticle', $out->isArticle() );
 
@@ -321,74 +308,49 @@ class SkinTemplate extends Skin {
                        $tpl->set( 'userlangattributes', $attrs );
                }
 
-               $newtalks = $this->getNewtalks( $out );
-
                wfProfileOut( __METHOD__ . '-stuff2' );
 
                wfProfileIn( __METHOD__ . '-stuff3' );
-               $tpl->setRef( 'newtalk', $newtalks );
+               $tpl->set( 'newtalk', $this->getNewtalks() );
                $tpl->setRef( 'skin', $this );
                $tpl->set( 'logo', $this->logoText() );
-               if ( $out->isArticle() && ( !isset( $oldid ) || isset( $diff ) ) &&
-                       $this->getTitle()->exists() )
-               {
-                       $article = new Article( $this->getTitle(), 0 );
-                       if ( !$wgDisableCounters ) {
-                               $viewcount = $wgLang->formatNum( $article->getCount() );
-                               if ( $viewcount ) {
-                                       $tpl->set( 'viewcount', wfMsgExt( 'viewcount', array( 'parseinline' ), $viewcount ) );
-                               } else {
-                                       $tpl->set( 'viewcount', false );
+
+               $tpl->set( 'copyright', false );
+               $tpl->set( 'viewcount', false );
+               $tpl->set( 'lastmod', false );
+               $tpl->set( 'credits', false );
+               $tpl->set( 'numberofwatchingusers', false );
+               if ( $out->isArticle() && $this->getTitle()->exists() ) {
+                       if ( $this->isRevisionCurrent() ) {
+                               $article = new Article( $this->getTitle(), 0 );
+                               if ( !$wgDisableCounters ) {
+                                       $viewcount = $wgLang->formatNum( $article->getCount() );
+                                       if ( $viewcount ) {
+                                               $tpl->set( 'viewcount', wfMsgExt( 'viewcount', array( 'parseinline' ), $viewcount ) );
+                                       }
                                }
-                       } else {
-                               $tpl->set( 'viewcount', false );
-                       }
 
-                       if( $wgPageShowWatchingUsers ) {
-                               $dbr = wfGetDB( DB_SLAVE );
-                               $res = $dbr->select( 'watchlist',
-                                       array( 'COUNT(*) AS n' ),
-                                       array( 'wl_title' => $dbr->strencode( $this->getTitle()->getDBkey() ), 'wl_namespace' => $this->getTitle()->getNamespace() ),
-                                       __METHOD__
-                               );
-                               $x = $dbr->fetchObject( $res );
-                               $numberofwatchingusers = $x->n;
-                               if( $numberofwatchingusers > 0 ) {
-                                       $tpl->set( 'numberofwatchingusers',
-                                               wfMsgExt( 'number_of_watching_users_pageview', array( 'parseinline' ),
-                                               $wgLang->formatNum( $numberofwatchingusers ) )
+                               if( $wgPageShowWatchingUsers ) {
+                                       $dbr = wfGetDB( DB_SLAVE );
+                                       $num = $dbr->selectField( 'watchlist', 'COUNT(*)',
+                                               array( 'wl_title' => $this->getTitle()->getDBkey(), 'wl_namespace' => $this->getTitle()->getNamespace() ),
+                                               __METHOD__
                                        );
-                               } else {
-                                       $tpl->set( 'numberofwatchingusers', false );
+                                       if( $num > 0 ) {
+                                               $tpl->set( 'numberofwatchingusers',
+                                                       wfMsgExt( 'number_of_watching_users_pageview', array( 'parseinline' ),
+                                                       $wgLang->formatNum( $num ) )
+                                               );
+                                       }
                                }
-                       } else {
-                               $tpl->set( 'numberofwatchingusers', false );
-                       }
-
-                       $tpl->set( 'copyright', $this->getCopyright() );
-
-                       $this->credits = false;
 
-                       if( $wgMaxCredits != 0 ){
-                               $this->credits = Action::factory( 'credits', $article )->getCredits( $wgMaxCredits, $wgShowCreditsIfMax );
-                       } else {
-                               $tpl->set( 'lastmod', $this->lastModified( $article ) );
+                               if ( $wgMaxCredits != 0 ) {
+                                       $tpl->set( 'credits', Action::factory( 'credits', $article )->getCredits( $wgMaxCredits, $wgShowCreditsIfMax ) );
+                               } else {
+                                       $tpl->set( 'lastmod', $this->lastModified( $article ) );
+                               }
                        }
-
-                       $tpl->setRef( 'credits', $this->credits );
-
-               } elseif ( isset( $oldid ) && !isset( $diff ) ) {
                        $tpl->set( 'copyright', $this->getCopyright() );
-                       $tpl->set( 'viewcount', false );
-                       $tpl->set( 'lastmod', false );
-                       $tpl->set( 'credits', false );
-                       $tpl->set( 'numberofwatchingusers', false );
-               } else {
-                       $tpl->set( 'copyright', false );
-                       $tpl->set( 'viewcount', false );
-                       $tpl->set( 'lastmod', false );
-                       $tpl->set( 'credits', false );
-                       $tpl->set( 'numberofwatchingusers', false );
                }
                wfProfileOut( __METHOD__ . '-stuff3' );
 
@@ -441,14 +403,14 @@ class SkinTemplate extends Skin {
 
                $tpl->set( 'reporttime', wfReportTime() );
                $tpl->set( 'sitenotice', $this->getSiteNotice() );
-               $tpl->set( 'bottomscripts', $this->bottomScripts( $out ) );
+               $tpl->set( 'bottomscripts', $this->bottomScripts() );
                $tpl->set( 'printfooter', $this->printSource() );
 
                # Add a <div class="mw-content-ltr/rtl"> around the body text
                # not for special pages or file pages AND only when viewing AND if the page exists
                # (or is in MW namespace, because that has default content)
                if( !in_array( $this->getTitle()->getNamespace(), array( NS_SPECIAL, NS_FILE ) ) &&
-                       in_array( $action, array( 'view', 'historysubmit' ) ) &&
+                       $wgRequest->getVal( 'action', 'view' ) == 'view' &&
                        ( $this->getTitle()->exists() || $this->getTitle()->getNamespace() == NS_MEDIAWIKI ) ) {
                        $pageLang = $this->getTitle()->getPageLanguage();
                        $realBodyAttribs = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(),
@@ -1167,7 +1129,7 @@ class SkinTemplate extends Skin {
 
                // A print stylesheet is attached to all pages, but nobody ever
                // figures that out. :)  Add a link...
-               if( $this->iscontent && ( $action == 'view' || $action == 'purge' ) ) {
+               if( $this->getTitle()->getNamespace() != NS_SPECIAL && ( $action == 'view' || $action == 'purge' ) ) {
                        if ( !$out->isPrintable() ) {
                                $nav_urls['print'] = array(
                                        'text' => wfMsg( 'printableversion' ),