Rendered list items should only add newlines around block-level children

CHANGELOG.md

@@ -6,6 +6,10 @@ Updates should follow the [Keep a CHANGELOG](https://keepachangelog.com/) princi
 
 ## [Unreleased][unreleased]
 
+### Fixed
+
+- Rendered list items should only add newlines around block-level children (#1059, #1061)
+
 ## [2.6.0] - 2024-12-07
 
 This is a **security release** to address potential denial of service attacks when parsing specially crafted,

src/Extension/CommonMark/Renderer/Block/ListItemRenderer.php

@@ -17,8 +17,9 @@
 namespace League\CommonMark\Extension\CommonMark\Renderer\Block;
 
 use League\CommonMark\Extension\CommonMark\Node\Block\ListItem;
-use League\CommonMark\Extension\TaskList\TaskListItemMarker;
+use League\CommonMark\Node\Block\AbstractBlock;
 use League\CommonMark\Node\Block\Paragraph;
+use League\CommonMark\Node\Block\TightBlockInterface;
 use League\CommonMark\Node\Node;
 use League\CommonMark\Renderer\ChildNodeRendererInterface;
 use League\CommonMark\Renderer\NodeRendererInterface;
@@ -39,11 +40,14 @@ public function render(Node $node, ChildNodeRendererInterface $childRenderer): \
         ListItem::assertInstanceOf($node);
 
         $contents = $childRenderer->renderNodes($node->children());
-        if (\substr($contents, 0, 1) === '<' && ! $this->startsTaskListItem($node)) {
+
+        $inTightList = ($parent = $node->parent()) && $parent instanceof TightBlockInterface && $parent->isTight();
+
+        if ($this->needsBlockSeparator($node->firstChild(), $inTightList)) {
             $contents = "\n" . $contents;
         }
 
-        if (\substr($contents, -1, 1) === '>') {
+        if ($this->needsBlockSeparator($node->lastChild(), $inTightList)) {
             $contents .= "\n";
         }
 
@@ -65,10 +69,12 @@ public function getXmlAttributes(Node $node): array
         return [];
     }
 
-    private function startsTaskListItem(ListItem $block): bool
+    private function needsBlockSeparator(?Node $child, bool $inTightList): bool
     {
-        $firstChild = $block->firstChild();
+        if ($child instanceof Paragraph && $inTightList) {
+            return false;
+        }
 
-        return $firstChild instanceof Paragraph && $firstChild->firstChild() instanceof TaskListItemMarker;
+        return $child instanceof AbstractBlock;
     }
 }

tests/functional/CMarkRegressionTest.php

@@ -27,12 +27,6 @@ public static function dataProvider(): \Generator
     {
         $tests = SpecReader::readFile(__DIR__ . '/../../vendor/commonmark/cmark/test/regression.txt');
         foreach ($tests as $example) {
-            // We can't currently render spec example 13 exactly how the upstream library does. We'll likely need to overhaul
-            // our rendering approach in order to fix that, so we'll use this temporary workaround for now.
-            if ($example['number'] === 13) {
-                $example['output'] = \str_replace('</script></li>', "</script>\n</li>", $example['output']);
-            }
-
             // The case-fold test from example 21 fails on PHP 8.0.* and below due to the behavior of mb_convert_case().
             // See https://3v4l.org/7TeXJ.
             if (\PHP_VERSION_ID < 81000 && $example['number'] === 21) {

tests/functional/CommonMarkJSRegressionTest.php

@@ -25,17 +25,6 @@ final class CommonMarkJSRegressionTest extends AbstractSpecTestCase
 {
     public static function dataProvider(): \Generator
     {
-        $tests = SpecReader::readFile(__DIR__ . '/../../vendor/commonmark/commonmark.js/test/regression.txt');
-        foreach ($tests as $example) {
-            // We can't currently render spec examples 18 or 24 exactly how the upstream library does. We'll likely need to overhaul
-            // our rendering approach in order to fix that, so we'll use this temporary workaround for now.
-            if ($example['number'] === 18) {
-                $example['output'] = \str_replace('</script></li>', "</script>\n</li>", $example['output']);
-            } elseif ($example['number'] === 24) {
-                $example['output'] = \str_replace("<pre>The following line is part of HTML block.\n\n</li>", "<pre>The following line is part of HTML block.\n</li>", $example['output']);
-            }
-
-            yield $example;
-        }
+        yield from SpecReader::readFile(__DIR__ . '/../../vendor/commonmark/commonmark.js/test/regression.txt');
     }
 }

tests/functional/Extension/Autolink/UrlAutolinkParserTest.php

@@ -49,7 +49,7 @@ public static function dataProviderForAutolinkTests(): iterable
         yield ['www.google.com', '<p><a href="http://www.google.com">www.google.com</a></p>'];
         yield [' http://leadingwhitespace.example.com', '<p><a href="http://leadingwhitespace.example.com">http://leadingwhitespace.example.com</a></p>'];
         yield ['http://trailingwhitespace.example.com ', '<p><a href="http://trailingwhitespace.example.com">http://trailingwhitespace.example.com</a></p>'];
-        yield ['- https://example.com/list-item', "<ul>\n<li>\n<a href=\"https://example.com/list-item\">https://example.com/list-item</a>\n</li>\n</ul>"];
+        yield ['- https://example.com/list-item', "<ul>\n<li><a href=\"https://example.com/list-item\">https://example.com/list-item</a></li>\n</ul>"];
 
         // Tests of "incomplete" URLs
         yield ['google.com is missing www and/or a protocol', '<p>google.com is missing www and/or a protocol</p>'];
@@ -60,7 +60,7 @@ public static function dataProviderForAutolinkTests(): iterable
         yield ['Maybe you\'re interested in https://www.google.com/search?q=php+commonmark!', '<p>Maybe you\'re interested in <a href="https://www.google.com/search?q=php+commonmark">https://www.google.com/search?q=php+commonmark</a>!</p>'];
         yield ['Or perhaps you\'re looking for my personal website https://www.colinodell.com...?', '<p>Or perhaps you\'re looking for my personal website <a href="https://www.colinodell.com">https://www.colinodell.com</a>...?</p>'];
         yield ['Check https://www.stackoverflow.com: they have all the answers', '<p>Check <a href="https://www.stackoverflow.com">https://www.stackoverflow.com</a>: they have all the answers</p>'];
-        yield ['- https://example.com/list-item-with-trailing-colon:', "<ul>\n<li>\n<a href=\"https://example.com/list-item-with-trailing-colon\">https://example.com/list-item-with-trailing-colon</a>:</li>\n</ul>"];
+        yield ['- https://example.com/list-item-with-trailing-colon:', "<ul>\n<li><a href=\"https://example.com/list-item-with-trailing-colon\">https://example.com/list-item-with-trailing-colon</a>:</li>\n</ul>"];
         yield ['Visit www.commonmark.org.', '<p>Visit <a href="http://www.commonmark.org">www.commonmark.org</a>.</p>'];
         yield ['Visit www.commonmark.org/a.b.', '<p>Visit <a href="http://www.commonmark.org/a.b">www.commonmark.org/a.b</a>.</p>'];
 

tests/functional/Extension/Footnote/spec.txt

@@ -106,12 +106,9 @@ A [footnote identifier] can come immediately after a word, or have a space betwe
 [^5]: Not allowed
 .
 <ul>
-<li>No spaces<sup id="fnref:1"><a class="footnote-ref" href="#fn:1" role="doc-noteref">1</a></sup>
-</li>
-<li>One space <sup id="fnref:2"><a class="footnote-ref" href="#fn:2" role="doc-noteref">2</a></sup>
-</li>
-<li>Exclamation mark with space! <sup id="fnref:3"><a class="footnote-ref" href="#fn:3" role="doc-noteref">3</a></sup>
-</li>
+<li>No spaces<sup id="fnref:1"><a class="footnote-ref" href="#fn:1" role="doc-noteref">1</a></sup></li>
+<li>One space <sup id="fnref:2"><a class="footnote-ref" href="#fn:2" role="doc-noteref">2</a></sup></li>
+<li>Exclamation mark with space! <sup id="fnref:3"><a class="footnote-ref" href="#fn:3" role="doc-noteref">3</a></sup></li>
 <li>Exclamation mark with no space![^4]</li>
 <li>Another exclamation mark![^5](not allowed)</li>
 </ul>

tests/functional/Extension/TableOfContents/md/custom-class.html

@@ -1,10 +1,7 @@
 <ul class="markdown-toc">
-<li>
-<a href="#content-hello">Hello</a>
+<li><a href="#content-hello">Hello</a>
 <ul>
-<li>
-<a href="#content-world">World</a>
-</li>
+<li><a href="#content-world">World</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/headings-with-inlines.html

@@ -1,13 +1,7 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-this-heading-has-a-link">This heading has a link</a>
-</li>
-<li>
-<a href="#content-20-reasons-why-marquee-tags-should-make-a-comeback">20 Reasons Why &lt;marquee&gt; Tags Should Make A Comeback</a>
-</li>
-<li>
-<a href="#content-i-love-using--characters-in-markdown">I love using * characters in Markdown!</a>
-</li>
+<li><a href="#content-this-heading-has-a-link">This heading has a link</a></li>
+<li><a href="#content-20-reasons-why-marquee-tags-should-make-a-comeback">20 Reasons Why &lt;marquee&gt; Tags Should Make A Comeback</a></li>
+<li><a href="#content-i-love-using--characters-in-markdown">I love using * characters in Markdown!</a></li>
 </ul>
 <p>My Awesome Blog Post</p>
 <h2><a id="content-this-heading-has-a-link" href="#content-this-heading-has-a-link" class="heading-permalink" aria-hidden="true" title="Permalink">¶</a><a href="https://example.com">This heading</a> has a link</h2>

tests/functional/Extension/TableOfContents/md/html-in-headings.html

@@ -1,7 +1,5 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-hello-world">Hello World! </a>
-</li>
+<li><a href="#content-hello-world">Hello World! </a></li>
 </ul>
 <p>Here's a sample Markdown document</p>
 <h1><a id="content-hello-world" href="#content-hello-world" class="heading-permalink" aria-hidden="true" title="Permalink">¶</a>Hello World! <i class="icon icon-waving-hand"></i></h1>

tests/functional/Extension/TableOfContents/md/min-max.html

@@ -1,16 +1,11 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-level-2">Level 2</a>
+<li><a href="#content-level-2">Level 2</a>
 <ul>
-<li>
-<a href="#content-level-3">Level 3</a>
+<li><a href="#content-level-3">Level 3</a>
 <ul>
-<li>
-<a href="#content-level-4">Level 4</a>
+<li><a href="#content-level-4">Level 4</a>
 <ul>
-<li>
-<a href="#content-level-5">Level 5</a>
-</li>
+<li><a href="#content-level-5">Level 5</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/position-before-headings.html

@@ -1,11 +1,8 @@
 <p>This is my document.</p>
 <ul class="table-of-contents">
-<li>
-<a href="#content-hello-world">Hello World!</a>
+<li><a href="#content-hello-world">Hello World!</a>
 <ul>
-<li>
-<a href="#content-isnt-markdown-great">Isn't Markdown Great?</a>
-</li>
+<li><a href="#content-isnt-markdown-great">Isn't Markdown Great?</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/position-placeholder-multiple.html

@@ -1,28 +1,18 @@
 <p>This is my document.</p>
 <ul class="table-of-contents">
-<li>
-<a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
+<li><a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
 <ul>
-<li>
-<a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a>
-</li>
-<li>
-<a href="#content-just-a-link-reference-down-here">Just a link reference down here</a>
-</li>
+<li><a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a></li>
+<li><a href="#content-just-a-link-reference-down-here">Just a link reference down here</a></li>
 </ul>
 </li>
 </ul>
 <h1><a id="content-another-copy-of-my-toc-is-here" href="#content-another-copy-of-my-toc-is-here" class="heading-permalink" aria-hidden="true" title="Permalink">¶</a>Another copy of my TOC is here</h1>
 <ul class="table-of-contents">
-<li>
-<a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
+<li><a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
 <ul>
-<li>
-<a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a>
-</li>
-<li>
-<a href="#content-just-a-link-reference-down-here">Just a link reference down here</a>
-</li>
+<li><a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a></li>
+<li><a href="#content-just-a-link-reference-down-here">Just a link reference down here</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/position-top.html

@@ -1,10 +1,7 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-hello-world">Hello World!</a>
+<li><a href="#content-hello-world">Hello World!</a>
 <ul>
-<li>
-<a href="#content-isnt-markdown-great">Isn't Markdown Great?</a>
-</li>
+<li><a href="#content-isnt-markdown-great">Isn't Markdown Great?</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/sample.html

@@ -1,22 +1,15 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-section-1">Section 1</a>
+<li><a href="#content-section-1">Section 1</a>
 <ul>
-<li>
-<a href="#content-section-11">Section 1.1</a>
+<li><a href="#content-section-11">Section 1.1</a>
 <ul>
-<li>
-<a href="#content-section-111">Section 1.1.1</a>
+<li><a href="#content-section-111">Section 1.1.1</a>
 <ul>
-<li>
-<a href="#content-section-1111">Section 1.1.1.1</a>
+<li><a href="#content-section-1111">Section 1.1.1.1</a>
 <ul>
-<li>
-<a href="#content-section-11111">Section 1.1.1.1.1</a>
+<li><a href="#content-section-11111">Section 1.1.1.1.1</a>
 <ul>
-<li>
-<a href="#content-section-111111">Section 1.1.1.1.1.1</a>
-</li>
+<li><a href="#content-section-111111">Section 1.1.1.1.1.1</a></li>
 </ul>
 </li>
 </ul>
@@ -27,39 +20,22 @@
 </li>
 </ul>
 </li>
-<li>
-<a href="#content-section-2">Section 2</a>
+<li><a href="#content-section-2">Section 2</a>
 <ul>
-<li>
-<a href="#content-section-21">Section 2.1</a>
-</li>
-<li>
-<a href="#content-section-22">Section 2.2</a>
+<li><a href="#content-section-21">Section 2.1</a></li>
+<li><a href="#content-section-22">Section 2.2</a>
 <ul>
-<li>
-<a href="#content-section-221">Section 2.2.1</a>
-</li>
-<li>
-<a href="#content-section-222">Section 2.2.2</a>
+<li><a href="#content-section-221">Section 2.2.1</a></li>
+<li><a href="#content-section-222">Section 2.2.2</a>
 <ul>
-<li>
-<a href="#content-section-2221">Section 2.2.2.1</a>
-</li>
-<li>
-<a href="#content-section-2222">Section 2.2.2.2</a>
+<li><a href="#content-section-2221">Section 2.2.2.1</a></li>
+<li><a href="#content-section-2222">Section 2.2.2.2</a>
 <ul>
-<li>
-<a href="#content-section-22221">Section 2.2.2.2.1</a>
-</li>
-<li>
-<a href="#content-section-22222">Section 2.2.2.2.2</a>
+<li><a href="#content-section-22221">Section 2.2.2.2.1</a></li>
+<li><a href="#content-section-22222">Section 2.2.2.2.2</a>
 <ul>
-<li>
-<a href="#content-section-222221">Section 2.2.2.2.2.1</a>
-</li>
-<li>
-<a href="#content-section-222222">Section 2.2.2.2.2.2</a>
-</li>
+<li><a href="#content-section-222221">Section 2.2.2.2.2.1</a></li>
+<li><a href="#content-section-222222">Section 2.2.2.2.2.2</a></li>
 </ul>
 </li>
 </ul>
@@ -70,9 +46,7 @@
 </li>
 </ul>
 </li>
-<li>
-<a href="#content-section-title-with-quotes">Section title with &quot;quotes&quot;</a>
-</li>
+<li><a href="#content-section-title-with-quotes">Section title with &quot;quotes&quot;</a></li>
 </ul>
 <p>Here's a sample Markdown document with lots of headings</p>
 <h1><a id="content-section-1" href="#content-section-1" class="heading-permalink" aria-hidden="true" title="Permalink">¶</a>Section 1</h1>

tests/functional/Extension/TableOfContents/md/setext-headings.html

@@ -1,10 +1,7 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-this-is-a-top-level">This is a top level</a>
+<li><a href="#content-this-is-a-top-level">This is a top level</a>
 <ul>
-<li>
-<a href="#content-this-is-another-level">This is another level</a>
-</li>
+<li><a href="#content-this-is-another-level">This is another level</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/style-bullet.html

@@ -1,29 +1,18 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-these">These</a>
+<li><a href="#content-these">These</a>
 <ul>
-<li>
-<a href="#content-should">Should</a>
+<li><a href="#content-should">Should</a>
 <ul>
-<li>
-<a href="#content-be">Be</a>
-</li>
+<li><a href="#content-be">Be</a></li>
 </ul>
 </li>
-<li>
-<a href="#content-in">In</a>
-</li>
+<li><a href="#content-in">In</a></li>
 </ul>
 </li>
-<li>
-<a href="#content-an">An</a>
-</li>
-<li>
-<a href="#content-unordered">Unordered</a>
+<li><a href="#content-an">An</a></li>
+<li><a href="#content-unordered">Unordered</a>
 <ul>
-<li>
-<a href="#content-list">List</a>
-</li>
+<li><a href="#content-list">List</a></li>
 </ul>
 </li>
 </ul>