Don't allow the root node to be treated like a comment frame

Also fix a bug where headings would be ignored while checking for
comment frames. See task for detailed explanation.

Bug: T303396
Change-Id: I6495826b4b050ea80680e0798ac6ab4497a7c09e

extension.json

@@ -322,7 +322,8 @@
 			"cases/sr-el/sr-el.json",
 			"cases/dt-tags-oldparser/dt-tags-oldparser.json",
 			"cases/dt-tags-parsoid/dt-tags-parsoid.json",
-			"cases/no-heading/no-heading.json",
+			"cases/single-comment-heading/single-comment-heading.json",
+			"cases/single-comment-no-heading/single-comment-no-heading.json",
 			"cases/lrm-signature/lrm-signature.json",
 			"cases/selflink-signature/selflink-signature.json",
 			"cases/reflist-oldparser/reflist-oldparser.json",
@@ -370,7 +371,10 @@
 			"cases/dt-tags-oldparser/dt-tags-oldparser-modified.html",
 			"cases/dt-tags-parsoid/dt-tags-parsoid.html",
 			"cases/dt-tags-parsoid/dt-tags-parsoid-modified.html",
-			"cases/no-heading/no-heading.html",
+			"cases/single-comment-heading/single-comment-heading.html",
+			"cases/single-comment-heading/single-comment-heading-modified.html",
+			"cases/single-comment-no-heading/single-comment-no-heading.html",
+			"cases/single-comment-no-heading/single-comment-no-heading-modified.html",
 			"cases/lrm-signature/lrm-signature.html",
 			"cases/selflink-signature/selflink-signature.html",
 			"cases/reflist-oldparser/reflist-oldparser.html",

includes/CommentModifier.php

