diff --git a/includes/CommentModifier.php b/includes/CommentModifier.php index ddbb27b5b..2f48adeb0 100644 --- a/includes/CommentModifier.php +++ b/includes/CommentModifier.php @@ -82,74 +82,6 @@ class CommentModifier { $target->parentNode->insertBefore( $linkNode, $target->nextSibling ); } - /** - * Get a node (if any) that contains the given comment, and nothing else. - * - * @param stdClass $comment Comment data returned by parser#groupThreads - * @return DOMElement|null - */ - private static function getFullyCoveredWrapper( $comment ) { - $ancestor = $comment->range->commonAncestorContainer; - - $isIgnored = function ( $node ) { - // Ignore empty text nodes - return $node->nodeType === XML_TEXT_NODE && CommentUtils::htmlTrim( $node->nodeValue ) === ''; - }; - - $firstNonemptyChild = function ( $node ) use ( $isIgnored ) { - $node = $node->firstChild; - while ( $node && $isIgnored( $node ) ) { - $node = $node->nextSibling; - } - return $node; - }; - - $lastNonemptyChild = function ( $node ) use ( $isIgnored ) { - $node = $node->lastChild; - while ( $node && $isIgnored( $node ) ) { - $node = $node->previousSibling; - } - return $node; - }; - - $startMatches = false; - $node = $ancestor; - while ( $node ) { - if ( $comment->range->startContainer === $node && $comment->range->startOffset === 0 ) { - $startMatches = true; - break; - } - $node = $firstNonemptyChild( $node ); - } - - $endMatches = false; - $node = $ancestor; - while ( $node ) { - $length = ( $node->nodeType === XML_TEXT_NODE ) ? - strlen( rtrim( $node->nodeValue, "\t\n\f\r " ) ) : - // PHP bug: childNodes can be null for comment nodes - // (it should always be a DOMNodeList, even if the node can't have children) - ( $node->childNodes ? $node->childNodes->length : 0 ); - if ( $comment->range->endContainer === $node && $comment->range->endOffset === $length ) { - $endMatches = true; - break; - } - $node = $lastNonemptyChild( $node ); - } - - if ( $startMatches && $endMatches ) { - // If this is the only child, go up one more level - while ( - $ancestor->parentNode && - $firstNonemptyChild( $ancestor->parentNode ) === $lastNonemptyChild( $ancestor->parentNode ) - ) { - $ancestor = $ancestor->parentNode; - } - return $ancestor; - } - return null; - } - /** * Given a comment, add a list item to its document's DOM tree, inside of which a reply to said * comment can be added. @@ -205,7 +137,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 (

), but it can be a

or that serves // as some kind of a fancy frame, which are often used for barnstars and announcements. - if ( $curLevel === 1 && ( $wrapper = self::getFullyCoveredWrapper( $curComment ) ) ) { + if ( $curLevel === 1 && ( $wrapper = CommentUtils::getFullyCoveredWrapper( $curComment ) ) ) { $target = $wrapper; $parent = $target->parentNode; } diff --git a/includes/CommentParser.php b/includes/CommentParser.php index 4fee193af..af8ecf3e3 100644 --- a/includes/CommentParser.php +++ b/includes/CommentParser.php @@ -977,11 +977,12 @@ class CommentParser { * we can't determine the source. */ public function getTranscludedFrom( stdClass $comment ) { - // If some template is used within the comment (e.g. {{ping|…}} or {{tl|…}}), that *does not* mean - // the comment is transcluded. We only want to consider comments to be transcluded if the wrapper - // element (usually
  • or

    ) is marked as part of a transclusion. - // TODO: This seems to work fine but I'm having a hard time explaining why it is correct... - $node = $comment->range->endContainer; + // If some template is used within the comment (e.g. {{ping|…}} or {{tl|…}}, or a + // non-substituted signature template), that *does not* mean the comment is transcluded. + // We only want to consider comments to be transcluded if the wrapper element (usually + //

  • or

    ) is marked as part of a transclusion. If we can't find a wrapper, using + // endContainer should avoid false negatives (although may have false positives). + $node = CommentUtils::getFullyCoveredWrapper( $comment ) ?: $comment->range->endContainer; // Find the node containing information about the transclusion: // 1. Find the closest ancestor with an 'about' attribute diff --git a/includes/CommentUtils.php b/includes/CommentUtils.php index 023493654..f1bc7b554 100644 --- a/includes/CommentUtils.php +++ b/includes/CommentUtils.php @@ -71,6 +71,74 @@ class CommentUtils { return trim( $str, "\t\n\f\r " ); } + /** + * Get a node (if any) that contains the given comment, and nothing else. + * + * @param stdClass $comment Comment data returned by parser#groupThreads + * @return DOMElement|null + */ + public static function getFullyCoveredWrapper( $comment ) { + $ancestor = $comment->range->commonAncestorContainer; + + $isIgnored = function ( $node ) { + // Ignore empty text nodes + return $node->nodeType === XML_TEXT_NODE && CommentUtils::htmlTrim( $node->nodeValue ) === ''; + }; + + $firstNonemptyChild = function ( $node ) use ( $isIgnored ) { + $node = $node->firstChild; + while ( $node && $isIgnored( $node ) ) { + $node = $node->nextSibling; + } + return $node; + }; + + $lastNonemptyChild = function ( $node ) use ( $isIgnored ) { + $node = $node->lastChild; + while ( $node && $isIgnored( $node ) ) { + $node = $node->previousSibling; + } + return $node; + }; + + $startMatches = false; + $node = $ancestor; + while ( $node ) { + if ( $comment->range->startContainer === $node && $comment->range->startOffset === 0 ) { + $startMatches = true; + break; + } + $node = $firstNonemptyChild( $node ); + } + + $endMatches = false; + $node = $ancestor; + while ( $node ) { + $length = ( $node->nodeType === XML_TEXT_NODE ) ? + strlen( rtrim( $node->nodeValue, "\t\n\f\r " ) ) : + // PHP bug: childNodes can be null for comment nodes + // (it should always be a DOMNodeList, even if the node can't have children) + ( $node->childNodes ? $node->childNodes->length : 0 ); + if ( $comment->range->endContainer === $node && $comment->range->endOffset === $length ) { + $endMatches = true; + break; + } + $node = $lastNonemptyChild( $node ); + } + + if ( $startMatches && $endMatches ) { + // If this is the only child, go up one more level + while ( + $ancestor->parentNode && + $firstNonemptyChild( $ancestor->parentNode ) === $lastNonemptyChild( $ancestor->parentNode ) + ) { + $ancestor = $ancestor->parentNode; + } + return $ancestor; + } + return null; + } + /** * Unwrap Parsoid sections * diff --git a/modules/modifier.js b/modules/modifier.js index 30a03be26..e3dca3fac 100644 --- a/modules/modifier.js +++ b/modules/modifier.js @@ -44,76 +44,6 @@ function addReplyLink( comment, linkNode ) { target.parentNode.insertBefore( linkNode, target.nextSibling ); } -/** - * Get a node (if any) that contains the given comment, and nothing else. - * - * @param {Object} comment Comment data returned by parser#groupThreads - * @return {HTMLElement|null} - */ -function getFullyCoveredWrapper( comment ) { - var nativeRange, ancestor, node, startMatches, endMatches, length; - - nativeRange = utils.getNativeRange( comment ); - ancestor = nativeRange.commonAncestorContainer; - - function isIgnored( node ) { - // Ignore empty text nodes, and our own reply buttons - return ( node.nodeType === Node.TEXT_NODE && utils.htmlTrim( node.textContent ) === '' ) || - ( node.className && node.className.indexOf( 'dt-init-replylink-buttons' ) !== -1 ); - } - - function firstNonemptyChild( node ) { - node = node.firstChild; - while ( node && isIgnored( node ) ) { - node = node.nextSibling; - } - return node; - } - - function lastNonemptyChild( node ) { - node = node.lastChild; - while ( node && isIgnored( node ) ) { - node = node.previousSibling; - } - return node; - } - - startMatches = false; - node = ancestor; - while ( node ) { - if ( comment.range.startContainer === node && comment.range.startOffset === 0 ) { - startMatches = true; - break; - } - node = firstNonemptyChild( node ); - } - - endMatches = false; - node = ancestor; - while ( node ) { - length = node.nodeType === Node.TEXT_NODE ? - node.textContent.replace( /[\t\n\f\r ]+$/, '' ).length : - node.childNodes.length; - if ( comment.range.endContainer === node && comment.range.endOffset === length ) { - endMatches = true; - break; - } - node = lastNonemptyChild( node ); - } - - if ( startMatches && endMatches ) { - // If this is the only child, go up one more level - while ( - ancestor.parentNode && - firstNonemptyChild( ancestor.parentNode ) === lastNonemptyChild( ancestor.parentNode ) - ) { - ancestor = ancestor.parentNode; - } - return ancestor; - } - return null; -} - /** * Given a comment, add a list item to its document's DOM tree, inside of which a reply to said * comment can be added. @@ -170,7 +100,7 @@ function addListItem( comment ) { // If the comment is fully covered by some wrapper element, insert replies outside that wrapper. // This will often just be a paragraph node (

    ), but it can be a

    or
  • that serves // as some kind of a fancy frame, which are often used for barnstars and announcements. - if ( curLevel === 1 && ( wrapper = getFullyCoveredWrapper( curComment ) ) ) { + if ( curLevel === 1 && ( wrapper = utils.getFullyCoveredWrapper( curComment ) ) ) { target = wrapper; parent = target.parentNode; } diff --git a/modules/parser.js b/modules/parser.js index 42b1131cd..468e4d976 100644 --- a/modules/parser.js +++ b/modules/parser.js @@ -975,11 +975,12 @@ function getAuthors( comment ) { function getTranscludedFrom( comment ) { var node, about, dataMw; - // If some template is used within the comment (e.g. {{ping|…}} or {{tl|…}}), that *does not* mean - // the comment is transcluded. We only want to consider comments to be transcluded if the wrapper - // element (usually
  • or

    ) is marked as part of a transclusion. - // TODO: This seems to work fine but I'm having a hard time explaining why it is correct... - node = comment.range.endContainer; + // If some template is used within the comment (e.g. {{ping|…}} or {{tl|…}}, or a + // non-substituted signature template), that *does not* mean the comment is transcluded. + // We only want to consider comments to be transcluded if the wrapper element (usually + //

  • or

    ) is marked as part of a transclusion. If we can't find a wrapper, using + // endContainer should avoid false negatives (although may have false positives). + node = utils.getFullyCoveredWrapper( comment ) || comment.range.endContainer; // Find the node containing information about the transclusion: // 1. Find the closest ancestor with an 'about' attribute diff --git a/modules/utils.js b/modules/utils.js index 7b457bf9a..5822db925 100644 --- a/modules/utils.js +++ b/modules/utils.js @@ -69,9 +69,80 @@ function htmlTrim( str ) { return str.replace( /^[\t\n\f\r ]+/, '' ).replace( /[\t\n\f\r ]+$/, '' ); } +/** + * Get a node (if any) that contains the given comment, and nothing else. + * + * @param {Object} comment Comment data returned by parser#groupThreads + * @return {HTMLElement|null} + */ +function getFullyCoveredWrapper( comment ) { + var nativeRange, ancestor, node, startMatches, endMatches, length; + + nativeRange = getNativeRange( comment ); + ancestor = nativeRange.commonAncestorContainer; + + function isIgnored( node ) { + // Ignore empty text nodes, and our own reply buttons + return ( node.nodeType === Node.TEXT_NODE && htmlTrim( node.textContent ) === '' ) || + ( node.className && node.className.indexOf( 'dt-init-replylink-buttons' ) !== -1 ); + } + + function firstNonemptyChild( node ) { + node = node.firstChild; + while ( node && isIgnored( node ) ) { + node = node.nextSibling; + } + return node; + } + + function lastNonemptyChild( node ) { + node = node.lastChild; + while ( node && isIgnored( node ) ) { + node = node.previousSibling; + } + return node; + } + + startMatches = false; + node = ancestor; + while ( node ) { + if ( comment.range.startContainer === node && comment.range.startOffset === 0 ) { + startMatches = true; + break; + } + node = firstNonemptyChild( node ); + } + + endMatches = false; + node = ancestor; + while ( node ) { + length = node.nodeType === Node.TEXT_NODE ? + node.textContent.replace( /[\t\n\f\r ]+$/, '' ).length : + node.childNodes.length; + if ( comment.range.endContainer === node && comment.range.endOffset === length ) { + endMatches = true; + break; + } + node = lastNonemptyChild( node ); + } + + if ( startMatches && endMatches ) { + // If this is the only child, go up one more level + while ( + ancestor.parentNode && + firstNonemptyChild( ancestor.parentNode ) === lastNonemptyChild( ancestor.parentNode ) + ) { + ancestor = ancestor.parentNode; + } + return ancestor; + } + return null; +} + module.exports = { getNativeRange: getNativeRange, childIndexOf: childIndexOf, closestElement: closestElement, + getFullyCoveredWrapper: getFullyCoveredWrapper, htmlTrim: htmlTrim };