From: daniel <dkinzler@wikimedia.org>
Date: Thu, 29 Aug 2019 13:25:16 +0000 (+0200)
Subject: Title::getTalkPage(): Restore behavior of interwiki-prefixed & fragment-only titles
X-Git-Tag: 1.34.0-rc.0~192^2
X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=53ef1fa67bad8ffde58bc3c9c0b232863578ea55;p=lhc%2Fweb%2Fwiklou.git

Title::getTalkPage(): Restore behavior of interwiki-prefixed & fragment-only titles

NamespaceInfo::getTalkPage will throw an exception for these.
With this patch, Title::getTalkPage() will be reverted to the old
behavior of returning an incorrect meaningless value. Logging has been
added to identify code paths that trigger this behavior.

This patch should be undone once all such code paths have been found and
fixed.

Bug: T227817
Change-Id: I4727c7bb54d6f712ddcab05ef278a08d728f5726
---

diff --git a/includes/Title.php b/includes/Title.php
index 96f196fe9b..0ef6b14d21 100644
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -1541,14 +1541,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 ) );
 	}
 
 	/**
@@ -1576,8 +1594,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;
 	}
 
 	/**
@@ -1590,8 +1651,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 6cfc3779fb..ea8f134330 100644
--- a/tests/phpunit/includes/TitleTest.php
+++ b/tests/phpunit/includes/TitleTest.php
@@ -837,8 +837,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,
+			],
 		];
 	}
 
@@ -898,6 +913,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