Rename CommentFormatter::addReplyLinks

Bug: T280351
Change-Id: I0d7627d63407e11cca6091f78e4d440eec6efa91

includes/CommentFormatter.php

@@ -19,6 +19,8 @@ class CommentFormatter {
 		HookUtils::TOPICSUBSCRIPTION,
 	];
 
+	protected const MARKER_COMMENT = '<!-- DiscussionTools addDiscussionTools called -->';
+	// Compatibility with old cached content
 	protected const REPLY_LINKS_COMMENT = '<!-- DiscussionTools addReplyLinks called -->';
 
 	/**
@@ -34,33 +36,36 @@ class CommentFormatter {
 	}
 
 	/**
-	 * Add reply links to some HTML
+	 * Add discussion tools to some HTML
 	 *
 	 * @param string &$text Parser text output
 	 * @param Language $lang Interface language
 	 */
-	public static function addReplyLinks( string &$text, Language $lang ) : void {
+	public static function addDiscussionTools( string &$text, Language $lang ) : void {
 		$start = microtime( true );
 
-		// Never add links twice.
-		// This is required because we try again to add links to cached content
+		// Never add tools twice.
+		// This is required because we try again to add tools to cached content
 		// to support query string or cookie enabling
+		if ( strpos( $text, static::MARKER_COMMENT ) !== false ) {
+			return;
+		}
+		// Compatibility with old cached content
 		if ( strpos( $text, static::REPLY_LINKS_COMMENT ) !== false ) {
 			return;
 		}
 
-		$text = $text . "\n" . static::REPLY_LINKS_COMMENT;
+		$text = $text . "\n" . static::MARKER_COMMENT;
 
 		try {
-			// Add reply links and hidden data about comment ranges.
-			$newText = static::addReplyLinksInternal( $text, $lang );
+			$newText = static::addDiscussionToolsInternal( $text, $lang );
 		} catch ( Throwable $e ) {
 			// Catch errors, so that they don't cause the entire page to not display.
 			// Log it and add the request ID in a comment to make it easier to find in the logs.
 			MWExceptionHandler::logException( $e );
 
 			$requestId = htmlspecialchars( WebRequest::getRequestId() );
-			$info = "<!-- [$requestId] DiscussionTools could not add reply links on this page -->";
+			$info = "<!-- [$requestId] DiscussionTools could not process this page -->";
 			$text .= "\n" . $info;
 
 			return;
@@ -74,13 +79,13 @@ class CommentFormatter {
 	}
 
 	/**
-	 * Add reply links to some HTML
+	 * Add discussion tools to some HTML
 	 *
 	 * @param string $html HTML
 	 * @param Language $lang Interface language
-	 * @return string HTML with reply links
+	 * @return string HTML with discussion tools
 	 */
-	protected static function addReplyLinksInternal( string $html, Language $lang ) : string {
+	protected static function addDiscussionToolsInternal( string $html, Language $lang ) : string {
 		// The output of this method can end up in the HTTP cache (Varnish). Avoid changing it;
 		// and when doing so, ensure that frontend code can handle both the old and new outputs.
 		// See controller#init in JS.

includes/Hooks/PageHooks.php

@@ -106,13 +106,13 @@ class PageHooks implements
 	 */
 	public function onOutputPageBeforeHTML( $output, &$text ) {
 		$lang = $output->getLanguage();
-		// Check after the parser cache if reply links need to be added for
+		// Check after the parser cache if tools need to be added for
 		// non-cacheable reasons i.e. query string or cookie
-		// The addReplyLinks method is responsible for ensuring that
-		// reply links aren't added twice.
+		// The addDiscussionTools method is responsible for ensuring that
+		// tools aren't added twice.
 		foreach ( CommentFormatter::USE_WITH_FEATURES as $feature ) {
 			if ( HookUtils::isFeatureEnabledForOutput( $output, $feature ) ) {
-				CommentFormatter::addReplyLinks( $text, $lang );
+				CommentFormatter::addDiscussionTools( $text, $lang );
 				break;
 			}
 		}

includes/Hooks/ParserHooks.php

@@ -33,7 +33,7 @@ class ParserHooks implements
 		$popts = $parser->getOptions();
 		// ParserOption for dtreply was set in onArticleParserOptions
 		if ( $popts->getOption( 'dtreply' ) ) {
-			CommentFormatter::addReplyLinks( $text, $popts->getUserLangObj() );
+			CommentFormatter::addDiscussionTools( $text, $popts->getUserLangObj() );
 		}
 	}
 

modules/controller.js

@@ -310,7 +310,7 @@ function init( $container, state ) {
 
 	// The page can be served from the HTTP cache (Varnish), containing data-mw-comment generated
 	// by an older version of our PHP code. Code below must be able to handle that.
-	// See CommentFormatter::addReplyLinks() in PHP.
+	// See CommentFormatter::addDiscussionTools() in PHP.
 
 	// Iterate over commentNodes backwards so replies are always deserialized before their parents.
 	for ( i = commentNodes.length - 1; i >= 0; i-- ) {

tests/phpunit/CommentFormatterTest.php

@@ -11,10 +11,10 @@ use Wikimedia\TestingAccessWrapper;
 class CommentFormatterTest extends IntegrationTestCase {
 
 	/**
-	 * @dataProvider provideAddReplyLinksInternal
-	 * @covers ::addReplyLinksInternal
+	 * @dataProvider provideAddDiscussionToolsInternal
+	 * @covers ::addDiscussionToolsInternal
 	 */
-	public function testAddReplyLinksInternal(
+	public function testAddDiscussionToolsInternal(
 		string $name, string $dom, string $expected, string $config, string $data
 	) : void {
 		$dom = self::getHtml( $dom );
@@ -28,7 +28,7 @@ class CommentFormatterTest extends IntegrationTestCase {
 
 		$commentFormatter = TestingAccessWrapper::newFromClass( MockCommentFormatter::class );
 
-		$actual = $commentFormatter->addReplyLinksInternal( $dom, RequestContext::getMain()->getLanguage() );
+		$actual = $commentFormatter->addDiscussionToolsInternal( $dom, RequestContext::getMain()->getLanguage() );
 
 		// Optionally write updated content to the "reply HTML" files
 		if ( getenv( 'DISCUSSIONTOOLS_OVERWRITE_TESTS' ) ) {
@@ -38,7 +38,7 @@ class CommentFormatterTest extends IntegrationTestCase {
 		self::assertEquals( $expected, $actual, $name );
 	}
 
-	public function provideAddReplyLinksInternal() : array {
+	public function provideAddDiscussionToolsInternal() : array {
 		return self::getJson( '../cases/formattedreply.json' );
 	}