Deprecate skin functions that are not skin responsibilities

Skin shouldn't be responsible for providing requested revisionId
nor if that revision is the current revision.
The OutputPage object has all required information (both the
currentRevisionID and the current Title object).

Change-Id: I2dbae4c6968a2b3b3cea3e09977e9579609b4cc5

diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34
index 8730484..df25d30 100644 (file)
--- a/RELEASE-NOTES-1.34
+++ b/RELEASE-NOTES-1.34
@@ -431,6 +431,8 @@ because of Phabricator reports.
   engines.
 * Skin::escapeSearchLink() is deprecated. Use Skin::getSearchLink() or the skin
   template option 'searchaction' instead.
+* Skin::getRevisionId() and Skin::isRevisionCurrent() have been deprecated.
+  Use OutputPage::getRevisionId() and OutputPage::isRevisionCurrent() instead.
 * LoadBalancer::haveIndex() and LoadBalancer::isNonZeroLoad() have
   been deprecated.
 * FileBackend::getWikiId() has been deprecated.

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index f8b7502..3f2dcf7 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -1661,6 +1661,16 @@ class OutputPage extends ContextSource {
                return $this->mRevisionId;
        }
 
+       /**
+        * Whether the revision displayed is the latest revision of the page
+        *
+        * @since 1.34
+        * @return bool
+        */
+       public function isRevisionCurrent() {
+               return $this->mRevisionId == 0 || $this->mRevisionId == $this->getTitle()->getLatestRevID();
+       }
+
        /**
         * Set the timestamp of the revision which will be displayed. This is used
         * to avoid a extra DB call in Skin::lastModified().

diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index e46f99d..bcc257a 100644 (file)
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -308,6 +308,7 @@ abstract class Skin extends ContextSource {
        /**
         * Get the current revision ID
         *
+        * @deprecated since 1.34, use OutputPage::getRevisionId instead
         * @return int
         */
        public function getRevisionId() {
@@ -317,11 +318,11 @@ abstract class Skin extends ContextSource {
        /**
         * Whether the revision displayed is the latest revision of the page
         *
+        * @deprecated since 1.34, use OutputPage::isRevisionCurrent instead
         * @return bool
         */
        public function isRevisionCurrent() {
-               $revID = $this->getRevisionId();
-               return $revID == 0 || $revID == $this->getTitle()->getLatestRevID();
+               return $this->getOutput()->isRevisionCurrent();
        }
 
        /**
@@ -701,7 +702,7 @@ abstract class Skin extends ContextSource {
         * @return string HTML text with an URL
         */
        function printSource() {
-               $oldid = $this->getRevisionId();
+               $oldid = $this->getOutput()->getRevisionId();
                if ( $oldid ) {
                        $canonicalUrl = $this->getTitle()->getCanonicalURL( 'oldid=' . $oldid );
                        $url = htmlspecialchars( wfExpandIRI( $canonicalUrl ) );
@@ -830,7 +831,7 @@ abstract class Skin extends ContextSource {
        function getCopyright( $type = 'detect' ) {
                $linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
                if ( $type == 'detect' ) {
-                       if ( !$this->isRevisionCurrent()
+                       if ( !$this->getOutput()->isRevisionCurrent()
                                && !$this->msg( 'history_copyright' )->inContentLanguage()->isDisabled()
                        ) {
                                $type = 'history';
@@ -934,7 +935,8 @@ abstract class Skin extends ContextSource {
 
                # No cached timestamp, load it from the database
                if ( $timestamp === null ) {
-                       $timestamp = Revision::getTimestampFromId( $this->getTitle(), $this->getRevisionId() );
+                       $timestamp = Revision::getTimestampFromId( $this->getTitle(),
+                               $this->getOutput()->getRevisionId() );
                }
 
                if ( $timestamp ) {
@@ -1088,8 +1090,8 @@ abstract class Skin extends ContextSource {
        function editUrlOptions() {
                $options = [ 'action' => 'edit' ];
 
-               if ( !$this->isRevisionCurrent() ) {
-                       $options['oldid'] = intval( $this->getRevisionId() );
+               if ( !$this->getOutput()->isRevisionCurrent() ) {
+                       $options['oldid'] = intval( $this->getOutput()->getRevisionId() );
                }
 
                return $options;

diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php
index d1345b8..f348135 100644 (file)
--- a/includes/skins/SkinTemplate.php
+++ b/includes/skins/SkinTemplate.php
@@ -371,7 +371,7 @@ class SkinTemplate extends Skin {
                $tpl->set( 'credits', false );
                $tpl->set( 'numberofwatchingusers', false );
                if ( $title->exists() ) {
-                       if ( $out->isArticle() && $this->isRevisionCurrent() ) {
+                       if ( $out->isArticle() && $out->isRevisionCurrent() ) {
                                if ( $wgMaxCredits != 0 ) {
                                        /** @var CreditsAction $action */
                                        $action = Action::factory(
@@ -975,7 +975,7 @@ class SkinTemplate extends Skin {
                                        // Whether to show the "Add a new section" tab
                                        // Checks if this is a current rev of talk page and is not forced to be hidden
                                        $showNewSection = !$out->forceHideNewSectionLink()
-                                               && ( ( $isTalk && $this->isRevisionCurrent() ) || $out->showNewSectionLink() );
+                                               && ( ( $isTalk && $out->isRevisionCurrent() ) || $out->showNewSectionLink() );
                                        $section = $request->getVal( 'section' );
 
                                        if ( $title->exists()
@@ -1295,7 +1295,7 @@ class SkinTemplate extends Skin {
 
                if ( $out->isArticle() ) {
                        // Also add a "permalink" while we're at it
-                       $revid = $this->getRevisionId();
+                       $revid = $this->getOutput()->getRevisionId();
                        if ( $revid ) {
                                $nav_urls['permalink'] = [
                                        'text' => $this->msg( 'permalink' )->text(),

diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 00b8d18..6520fc5 100644 (file)
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -3029,6 +3029,35 @@ class OutputPageTest extends MediaWikiTestCase {
                ];
        }
 
+       /**
+        * @param int $titleLastRevision Last Title revision to set
+        * @param int $outputRevision Revision stored in OutputPage
+        * @param bool $expectedResult Expected result of $output->isRevisionCurrent call
+        * @covers OutputPage::isRevisionCurrent
+        * @dataProvider provideIsRevisionCurrent
+        */
+       public function testIsRevisionCurrent( $titleLastRevision, $outputRevision, $expectedResult ) {
+               $titleMock = $this->getMock( Title::class, [], [], '', false );
+               $titleMock->expects( $this->any() )
+                       ->method( 'getLatestRevID' )
+                       ->willReturn( $titleLastRevision );
+
+               $output = $this->newInstance( [], null, [ 'notitle' => true ] );
+               $output->setTitle( $titleMock );
+               $output->setRevisionId( $outputRevision );
+               $this->assertEquals( $expectedResult, $output->isRevisionCurrent() );
+       }
+
+       public function provideIsRevisionCurrent() {
+               return [
+                       [ 10, null, true ],
+                       [ 42, 42, true ],
+                       [ null, 0, true ],
+                       [ 42, 47, false ],
+                       [ 47, 42, false ]
+               ];
+       }
+
        /**
         * @return OutputPage
         */