From: jenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Date: Mon, 16 Sep 2019 23:16:16 +0000 (+0000)
Subject: Merge "Title::getTalkPage(): Restore behavior of interwiki-prefixed & fragment-only... 
X-Git-Tag: 1.34.0-rc.0~192
X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dcompta/comptes/journal.php?a=commitdiff_plain;h=f97d13a10c1b7dac7c4463c3ccc75e21a731c7a2;hp=cfd56e52d96e7c65215a4d9d2f10962794c67f88;p=lhc%2Fweb%2Fwiklou.git

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

diff --git a/includes/Title.php b/includes/Title.php
index de418a7163..0f5c3841ff 100644
--- 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 82ccb9ae32..608d59078e 100644
--- 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