From 420c514091919142c4e74a03f61de37bc3d07454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Tue, 28 Apr 2020 21:20:23 +0200 Subject: [PATCH] Insert replies outside of decorative comment frames When there is a wrapper element whose range matches the range of a comment, any replies will now be added outside of that wrapper, instead of directly after the comment (inside the wrapper). Bug: T250126 Change-Id: I6b42c4db019ae998e91eebd324f9cbd2aa791b4f --- includes/CommentModifier.php | 78 ++++++++++++++++++ modules/modifier.js | 82 ++++++++++++++++++- .../en-big-oldparser-modified.html | 8 +- .../en-big-parsoid-modified.html | 8 +- tests/cases/wrappers/wrappers-modified.html | 30 +++++-- tests/cases/wrappers/wrappers.html | 30 +++++-- 6 files changed, 215 insertions(+), 21 deletions(-) diff --git a/includes/CommentModifier.php b/includes/CommentModifier.php index dac829617..ddbb27b5b 100644 --- a/includes/CommentModifier.php +++ b/includes/CommentModifier.php @@ -82,6 +82,74 @@ 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. @@ -134,7 +202,17 @@ class CommentModifier { if ( $curLevel < $desiredLevel ) { // Insert more lists after the target to increase nesting. + // 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 ) ) ) { + $target = $wrapper; + $parent = $target->parentNode; + } + // If we can't insert a list directly inside this element, insert after it. + // The wrapper check above handles most cases, but this special case is still needed for comments + // consisting of multiple paragraphs with no fancy frames. // TODO Improve this check if ( strtolower( $parent->tagName ) === 'p' || strtolower( $parent->tagName ) === 'pre' ) { $parent = $parent->parentNode; diff --git a/modules/modifier.js b/modules/modifier.js index 8e7cc8922..30a03be26 100644 --- a/modules/modifier.js +++ b/modules/modifier.js @@ -44,6 +44,76 @@ 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. @@ -57,7 +127,7 @@ function addReplyLink( comment, linkNode ) { function addListItem( comment ) { var curComment, curLevel, desiredLevel, - target, parent, listType, itemType, list, item, newNode, + target, parent, wrapper, listType, itemType, list, item, newNode, listTypeMap = { li: 'ul', dd: 'dl' @@ -97,7 +167,17 @@ function addListItem( comment ) { if ( curLevel < desiredLevel ) { // Insert more lists after the target to increase nesting. + // 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 ) ) ) { + target = wrapper; + parent = target.parentNode; + } + // If we can't insert a list directly inside this element, insert after it. + // The wrapper check above handles most cases, but this special case is still needed for comments + // consisting of multiple paragraphs with no fancy frames. // TODO Improve this check if ( parent.tagName.toLowerCase() === 'p' || parent.tagName.toLowerCase() === 'pre' ) { parent = parent.parentNode; diff --git a/tests/cases/en-big-oldparser/en-big-oldparser-modified.html b/tests/cases/en-big-oldparser/en-big-oldparser-modified.html index 512f58de1..0c3dc2b72 100644 --- a/tests/cases/en-big-oldparser/en-big-oldparser-modified.html +++ b/tests/cases/en-big-oldparser/en-big-oldparser-modified.html @@ -658,7 +658,7 @@ Evolutionary history of life · ‎Timeline of the evolutionary ... · ‎Earlie

There is some interesting chatter on a related topic at phab:T214998. --Izno (talk) 15:42, 6 July 2019 (UTC)

Reply to Izno|2019-07-06T15:42:00.000Z|0

Page preview discrepancy for an ITN item

-
Resolved: Thanks for pointing out the post above; closing this due to derpiness –Deacon Vorbis (carbon • videos) 21:29, 6 July 2019 (UTC)
Reply to Deacon Vorbis|2019-07-06T21:29:00.000Z|0
+
Resolved: Thanks for pointing out the post above; closing this due to derpiness –Deacon Vorbis (carbon • videos) 21:29, 6 July 2019 (UTC)
Reply to Deacon Vorbis|2019-07-06T21:29:00.000Z|0

The page preview for final item (as I'm writing this) on the "In the News" list on the main page (the Antananarivo stampede) mentions it occurred on 26 July, despite the article (correctly) saying it was 26 June. Looking at mw:Page Previews, it just says it uses a portion of the opening paragraph, so I have no idea what's causing the discrepancy. Does anyone know why this is happening and/or how to fix this? Thanks, –Deacon Vorbis (carbon • videos) 20:50, 6 July 2019 (UTC)

User:Deacon Vorbis, see #Stuck tooltips above. Over at Phabricator, it has an "Unbreak Now!" status, which means "drop everything you're doing and fix this". Nyttend (talk) 21:21, 6 July 2019 (UTC)
Reply to Nyttend|2019-07-06T21:21:00.000Z|0
Reply to Deacon Vorbis|2019-07-06T20:50:00.000Z|0
@@ -1684,7 +1684,7 @@ Sorry should have said, monobook, Edge, Win10. Catholic Church sidebar}}, not {{Sidebar with collapsible lists}}. It was this edit by Royalistandlegitimist that broke the template. I advise Royalistandlegitimist to take advantage of the sandbox and testcases before making changes to the template. Nardog (talk) 09:11, 1 August 2019 (UTC)
Thanks, Nardog, for the fix! (Apologies to Paine; not only weren't they the cause of the problem, but that edit was (nearly exactly!) a year ago. I was too tired and in too much of a hurry to see clearly.) Mathglot (talk) 17:41, 1 August 2019 (UTC)
Reply to Mathglot|2019-08-01T17:41:00.000Z|0
Reply to Nardog|2019-08-01T09:11:00.000Z|0
Reply to Mathglot|2019-08-01T09:03:00.000Z|0

The notification button

-
Resolved: Fixed in the last update of MediaWiki. Nardog (talk) 05:45, 2 August 2019 (UTC)
Reply to Nardog|2019-08-02T05:45:00.000Z|0
+
Resolved: Fixed in the last update of MediaWiki. Nardog (talk) 05:45, 2 August 2019 (UTC)
Reply to Nardog|2019-08-02T05:45:00.000Z|0
Tracked in Phabricator
Task T228744

Hi, so I am using my mobile.

@@ -2410,8 +2410,8 @@ above now done, but this page will work and others not. -- GreenC 19:00, 15 August 2019 (UTC)
Reply to GreenC|2019-08-15T19:00:00.000Z|0
Reply to Golbez|2019-08-15T18:58:00.000Z|0
Reply to 213.133.84.227|2019-08-15T18:45:00.000Z|0

Enable chess PGN viewer for chess articles

-
Editors are generally in support of an interactive PGN viewer. Discussion on the specific implementation can be found at Wikipedia:Interface administrators' noticeboard#Chess viewer Wug·a·po·des​ 22:44, 27 August 2019 (UTC)
Reply to Wugapodes|2019-08-27T22:44:00.000Z|0
-
+
Editors are generally in support of an interactive PGN viewer. Discussion on the specific implementation can be found at Wikipedia:Interface administrators' noticeboard#Chess viewer Wug·a·po·des​ 22:44, 27 August 2019 (UTC)
+
Reply to Wugapodes|2019-08-27T22:44:00.000Z|0
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.

Should the interactive portable game notation viewer in use on the Hebrew, Russian, and Ukrainian Wikipedias be enabled on the English Wikipedia? 05:28, 30 July 2019 (UTC) diff --git a/tests/cases/en-big-parsoid/en-big-parsoid-modified.html b/tests/cases/en-big-parsoid/en-big-parsoid-modified.html index 22f9f9d6c..0e533b0b8 100644 --- a/tests/cases/en-big-parsoid/en-big-parsoid-modified.html +++ b/tests/cases/en-big-parsoid/en-big-parsoid-modified.html @@ -469,7 +469,7 @@ Evolutionary history of life · ‎Timeline of the evolutionary ... · ‎Earlie

Page preview discrepancy for an ITN item

-
Resolved: Thanks for pointing out the post above; closing this due to derpiness –Deacon Vorbis (carbon • videos) 21:29, 6 July 2019 (UTC)
Reply to Deacon Vorbis|2019-07-06T21:29:00.000Z|0
+
Resolved: Thanks for pointing out the post above; closing this due to derpiness –Deacon Vorbis (carbon • videos) 21:29, 6 July 2019 (UTC)
Reply to Deacon Vorbis|2019-07-06T21:29:00.000Z|0

The page preview for final item (as I'm writing this) on the "In the News" list on the main page (the Antananarivo stampede) mentions it occurred on 26 July, despite the article (correctly) saying it was 26 June. Looking at mw:Page Previews, it just says it uses a portion of the opening paragraph, so I have no idea what's causing the discrepancy. Does anyone know why this is happening and/or how to fix this? Thanks, –Deacon Vorbis (carbon • videos) 20:50, 6 July 2019 (UTC)

User:Deacon Vorbis, see #Stuck tooltips above. Over at Phabricator, it has an "Unbreak Now!" status, which means "drop everything you're doing and fix this". Nyttend (talk) 21:21, 6 July 2019 (UTC)
Reply to Nyttend|2019-07-06T21:21:00.000Z|0
Reply to Deacon Vorbis|2019-07-06T20:50:00.000Z|0
@@ -1568,7 +1568,7 @@ Sorry should have said, monobook, Edge, Win10.
Thanks, Nardog, for the fix! (Apologies to Paine; not only weren't they the cause of the problem, but that edit was (nearly exactly!) a year ago. I was too tired and in too much of a hurry to see clearly.) Mathglot (talk) 17:41, 1 August 2019 (UTC)
Reply to Mathglot|2019-08-01T17:41:00.000Z|0
Reply to Nardog|2019-08-01T09:11:00.000Z|0
Reply to Mathglot|2019-08-01T09:03:00.000Z|0

The notification button

-
Resolved: Fixed in the last update of MediaWiki. Nardog (talk) 05:45, 2 August 2019 (UTC)
Reply to Nardog|2019-08-02T05:45:00.000Z|0
+
Resolved: Fixed in the last update of MediaWiki. Nardog (talk) 05:45, 2 August 2019 (UTC)
Reply to Nardog|2019-08-02T05:45:00.000Z|0

Hi, so I am using my mobile.

@@ -2339,9 +2339,9 @@ above now done, but
Editors are generally in support of an interactive PGN viewer. Discussion on the specific implementation can be found at Wikipedia:Interface administrators' noticeboard#Chess viewer Wug·a·po·des​ 22:44, 27 August 2019 (UTC)
Reply to Wugapodes|2019-08-27T22:44:00.000Z|0
+ ">Editors are generally in support of an interactive PGN viewer. Discussion on the specific implementation can be found at Wikipedia:Interface administrators' noticeboard#Chess viewer Wug·a·po·des​ 22:44, 27 August 2019 (UTC) - +
Reply to Wugapodes|2019-08-27T22:44:00.000Z|0
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.

Should the interactive portable game notation viewer in use on the Hebrew, Russian, and Ukrainian Wikipedias be enabled on the English Wikipedia? 05:28, 30 July 2019 (UTC)

diff --git a/tests/cases/wrappers/wrappers-modified.html b/tests/cases/wrappers/wrappers-modified.html index 697100a7e..08d0115d1 100644 --- a/tests/cases/wrappers/wrappers-modified.html +++ b/tests/cases/wrappers/wrappers-modified.html @@ -1,11 +1,29 @@ -

paragraph (outside)

+

paragraph

blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)

Reply to Matma Rex|2020-01-22T23:19:00.000Z|0
-

preformatted (outside)

+

preformatted

blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
Reply to Matma Rex|2020-01-22T23:19:00.000Z|1
-

div (inside)

-
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
Reply to Matma Rex|2020-01-22T23:19:00.000Z|2
+

div with one comment

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
Reply to Matma Rex|2020-01-22T23:19:00.000Z|2
-

table (inside)

-
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
Reply to Matma Rex|2020-01-22T23:19:00.000Z|3
+

table with one comment

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
Reply to Matma Rex|2020-01-22T23:19:00.000Z|3
+ +

div with multiple comments

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
Reply to Matma Rex|2020-01-22T23:19:00.000Z|5
Reply to Matma Rex|2020-01-22T23:19:00.000Z|4
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
Reply to Matma Rex|2020-01-22T23:19:00.000Z|6
+ +

table with multiple comments

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
Reply to Matma Rex|2020-01-22T23:19:00.000Z|8
Reply to Matma Rex|2020-01-22T23:19:00.000Z|7
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
Reply to Matma Rex|2020-01-22T23:19:00.000Z|9
diff --git a/tests/cases/wrappers/wrappers.html b/tests/cases/wrappers/wrappers.html index f67c199b7..d400bfa6a 100644 --- a/tests/cases/wrappers/wrappers.html +++ b/tests/cases/wrappers/wrappers.html @@ -1,11 +1,29 @@ -

paragraph (outside)

+

paragraph

blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)

-

preformatted (outside)

+

preformatted

blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
-

div (inside)

-
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
+

div with one comment

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
-

table (inside)

-
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
+

table with one comment

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
+ +

div with multiple comments

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
+ +

table with multiple comments

+
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +
blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC)
+blah blah Matma Rex | talk 23:19, 22 January 2020 (UTC) +