Repository: thephpleague/commonmark (php)

Issue:
Incorrect rendering of Embed after a list block
### Version(s) affected

2.3.3

### Description

An embeddable link used after a list gets rendered as a paragraph rather than getting embedded. In the code below, the first embed is rendered as a paragraph, the second one is rendered correctly as an iframe.

### How to reproduce

``` php
use League\CommonMark\Environment\Environment;
use League\CommonMark\Extension\CommonMark\CommonMarkCoreExtension;
use League\CommonMark\Extension\Embed\Bridge\OscaroteroEmbedAdapter;
use League\CommonMark\Extension\Embed\EmbedExtension;
use League\CommonMark\MarkdownConverter;

require 'vendor/autoload.php';

$config = [
    'embed' => [
        'adapter' => new OscaroteroEmbedAdapter(),
        'allowed_domains' => ['youtube.com'],
        'fallback' => 'link',
    ],
];

$environment = new Environment($config);
$environment->addExtension(new CommonMarkCoreExtension());
$environment->addExtension(new EmbedExtension());
$converter = new MarkdownConverter($environment);

$markdown = <<<MARKDOWN
# Embed Bug

- Hello
- World

https://www.youtube.com/watch?v=dQw4w9WgXcQ

https://www.youtube.com/watch?v=dQw4w9WgXcQ
MARKDOWN;

echo $converter->convert($markdown);
```

Pull request description:
Fix embed parsing
Closes #898 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 372495b8..58b2041d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,17 @@ Updates should follow the [Keep a CHANGELOG](https://keepachangelog.com/) princi
 
 ## [Unreleased][unreleased]
 
+### Changed
+
+- Made a number of small tweaks to the embed extension's parsing behavior to fix #898:
+    - Changed `EmbedStartParser` to always capture embed-like lines in container blocks, regardless of parent block type
+    - Changed `EmbedProcessor` to also remove `Embed` blocks that aren't direct children of the `Document`
+    - Increased the priority of `EmbedProcessor` to `1010`
+
+### Fixed
+
+- Fixed `EmbedExtension` not parsing embeds following a list block (#898)
+
 ## [2.3.3] - 2022-06-07
 
 ### Fixed
diff --git a/src/Extension/Embed/EmbedExtension.php b/src/Extension/Embed/EmbedExtension.php
index 616a8378..babf048d 100644
--- a/src/Extension/Embed/EmbedExtension.php
+++ b/src/Extension/Embed/EmbedExtension.php
@@ -42,7 +42,7 @@ final class EmbedExtension implements ConfigurableExtensionInterface
 
         $environment
             ->addBlockStartParser(new EmbedStartParser(), 300)
-            ->addEventListener(DocumentParsedEvent::class, new EmbedProcessor($adapter, $environment->getConfiguration()->get('embed.fallback')))
+            ->addEventListener(DocumentParsedEvent::class, new EmbedProcessor($adapter, $environment->getConfiguration()->get('embed.fallback')), 1010)
             ->addRenderer(Embed::class, new EmbedRenderer());
     }
 }
diff --git a/src/Extension/Embed/EmbedProcessor.php b/src/Extension/Embed/EmbedProcessor.php
index 5df099eb..68fb9eeb 100644
--- a/src/Extension/Embed/EmbedProcessor.php
+++ b/src/Extension/Embed/EmbedProcessor.php
@@ -16,6 +16,7 @@ namespace League\CommonMark\Extension\Embed;
 use League\CommonMark\Event\DocumentParsedEvent;
 use League\CommonMark\Extension\CommonMark\Node\Inline\Link;
 use League\CommonMark\Node\Block\Paragraph;
+use League\CommonMark\Node\Inline\Text;
 use League\CommonMark\Node\NodeIterator;
 
 final class EmbedProcessor