@@ -129,7 +129,7 @@ class CommentModifier {
 		// If the comment is fully covered by some wrapper element, insert replies outside that wrapper.
 		// This will often just be a paragraph node (<p>), but it can be a <div> or <table> that serves
 		// as some kind of a fancy frame, which are often used for barnstars and announcements.
-		$covered = CommentUtils::getFullyCoveredSiblings( $curComment );
+		$covered = CommentUtils::getFullyCoveredSiblings( $curComment, $curComment->getRootNode() );
 		if ( $curComment->getLevel() === 1 && $covered ) {
 			$target = end( $covered );
 			$parent = $target->parentNode;

includes/CommentUtils.php

@@ -82,7 +82,6 @@ class CommentUtils {
 	public static function isOurGeneratedNode( Node $node ): bool {
 		return $node instanceof Element && (
 			DOMCompat::getClassList( $node )->contains( 'ext-discussiontools-init-replylink-buttons' ) ||
-			$node->hasAttribute( 'data-mw-comment' ) ||
 			$node->hasAttribute( 'data-mw-comment-start' ) ||
 			$node->hasAttribute( 'data-mw-comment-end' )
 		);
@@ -392,9 +391,12 @@ class CommentUtils {
 	 * Get the nodes (if any) that contain the given thread item, and nothing else.
 	 *
 	 * @param ThreadItem $item
+	 * @param ?Node $excludedAncestorNode Node that shouldn't be included in the result, even if it
+	 *     contains the item and nothing else. This is intended to avoid traversing outside of a node
+	 *     which is a container for all the thread items.
 	 * @return Node[]|null
 	 */
-	public static function getFullyCoveredSiblings( ThreadItem $item ): ?array {
+	public static function getFullyCoveredSiblings( ThreadItem $item, ?Node $excludedAncestorNode = null ): ?array {
 		$siblings = self::getCoveredSiblings( $item->getRange() );
 
 		$makeRange = static function ( $siblings ) {
@@ -412,6 +414,7 @@ class CommentUtils {
 			// If these are all of the children (or the only child), go up one more level
 			while (
 				( $parent = $siblings[ 0 ]->parentNode ) &&
+				$parent !== $excludedAncestorNode &&
 				self::compareRanges( $makeRange( [ $parent ] ), $item->getRange() ) === 'equal'
 			) {
 				$siblings = [ $parent ];

modules/modifier.js

@@ -95,7 +95,7 @@ function addListItem( comment, replyIndentation ) {
 	// If the comment is fully covered by some wrapper element, insert replies outside that wrapper.
 	// This will often just be a paragraph node (<p>), but it can be a <div> or <table> that serves
 	// as some kind of a fancy frame, which are often used for barnstars and announcements.
-	var covered = utils.getFullyCoveredSiblings( curComment );
+	var covered = utils.getFullyCoveredSiblings( curComment, curComment.rootNode );
 	if ( curComment.level === 1 && covered ) {
 		target = covered[ covered.length - 1 ];
 		parent = target.parentNode;

modules/utils.js

@@ -53,7 +53,6 @@ function isRenderingTransparentNode( node ) {
 function isOurGeneratedNode( node ) {
 	return node.nodeType === Node.ELEMENT_NODE && (
 		node.classList.contains( 'ext-discussiontools-init-replylink-buttons' ) ||
-		node.hasAttribute( 'data-mw-comment' ) ||
 		node.hasAttribute( 'data-mw-comment-start' ) ||
 		node.hasAttribute( 'data-mw-comment-end' )
 	);
@@ -343,9 +342,12 @@ function getCoveredSiblings( range ) {
  * Get the nodes (if any) that contain the given thread item, and nothing else.
  *
  * @param {ThreadItem} item Thread item
+ * @param {Node} [excludedAncestorNode] Node that shouldn't be included in the result, even if it
+ *     contains the item and nothing else. This is intended to avoid traversing outside of a node
+ *     which is a container for all the thread items.
  * @return {Node[]|null}
  */
-function getFullyCoveredSiblings( item ) {
+function getFullyCoveredSiblings( item, excludedAncestorNode ) {
 	var siblings = getCoveredSiblings( item.getNativeRange() );
 
 	function makeRange( sibs ) {
@@ -362,6 +364,7 @@ function getFullyCoveredSiblings( item ) {
 		var parent;
 		while (
 			( parent = siblings[ 0 ].parentNode ) &&
+			parent !== excludedAncestorNode &&
 			compareRanges( makeRange( [ parent ] ), item.getNativeRange() ) === 'equal'
 		) {
 			siblings = [ parent ];

tests/cases/comments.json

@@ -144,10 +144,18 @@
 		"data": "../data/enwiki-data.json"
 	},
 	{
-		"name": "No heading",
-		"title": "Talk:No heading",
-		"dom": "cases/no-heading/no-heading.html",
-		"expected": "../cases/no-heading/no-heading.json",
+		"name": "Single comment, heading",
+		"title": "Talk:Single comment",
+		"dom": "cases/single-comment-no-heading/single-comment-no-heading.html",
+		"expected": "../cases/single-comment-no-heading/single-comment-no-heading.json",
+		"config": "../data/enwiki-config.json",
+		"data": "../data/enwiki-data.json"
+	},
+	{
+		"name": "Single comment with heading",
+		"title": "Talk:Single comment",
+		"dom": "cases/single-comment-heading/single-comment-heading.html",
+		"expected": "../cases/single-comment-heading/single-comment-heading.json",
 		"config": "../data/enwiki-config.json",
 		"data": "../data/enwiki-data.json"
 	},

tests/cases/modified.json

@@ -81,6 +81,22 @@
 		"config": "../data/enwiki-config.json",
 		"data": "../data/enwiki-data.json"
 	},
+	{
+		"name": "Single comment, heading",
+		"title": "Talk:Single comment",
+		"dom": "cases/single-comment-no-heading/single-comment-no-heading.html",
+		"expected": "cases/single-comment-no-heading/single-comment-no-heading-modified.html",
+		"config": "../data/enwiki-config.json",
+		"data": "../data/enwiki-data.json"
+	},
+	{
+		"name": "Single comment with heading",
+		"title": "Talk:Single comment",
+		"dom": "cases/single-comment-heading/single-comment-heading.html",
+		"expected": "cases/single-comment-heading/single-comment-heading-modified.html",
+		"config": "../data/enwiki-config.json",
+		"data": "../data/enwiki-data.json"
+	},
 	{
 		"name": "Must split a list to reply to one of the comments",
 		"title": "Talk:Split list",

tests/cases/single-comment-heading/single-comment-heading-modified.html

@@ -0,0 +1,5 @@
+<div class="mw-parser-output">
+<h2><span class="mw-headline" id="hello">hello</span></h2>
+<p>aaa <b><a href="/wiki/User:Matma_Rex" title="User:Matma Rex">Matma Rex</a> | <a href="/wiki/User_talk:Matma_Rex" title="User talk:Matma Rex">talk</a></b> 23:19, 22 January 2020 (UTC)
+</p><dl><dd>Reply to c-Matma_Rex-2020-01-22T23:19:00.000Z-hello</dd></dl>
+</div>

tests/cases/single-comment-heading/single-comment-heading.html

@@ -0,0 +1,5 @@
+<div class="mw-parser-output">
+<h2><span class="mw-headline" id="hello">hello</span></h2>
+<p>aaa <b><a href="/wiki/User:Matma_Rex" title="User:Matma Rex">Matma Rex</a> | <a href="/wiki/User_talk:Matma_Rex" title="User talk:Matma Rex">talk</a></b> 23:19, 22 January 2020 (UTC)
+</p>
+</div>

tests/cases/single-comment-heading/single-comment-heading.json

@@ -0,0 +1,37 @@
+[
+	{
+		"placeholderHeading": false,
+		"type": "heading",
+		"range": [
+			"1/0/0",
+			"1/0/1"
+		],
+		"headingLevel": 2,
+		"level": 0,
+		"name": "h-Matma_Rex-2020-01-22T23:19:00.000Z",
+		"id": "h-hello-2020-01-22T23:19:00.000Z",
+		"warnings": [],
+		"replies": [
+			{
+				"type": "comment",
+				"timestamp": "2020-01-22T23:19:00.000Z",
+				"author": "Matma Rex",
+				"range": [
+					"3/0",
+					"3/2/29"
+				],
+				"signatureRanges": [
+					[
+						"3/1",
+						"3/2/29"
+					]
+				],
+				"level": 1,
+				"name": "c-Matma_Rex-2020-01-22T23:19:00.000Z",
+				"id": "c-Matma_Rex-2020-01-22T23:19:00.000Z-hello",
+				"warnings": [],
+				"replies": []
+			}
+		]
+	}
+]

tests/cases/single-comment-no-heading/single-comment-no-heading-modified.html

@@ -0,0 +1,4 @@
+<div class="mw-parser-output">
+<p>aaa <b><a href="/wiki/User:Matma_Rex" title="User:Matma Rex">Matma Rex</a> | <a href="/wiki/User_talk:Matma_Rex" title="User talk:Matma Rex">talk</a></b> 23:19, 22 January 2020 (UTC)
+</p><dl><dd>Reply to c-Matma_Rex-2020-01-22T23:19:00.000Z</dd></dl>
+</div>