Merge "Title::getTalkPage(): Restore behavior of interwiki-prefixed & fragment-only...

diff --git a/includes/Title.php b/includes/Title.php
index de418a7..0f5c384 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -1561,14 +1561,32 @@ class Title implements LinkTarget, IDBAccessObject {
         * Get a Title object associated with the talk page of this article
         *
         * @deprecated since 1.34, use getTalkPageIfDefined() or NamespaceInfo::getTalkPage()
-        *             with NamespaceInfo::canHaveTalkPage().
+        *             with NamespaceInfo::canHaveTalkPage(). Note that the new method will
+        *             throw if asked for the talk page of a section-only link, or of an interwiki
+        *             link.
         * @return Title The object for the talk page
         * @throws MWException if $target doesn't have talk pages, e.g. because it's in NS_SPECIAL
         *         or because it's a relative link, or an interwiki link.
         */
        public function getTalkPage() {
-               return self::castFromLinkTarget(
-                       MediaWikiServices::getInstance()->getNamespaceInfo()->getTalkPage( $this ) );
+               // NOTE: The equivalent code in NamespaceInfo is less lenient about producing invalid titles.
+               //       Instead of failing on invalid titles, let's just log the issue for now.
+               //       See the discussion on T227817.
+
+               // Is this the same title?
+               $talkNS = MediaWikiServices::getInstance()->getNamespaceInfo()->getTalk( $this->mNamespace );
+               if ( $this->mNamespace == $talkNS ) {
+                       return $this;
+               }
+
+               $title = self::makeTitle( $talkNS, $this->mDbkeyform );
+
+               $this->warnIfPageCannotExist( $title, __METHOD__ );
+
+               return $title;
+               // TODO: replace the above with the code below:
+               // return self::castFromLinkTarget(
+               // MediaWikiServices::getInstance()->getNamespaceInfo()->getTalkPage( $this ) );
        }
 
        /**
@@ -1596,8 +1614,51 @@ class Title implements LinkTarget, IDBAccessObject {
         * @return Title The object for the subject page
         */
        public function getSubjectPage() {
-               return self::castFromLinkTarget(
-                       MediaWikiServices::getInstance()->getNamespaceInfo()->getSubjectPage( $this ) );
+               // Is this the same title?
+               $subjectNS = MediaWikiServices::getInstance()->getNamespaceInfo()
+                       ->getSubject( $this->mNamespace );
+               if ( $this->mNamespace == $subjectNS ) {
+                       return $this;
+               }
+               // NOTE: The equivalent code in NamespaceInfo is less lenient about producing invalid titles.
+               //       Instead of failing on invalid titles, let's just log the issue for now.
+               //       See the discussion on T227817.
+               $title = self::makeTitle( $subjectNS, $this->mDbkeyform );
+
+               $this->warnIfPageCannotExist( $title, __METHOD__ );
+
+               return $title;
+               // TODO: replace the above with the code below:
+               // return self::castFromLinkTarget(
+               // MediaWikiServices::getInstance()->getNamespaceInfo()->getSubjectPage( $this ) );
+       }
+
+       /**
+        * @param Title $title
+        * @param string $method
+        *
+        * @return bool whether a warning was issued
+        */
+       private function warnIfPageCannotExist( Title $title, $method ) {
+               if ( $this->getText() == '' ) {
+                       wfLogWarning(
+                               $method . ': called on empty title ' . $this->getFullText() . ', returning '
+                               . $title->getFullText()
+                       );
+
+                       return true;
+               }
+
+               if ( $this->getInterwiki() !== '' ) {
+                       wfLogWarning(
+                               $method . ': called on interwiki title ' . $this->getFullText() . ', returning '
+                               . $title->getFullText()
+                       );
+
+                       return true;
+               }
+
+               return false;
        }
 
        /**
@@ -1610,8 +1671,23 @@ class Title implements LinkTarget, IDBAccessObject {
         * @return Title
         */
        public function getOtherPage() {
-               return self::castFromLinkTarget(
-                       MediaWikiServices::getInstance()->getNamespaceInfo()->getAssociatedPage( $this ) );
+               // NOTE: Depend on the methods in this class instead of their equivalent in NamespaceInfo,
+               //       until their semantics has become exactly the same.
+               //       See the discussion on T227817.
+               if ( $this->isSpecialPage() ) {
+                       throw new MWException( 'Special pages cannot have other pages' );
+               }
+               if ( $this->isTalkPage() ) {
+                       return $this->getSubjectPage();
+               } else {
+                       if ( !$this->canHaveTalkPage() ) {
+                               throw new MWException( "{$this->getPrefixedText()} does not have an other page" );
+                       }
+                       return $this->getTalkPage();
+               }
+               // TODO: replace the above with the code below:
+               // return self::castFromLinkTarget(
+               // MediaWikiServices::getInstance()->getNamespaceInfo()->getAssociatedPage( $this ) );
        }
 
        /**

diff --git a/tests/phpunit/includes/TitleTest.php b/tests/phpunit/includes/TitleTest.php
index 82ccb9a..608d590 100644 (file)
--- a/tests/phpunit/includes/TitleTest.php
+++ b/tests/phpunit/includes/TitleTest.php
@@ -834,8 +834,23 @@ class TitleTest extends MediaWikiTestCase {
                return [
                        [ Title::makeTitle( NS_SPECIAL, 'Test' ) ],
                        [ Title::makeTitle( NS_MEDIA, 'Test' ) ],
-                       [ Title::makeTitle( NS_MAIN, '', 'Kittens' ) ],
-                       [ Title::makeTitle( NS_MAIN, 'Kittens', '', 'acme' ) ],
+               ];
+       }
+
+       public static function provideGetTalkPage_broken() {
+               // These cases *should* be bad, but are not treated as bad, for backwards compatibility.
+               // See discussion on T227817.
+               return [
+                       [
+                               Title::makeTitle( NS_MAIN, '', 'Kittens' ),
+                               Title::makeTitle( NS_TALK, '' ), // Section is lost!
+                               false,
+                       ],
+                       [
+                               Title::makeTitle( NS_MAIN, 'Kittens', '', 'acme' ),
+                               Title::makeTitle( NS_TALK, 'Kittens', '' ), // Interwiki prefix is lost!
+                               true,
+                       ],
                ];
        }
 
@@ -895,6 +910,23 @@ class TitleTest extends MediaWikiTestCase {
                $title->getTalkPage();
        }
 
+       /**
+        * @dataProvider provideGetTalkPage_broken
+        * @covers Title::getTalkPageIfDefined
+        */
+       public function testGetTalkPage_broken( Title $title, Title $expected, $valid ) {
+               $errorLevel = error_reporting( E_ERROR );
+
+               // NOTE: Eventually we want to throw in this case. But while there is still code that
+               // calls this method without checking, we want to avoid fatal errors.
+               // See discussion on T227817.
+               $result = $title->getTalkPage();
+               $this->assertTrue( $expected->equals( $result ) );
+               $this->assertSame( $valid, $result->isValid() );
+
+               error_reporting( $errorLevel );
+       }
+
        /**
         * @dataProvider provideGetTalkPage_good
         * @covers Title::getTalkPageIfDefined