@@ -34,9 +35,18 @@ final class EmbedProcessor
 
     public function __invoke(DocumentParsedEvent $event): void
     {
-        $embeds = [];
-        foreach (new NodeIterator($event->getDocument()) as $node) {
-            if ($node instanceof Embed) {
+        $document = $event->getDocument();
+        $embeds   = [];
+        foreach (new NodeIterator($document) as $node) {
+            if (! ($node instanceof Embed)) {
+                continue;
+            }
+
+            if ($node->parent() !== $document) {
+                $replacement = new Paragraph();
+                $replacement->appendChild(new Text($node->getUrl()));
+                $node->replaceWith($replacement);
+            } else {
                 $embeds[] = $node;
             }
         }
diff --git a/src/Extension/Embed/EmbedStartParser.php b/src/Extension/Embed/EmbedStartParser.php
index 951e212d..5ff38086 100644
--- a/src/Extension/Embed/EmbedStartParser.php
+++ b/src/Extension/Embed/EmbedStartParser.php
@@ -13,7 +13,6 @@ declare(strict_types=1);
 
 namespace League\CommonMark\Extension\Embed;
 
-use League\CommonMark\Node\Block\Document;
 use League\CommonMark\Parser\Block\BlockStart;
 use League\CommonMark\Parser\Block\BlockStartParserInterface;
 use League\CommonMark\Parser\Cursor;
@@ -24,7 +23,7 @@ class EmbedStartParser implements BlockStartParserInterface
 {
     public function tryStart(Cursor $cursor, MarkdownParserStateInterface $parserState): ?BlockStart
     {
-        if ($cursor->isIndented() || $parserState->getParagraphContent() !== null || ! ($parserState->getLastMatchedBlockParser()->getBlock() instanceof Document)) {
+        if ($cursor->isIndented() || $parserState->getParagraphContent() !== null || ! ($parserState->getActiveBlockParser()->isContainer())) {
             return BlockStart::none();
         }
 


Test patch:
diff --git a/tests/functional/Extension/Embed/EmbedExtensionTest.php b/tests/functional/Extension/Embed/EmbedExtensionTest.php
index 96deff1b..218d5edf 100644
--- a/tests/functional/Extension/Embed/EmbedExtensionTest.php
+++ b/tests/functional/Extension/Embed/EmbedExtensionTest.php
@@ -15,6 +15,7 @@ namespace League\CommonMark\Tests\Functional\Extension\Embed;
 
 use League\CommonMark\ConverterInterface;
 use League\CommonMark\Environment\Environment;
+use League\CommonMark\Extension\Autolink\AutolinkExtension;
 use League\CommonMark\Extension\CommonMark\CommonMarkCoreExtension;
 use League\CommonMark\Extension\Embed\EmbedExtension;
 use League\CommonMark\MarkdownConverter;
@@ -30,10 +31,17 @@ final class EmbedExtensionTest extends AbstractLocalDataTest
     {
         $config['embed']['adapter'] = new FakeAdapter();
 
+        $enableAutolinkExtension = $config['enable_autolinks'] ?? false;
+        unset($config['enable_autolinks']);
+
         $environment = new Environment($config);
         $environment->addExtension(new CommonMarkCoreExtension());
         $environment->addExtension(new EmbedExtension());
 
+        if ($enableAutolinkExtension) {
+            $environment->addExtension(new AutolinkExtension());
+        }
+
         return new MarkdownConverter($environment);
     }
 
diff --git a/tests/functional/Extension/Embed/data/after_list_regression.html b/tests/functional/Extension/Embed/data/after_list_regression.html
new file mode 100644
index 00000000..f38362c5
--- /dev/null
+++ b/tests/functional/Extension/Embed/data/after_list_regression.html
@@ -0,0 +1,7 @@
+<h1>Embed Bug</h1>
+<ul>
+<li>Both links below</li>
+<li>Should be embeds</li>
+</ul>
+<iframe class="embed" src="https://www.youtube.com/watch?v=dQw4w9WgXcQ"></iframe>
+<iframe class="embed" src="https://www.youtube.com/watch?v=dQw4w9WgXcQ"></iframe>
diff --git a/tests/functional/Extension/Embed/data/after_list_regression.md b/tests/functional/Extension/Embed/data/after_list_regression.md
new file mode 100644
index 00000000..891fc1c3
--- /dev/null
+++ b/tests/functional/Extension/Embed/data/after_list_regression.md
@@ -0,0 +1,8 @@
+# Embed Bug
+
+- Both links below
+- Should be embeds
+
+https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+https://www.youtube.com/watch?v=dQw4w9WgXcQ
diff --git a/tests/functional/Extension/Embed/data/embeds-with-autolinks-enabled.html b/tests/functional/Extension/Embed/data/embeds-with-autolinks-enabled.html
new file mode 100644
index 00000000..aece4b4d
--- /dev/null
+++ b/tests/functional/Extension/Embed/data/embeds-with-autolinks-enabled.html
@@ -0,0 +1,27 @@
+<p>This is an embed:</p>
+<iframe class="embed" src="https://www.youtube.com/watch?v=dQw4w9WgXcQ"></iframe>
+<p>So is this, with only three spaces of indentation:</p>
+<iframe class="embed" src="https://www.youtube.com/watch?v=dQw4w9WgXcQ"></iframe>
+<p>This is not, because it's fully indented:</p>
+<pre><code>https://www.youtube.com/watch?v=dQw4w9WgXcQ
+</code></pre>
+<p>This is not, because it has extra bits after the URL:</p>
+<p><a href="https://www.youtube.com/watch?v=dQw4w9WgXcQ">https://www.youtube.com/watch?v=dQw4w9WgXcQ</a> &lt;-- you gotta watch this!</p>
+<p>This is not, because it's in a fenced code block:</p>
+<pre><code class="language-md">
+https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+</code></pre>
+<p>And this isn't either because it's inline: <img src="https://www.youtube.com/watch?v=dQw4w9WgXcQ" alt="" /></p>
+<p>Embeds can't be nested in other blocks:</p>
+<ul>
+<li>https://www.youtube.com/watch?v=dQw4w9WgXcQ
+<ul>
+<li>https://www.youtube.com/watch?v=dQw4w9WgXcQ</li>
+</ul>
+</li>
+</ul>
+<p>This isn't valid because it's a lazy paragraph continuation:
+<a href="https://www.youtube.com/watch?v=dQw4w9WgXcQ">https://www.youtube.com/watch?v=dQw4w9WgXcQ</a></p>
+<iframe class="embed" src="https://www.youtube.com/watch?v=dQw4w9WgXcQ"></iframe>
+<p>^ This is fine, though</p>
diff --git a/tests/functional/Extension/Embed/data/embeds-with-autolinks-enabled.md b/tests/functional/Extension/Embed/data/embeds-with-autolinks-enabled.md
new file mode 100644
index 00000000..c3c2eea5
--- /dev/null
+++ b/tests/functional/Extension/Embed/data/embeds-with-autolinks-enabled.md
@@ -0,0 +1,40 @@
+---
+enable_autolinks: true
+---
+
+This is an embed:
+
+https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+So is this, with only three spaces of indentation:
+
+   https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+This is not, because it's fully indented:
+
+    https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+This is not, because it has extra bits after the URL:
+
+https://www.youtube.com/watch?v=dQw4w9WgXcQ <-- you gotta watch this!
+
+This is not, because it's in a fenced code block:
+
+```md
+
+https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+```
+
+And this isn't either because it's inline: ![](https://www.youtube.com/watch?v=dQw4w9WgXcQ)
+
+Embeds can't be nested in other blocks:
+
+- https://www.youtube.com/watch?v=dQw4w9WgXcQ
+    - https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+This isn't valid because it's a lazy paragraph continuation:
+https://www.youtube.com/watch?v=dQw4w9WgXcQ
+
+https://www.youtube.com/watch?v=dQw4w9WgXcQ
+^ This is fine, though