Revert "Use setExtensionData() instead of marker comments where possible (2/3)"

This reverts commit 0ac420ecbc.

Reason for revert: this was supposed to be merged later; revert it now and reapply in a bit

Change-Id: I33fb07856152c2401b3a071c143f27f1e9753287

includes/CommentFormatter.php

@@ -707,46 +707,52 @@ class CommentFormatter {
 	/**
 	 * Post-process visual enhancements features for page subtitle
 	 *
-	 * @param ParserOutput $pout
+	 * @param string &$text
 	 * @param Language $lang
 	 * @param UserIdentity $user
 	 * @return ?string
 	 */
 	public static function postprocessVisualEnhancementsSubtitle(
-		ParserOutput $pout, Language $lang, UserIdentity $user
+		string &$text, Language $lang, UserIdentity $user
 	): ?string {
-		$itemData = $pout->getExtensionData( 'DiscussionTools-newestComment' );
+		preg_match( '/<!--__DTLATESTCOMMENTPAGE__(.*?)__-->/', $text, $matches );
-		if ( $itemData && $itemData['timestamp'] && $itemData['id'] ) {
+		// The subtitle is inserted into the correct place by the caller, so cleanup the comment here
-			$relativeTime = static::getSignatureRelativeTime(
+		$text = preg_replace( '/<!--__DTLATESTCOMMENTPAGE__(.*?)-->/', '', $text );
-				new MWTimestamp( $itemData['timestamp'] ),
+		if ( count( $matches ) ) {
-				$lang,
+			$itemData = json_decode( htmlspecialchars_decode( $matches[1] ), true );
-				$user
-			);
-			$commentLink = Html::element( 'a', [
-				'href' => '#' . Sanitizer::escapeIdForLink( $itemData['id'] )
-			], $relativeTime );
 
-			if ( isset( $itemData['heading'] ) ) {
+			if ( $itemData && $itemData['timestamp'] && $itemData['id'] ) {
-				$headingLink = Html::element( 'a', [
+				$relativeTime = static::getSignatureRelativeTime(
-					'href' => '#' . Sanitizer::escapeIdForLink( $itemData['heading']['linkableTitle'] )
+					new MWTimestamp( $itemData['timestamp'] ),
-				], $itemData['heading']['text'] );
+					$lang,
-				$label = wfMessage( 'discussiontools-pageframe-latestcomment' )
+					$user
-					->rawParams( $commentLink )
+				);
-					->params( $itemData['author'] )
+				$commentLink = Html::element( 'a', [
-					->rawParams( $headingLink )
+					'href' => '#' . Sanitizer::escapeIdForLink( $itemData['id'] )
-					->inLanguage( $lang )->escaped();
+				], $relativeTime );
-			} else {
+
-				$label = wfMessage( 'discussiontools-pageframe-latestcomment-notopic' )
+				if ( isset( $itemData['heading'] ) ) {
-					->rawParams( $commentLink )
+					$headingLink = Html::element( 'a', [
-					->params( $itemData['author'] )
+						'href' => '#' . Sanitizer::escapeIdForLink( $itemData['heading']['linkableTitle'] )
-					->inLanguage( $lang )->escaped();
+					], $itemData['heading']['text'] );
+					$label = wfMessage( 'discussiontools-pageframe-latestcomment' )
+						->rawParams( $commentLink )
+						->params( $itemData['author'] )
+						->rawParams( $headingLink )
+						->inLanguage( $lang )->escaped();
+				} else {
+					$label = wfMessage( 'discussiontools-pageframe-latestcomment-notopic' )
+						->rawParams( $commentLink )
+						->params( $itemData['author'] )
+						->inLanguage( $lang )->escaped();
+				}
+
+				return Html::rawElement(
+					'div',
+					[ 'class' => 'ext-discussiontools-init-pageframe-latestcomment' ],
+					$label
+				);
 			}
-
-			return Html::rawElement(
-				'div',
-				[ 'class' => 'ext-discussiontools-init-pageframe-latestcomment' ],
-				$label
-			);
 		}
 		return null;
 	}
@@ -790,43 +796,42 @@ class CommentFormatter {
 	/**
 	 * Check if the talk page had no comments or headings.
 	 *
-	 * @param ParserOutput $pout
+	 * @param string $text
 	 * @return bool
 	 */
-	public static function isEmptyTalkPage( ParserOutput $pout ): bool {
+	public static function isEmptyTalkPage( string $text ): bool {
-		return $pout->getExtensionData( 'DiscussionTools-isEmptyTalkPage' ) === true;
+		return strpos( $text, '<!--__DTEMPTYTALKPAGE__-->' ) !== false;
 	}
 
 	/**
 	 * Append content to an empty talk page
 	 *
-	 * @param ParserOutput $pout
+	 * @param string $text
 	 * @param string $content
+	 * @return string
 	 */
-	public static function appendToEmptyTalkPage( ParserOutput $pout, string $content ): void {
+	public static function appendToEmptyTalkPage( string $text, string $content ): string {
-		$text = $pout->getRawText();
+		return str_replace( '<!--__DTEMPTYTALKPAGE__-->', $content, $text );
-		$text .= $content;
-		$pout->setText( $text );
 	}
 
 	/**
 	 * Check if the talk page has content above the first heading, in the lede section.
 	 *
-	 * @param ParserOutput $pout
+	 * @param string $text
 	 * @return bool
 	 */
-	public static function hasLedeContent( ParserOutput $pout ): bool {
+	public static function hasLedeContent( string $text ): bool {
-		return $pout->getExtensionData( 'DiscussionTools-hasLedeContent' ) === true;
+		return strpos( $text, '<!--__DTHASLEDECONTENT__-->' ) !== false;
 	}
 
 	/**
 	 * Check if the talk page has comments above the first heading, in the lede section.
 	 *
-	 * @param ParserOutput $pout
+	 * @param string $text
 	 * @return bool
 	 */
-	public static function hasCommentsInLedeContent( ParserOutput $pout ): bool {
+	public static function hasCommentsInLedeContent( string $text ): bool {
-		return $pout->getExtensionData( 'DiscussionTools-hasCommentsInLedeContent' ) === true;
+		return strpos( $text, '<!--__DTHASCOMMENTSINLEDECONTENT__-->' ) !== false;
 	}
 
 	public static function isLanguageRequiringReplyIcon( Language $lang ): bool {

includes/Hooks/PageHooks.php

@@ -176,42 +176,6 @@ class PageHooks implements
 		if ( $output->getSkin()->getSkinName() === 'minerva' ) {
 			$title = $output->getTitle();
 
-			if (
-				$title->isTalkPage() &&
-				HookUtils::isFeatureEnabledForOutput( $output, HookUtils::REPLYTOOL ) && (
-					// 'DiscussionTools-ledeButton' property may be already set to true or false.
-					// Examine the other conditions only if it's unset.
-					$output->getProperty( 'DiscussionTools-ledeButton' ) ?? (
-						// Header shown on all talk pages, see Article::showNamespaceHeader
-						!$output->getContext()->msg( 'talkpageheader' )->isDisabled() &&
-						// Check if it isn't empty since it may use parser functions to only show itself on some pages
-						trim( $output->getContext()->msg( 'talkpageheader' )->text() ) !== ''
-					)
-				)
-			) {
-				$output->addBodyClasses( 'ext-discussiontools-init-lede-hidden' );
-				$output->enableOOUI();
-				$output->prependHTML(
-					Html::rawElement( 'div',
-						[ 'class' => 'ext-discussiontools-init-lede-button-container' ],
-						( new ButtonWidget( [
-							'label' => $output->getContext()->msg( 'discussiontools-ledesection-button' )->text(),
-							'classes' => [ 'ext-discussiontools-init-lede-button' ],
-							'framed' => false,
-							'icon' => 'info',
-							'infusable' => true,
-						] ) )
-					)
-				);
-
-				// Preload jquery.makeCollapsible for LedeSectionDialog.
-				// Using the same approach as in Skin::getDefaultModules in MediaWiki core.
-				if ( strpos( $output->getHTML(), 'mw-collapsible' ) !== false ) {
-					$output->addModules( 'jquery.makeCollapsible' );
-					$output->addModuleStyles( 'jquery.makeCollapsible.styles' );
-				}
-			}
-
 			if (
 				HookUtils::isFeatureEnabledForOutput( $output, HookUtils::NEWTOPICTOOL ) &&
 				// Only add the button if "New section" tab would be shown in a normal skin.
@@ -240,6 +204,43 @@ class PageHooks implements
 						->setAttributes( [ 'data-event-name' => 'talkpage.add-topic' ] )
 				) );
 			}
+
+			if (
+				$title->isTalkPage() &&
+				HookUtils::isFeatureEnabledForOutput( $output, HookUtils::REPLYTOOL ) && (
+					CommentFormatter::hasLedeContent( $output->getHTML() ) || (
+						// Header shown on all talk pages, see Article::showNamespaceHeader
+						!$output->getContext()->msg( 'talkpageheader' )->isDisabled() &&
+						// Check if it isn't empty since it may use parser functions to only show itself on some pages
+						trim( $output->getContext()->msg( 'talkpageheader' )->text() ) !== ''
+					)
+				) &&
+				// If there are comments in the lede section, we can't really separate them from other lede
+				// content, so keep the whole section visible.
+				!CommentFormatter::hasCommentsInLedeContent( $output->getHTML() )
+			) {
+				$output->addBodyClasses( 'ext-discussiontools-init-lede-hidden' );
+				$output->enableOOUI();
+				$output->prependHTML(
+					Html::rawElement( 'div',
+						[ 'class' => 'ext-discussiontools-init-lede-button-container' ],
+						( new ButtonWidget( [
+							'label' => $output->getContext()->msg( 'discussiontools-ledesection-button' )->text(),
+							'classes' => [ 'ext-discussiontools-init-lede-button' ],
+							'framed' => false,
+							'icon' => 'info',
+							'infusable' => true,
+						] ) )
+					)
+				);
+
+				// Preload jquery.makeCollapsible for LedeSectionDialog.
+				// Using the same approach as in Skin::getDefaultModules in MediaWiki core.
+				if ( strpos( $output->getHTML(), 'mw-collapsible' ) !== false ) {
+					$output->addModules( 'jquery.makeCollapsible' );
+					$output->addModuleStyles( 'jquery.makeCollapsible.styles' );
+				}
+			}
 		}
 	}
 
@@ -279,6 +280,17 @@ class PageHooks implements
 			);
 		}
 
+		if (
+			CommentFormatter::isEmptyTalkPage( $text ) &&
+			HookUtils::shouldDisplayEmptyState( $output->getContext() )
+		) {
+			$output->enableOOUI();
+			$text = CommentFormatter::appendToEmptyTalkPage(
+				$text, $this->getEmptyStateHtml( $output->getContext() )
+			);
+			$output->addBodyClasses( 'ext-discussiontools-emptystate-shown' );
+		}
+
 		if ( HookUtils::isFeatureEnabledForOutput( $output, HookUtils::VISUALENHANCEMENTS ) ) {
 			$output->enableOOUI();
 			if ( HookUtils::isFeatureEnabledForOutput( $output, HookUtils::TOPICSUBSCRIPTION ) ) {
@@ -313,6 +325,14 @@ class PageHooks implements
 			$text = CommentFormatter::postprocessVisualEnhancements(
 				$text, $lang, $output->getUser(), $isMobile
 			);
+
+			$subtitle = CommentFormatter::postprocessVisualEnhancementsSubtitle(
+				$text, $lang, $output->getUser()
+			);
+
+			if ( $subtitle ) {
+				$output->addSubtitle( $subtitle );
+			}
 		}
 
 		return true;
@@ -327,64 +347,7 @@ class PageHooks implements
 	 * @return void This hook must not abort, it must return no value
 	 */
 	public function onOutputPageParserOutput( $output, $pout ): void {
-		// ParserOutputPostCacheTransform hook would be a better place to do this,
+		CommentFormatter::postprocessTableOfContents( $pout, $output->getLanguage() );
-		// so that when the ParserOutput is used directly without using this hook,
-		// we don't leave half-baked interface elements in it (see e.g. T292345, T294168).
-		// But that hook doesn't provide parameters that we need to render correctly
-		// (including the page title, interface language, and current user).
-
-		// This hook can be executed more than once per page view if the page content is composed from
-		// multiple sources!
-
-		$isMobile = $this->isMobile();
-		$lang = $output->getLanguage();
-
-		CommentFormatter::postprocessTableOfContents( $pout, $lang );
-
-		if (
-			CommentFormatter::isEmptyTalkPage( $pout ) &&
-			HookUtils::shouldDisplayEmptyState( $output->getContext() )
-		) {
-			$output->enableOOUI();
-			CommentFormatter::appendToEmptyTalkPage(
-				$pout, $this->getEmptyStateHtml( $output->getContext() )
-			);
-			$output->addBodyClasses( 'ext-discussiontools-emptystate-shown' );
-		}
-
-		if ( HookUtils::isFeatureEnabledForOutput( $output, HookUtils::VISUALENHANCEMENTS ) ) {
-			$subtitle = CommentFormatter::postprocessVisualEnhancementsSubtitle(
-				$pout, $lang, $output->getUser()
-			);
-
-			if ( $subtitle ) {
-				$output->addSubtitle( $subtitle );
-			}
-		}
-
-		if ( $output->getSkin()->getSkinName() === 'minerva' ) {
-			$title = $output->getTitle();
-
-			if (
-				$title->isTalkPage() &&
-				HookUtils::isFeatureEnabledForOutput( $output, HookUtils::REPLYTOOL )
-			) {
-				if (
-					CommentFormatter::hasCommentsInLedeContent( $pout )
-				) {
-					// If there are comments in the lede section, we can't really separate them from other lede
-					// content, so keep the whole section visible.
-					$output->setProperty( 'DiscussionTools-ledeButton', false );
-
-				} elseif (
-					CommentFormatter::hasLedeContent( $pout ) &&
-					$output->getProperty( 'DiscussionTools-ledeButton' ) === null
-				) {
-					// If there is lede content and the lede button hasn't been disabled above, enable it.
-					$output->setProperty( 'DiscussionTools-ledeButton', true );
-				}
-			}
-		}
 	}
 
 	/**