Trim signatures when added in an empty existing node, too

Add unit tests for appendSignature().

Bug: T276612
Change-Id: Ic44c52f4d54492e092f9396c626380e2637b6f0f

extension.json

@@ -243,6 +243,7 @@
 			"cases/unwrap.json",
 			"cases/isWikitextSigned.json",
 			"cases/isHtmlSigned.json",
+			"cases/appendSignature.json",
 			"cases/linearWalk.json",
 			"cases/sanitize-wikitext-linebreaks.json",
 			"cases/timestamp-regex.json",

includes/CommentModifier.php

@@ -474,7 +474,10 @@ class CommentModifier {
 		// add another paragraph to contain the signature.
 		if ( strtolower( $container->lastChild->nodeName ) !== 'p' ) {
 			$container->appendChild( $doc->createElement( 'p' ) );
-			// Trim the signature to prevent leading whitespace triggering preformatted text (T269188)
+		}
+		// If the last node is empty, trim the signature to prevent leading whitespace triggering
+		// preformatted text (T269188, T276612)
+		if ( !$container->lastChild->firstChild ) {
 			$signature = trim( $signature );
 		}
 		// Sign the last line

modules/modifier.js

@@ -429,7 +429,10 @@ function appendSignature( container ) {
 	// add another paragraph to contain the signature.
 	if ( container.lastChild.nodeName.toLowerCase() !== 'p' ) {
 		container.appendChild( doc.createElement( 'p' ) );
-		// Trim the signature to prevent leading whitespace triggering preformatted text (T269188)
+	}
+	// If the last node is empty, trim the signature to prevent leading whitespace triggering
+	// preformatted text (T269188, T276612)
+	if ( !container.lastChild.firstChild ) {
 		signature = signature.trim();
 	}
 	// Sign the last line
@@ -535,5 +538,6 @@ module.exports = {
 	addHtmlReply: addHtmlReply,
 	isWikitextSigned: isWikitextSigned,
 	isHtmlSigned: isHtmlSigned,
+	appendSignature: appendSignature,
 	sanitizeWikitextLinebreaks: sanitizeWikitextLinebreaks
 };

tests/cases/appendSignature.json

@@ -0,0 +1,17 @@
+[
+	{
+		"msg": "Simple message",
+		"html": "<p>Foo bar</p>",
+		"expected": "<p>Foo bar<span typeof=\"mw:Transclusion\" data-mw=\"{&quot;parts&quot;:[&quot; ~~~~&quot;]}\"></span></p>"
+	},
+	{
+		"msg": "List",
+		"html": "<p>Foo bar</p><ul><li>A</li><li>B</li></ul>",
+		"expected": "<p>Foo bar</p><ul><li>A</li><li>B</li></ul><p><span typeof=\"mw:Transclusion\" data-mw=\"{&quot;parts&quot;:[&quot;~~~~&quot;]}\"></span></p>"
+	},
+	{
+		"msg": "Empty trailing paragraph",
+		"html": "<p>Foo bar</p><p></p>",
+		"expected": "<p>Foo bar</p><p><span typeof=\"mw:Transclusion\" data-mw=\"{&quot;parts&quot;:[&quot;~~~~&quot;]}\"></span></p>"
+	}
+]

tests/phpunit/CommentModifierTest.php

@@ -4,6 +4,7 @@ namespace MediaWiki\Extension\DiscussionTools\Tests;
 
 use MediaWiki\Extension\DiscussionTools\CommentModifier;
 use Wikimedia\Parsoid\Utils\DOMCompat;
+use Wikimedia\Parsoid\Wt2Html\XMLSerializer;
 
 /**
  * @coversDefaultClass \MediaWiki\Extension\DiscussionTools\CommentModifier
@@ -160,6 +161,30 @@ class CommentModifierTest extends IntegrationTestCase {
 		return self::getJson( '../cases/isHtmlSigned.json' );
 	}
 
+	/**
+	 * @dataProvider provideAppendSignature
+	 * @covers ::appendSignature
+	 */
+	public function testAppendSignature(
+		string $msg, string $html, string $expected
+	) : void {
+		$doc = self::createDocument( '' );
+		$container = $doc->createElement( 'div' );
+		DOMCompat::setInnerHTML( $container, $html );
+
+		CommentModifier::appendSignature( $container );
+
+		self::assertEquals(
+			$expected,
+			XMLSerializer::serialize( $container, [ 'innerXML' => true, 'smartQuote' => false ] )['html'],
+			$msg
+		);
+	}
+
+	public function provideAppendSignature() : array {
+		return self::getJson( '../cases/appendSignature.json' );
+	}
+
 	/**
 	 * @dataProvider provideSanitizeWikitextLinebreaks
 	 * @covers ::sanitizeWikitextLinebreaks

tests/qunit/modifier.test.js

@@ -152,6 +152,23 @@ QUnit.test( 'isHtmlSigned', function ( assert ) {
 	} );
 } );
 
+QUnit.test( 'appendSignature', function ( assert ) {
+	var cases = require( '../cases/appendSignature.json' );
+
+	cases.forEach( function ( caseItem ) {
+		var container = document.createElement( 'div' );
+		container.innerHTML = caseItem.html;
+
+		modifier.appendSignature( container );
+
+		assert.strictEqual(
+			container.innerHTML,
+			caseItem.expected,
+			caseItem.msg
+		);
+	} );
+} );
+
 QUnit.test( 'sanitizeWikitextLinebreaks', function ( assert ) {
 	var cases = require( '../cases/sanitize-wikitext-linebreaks.json' );