From fb08abe062e900dcae47afb2abefe2e285578290 Mon Sep 17 00:00:00 2001 From: DLynch Date: Sun, 12 Feb 2023 02:57:31 +0000 Subject: [PATCH] Revert "Use setExtensionData() instead of marker comments where possible (2/3)" This reverts commit 0ac420ecbc87f6e835b5d08d7a6a32761e558176. Reason for revert: this was supposed to be merged later; revert it now and reapply in a bit Change-Id: I33fb07856152c2401b3a071c143f27f1e9753287 --- includes/CommentFormatter.php | 97 +++++++++++----------- includes/Hooks/PageHooks.php | 151 +++++++++++++--------------------- 2 files changed, 108 insertions(+), 140 deletions(-) diff --git a/includes/CommentFormatter.php b/includes/CommentFormatter.php index 9dd9adaba..f49fcda2c 100644 --- a/includes/CommentFormatter.php +++ b/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' ); - if ( $itemData && $itemData['timestamp'] && $itemData['id'] ) { - $relativeTime = static::getSignatureRelativeTime( - new MWTimestamp( $itemData['timestamp'] ), - $lang, - $user - ); - $commentLink = Html::element( 'a', [ - 'href' => '#' . Sanitizer::escapeIdForLink( $itemData['id'] ) - ], $relativeTime ); + preg_match( '//', $text, $matches ); + // The subtitle is inserted into the correct place by the caller, so cleanup the comment here + $text = preg_replace( '//', '', $text ); + if ( count( $matches ) ) { + $itemData = json_decode( htmlspecialchars_decode( $matches[1] ), true ); - if ( isset( $itemData['heading'] ) ) { - $headingLink = Html::element( 'a', [ - 'href' => '#' . Sanitizer::escapeIdForLink( $itemData['heading']['linkableTitle'] ) - ], $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(); + if ( $itemData && $itemData['timestamp'] && $itemData['id'] ) { + $relativeTime = static::getSignatureRelativeTime( + new MWTimestamp( $itemData['timestamp'] ), + $lang, + $user + ); + $commentLink = Html::element( 'a', [ + 'href' => '#' . Sanitizer::escapeIdForLink( $itemData['id'] ) + ], $relativeTime ); + + if ( isset( $itemData['heading'] ) ) { + $headingLink = Html::element( 'a', [ + 'href' => '#' . Sanitizer::escapeIdForLink( $itemData['heading']['linkableTitle'] ) + ], $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 { - return $pout->getExtensionData( 'DiscussionTools-isEmptyTalkPage' ) === true; + public static function isEmptyTalkPage( string $text ): bool { + return strpos( $text, '' ) !== 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 { - $text = $pout->getRawText(); - $text .= $content; - $pout->setText( $text ); + public static function appendToEmptyTalkPage( string $text, string $content ): string { + return str_replace( '', $content, $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 { - return $pout->getExtensionData( 'DiscussionTools-hasLedeContent' ) === true; + public static function hasLedeContent( string $text ): bool { + return strpos( $text, '' ) !== 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 { - return $pout->getExtensionData( 'DiscussionTools-hasCommentsInLedeContent' ) === true; + public static function hasCommentsInLedeContent( string $text ): bool { + return strpos( $text, '' ) !== false; } public static function isLanguageRequiringReplyIcon( Language $lang ): bool { diff --git a/includes/Hooks/PageHooks.php b/includes/Hooks/PageHooks.php index f58e20593..b78df6f86 100644 --- a/includes/Hooks/PageHooks.php +++ b/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, - // 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 ); - } - } - } + CommentFormatter::postprocessTableOfContents( $pout, $output->getLanguage() ); } /**