Incorrect rendering of Embed after a list block

[SeriousKen]

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

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);

[SeriousKen]

It seems that the way List blocks are implemented the list stays open while the parser looks for list items, the logic in the Embed block's tryStart fails because it checks if it is in a Document block, which it isn't (the List block is still open) so it will get identified as a Paragraph block instead.

[colinodell]

Great catch! You're right, that List block is open during the tryStart() method and only gets closed afterward, when we try to add the newly-parsed child to an open block. This means we can't accurately determine what block the Embed will get added to during the tryStart() method.

Furthermore, the decision of whether a parent block should contain a child block is intended to be decided by the parent, not the child. This complicates things because unlike every other block, Embed is only intended to work at the root level of the Document.

We could check whether the parent is either a Document (which we want) or a ListBlock (which we know won't accept the Embed as a child), but relying on assumed behavior of ListBlock feels like too much of a hack.

Instead, I think it would be better to allow the Embed to be parsed regardless of the parent container and then leverage our EmbedProcessor to replace it with a Paragraph if the parent block isn't the Document. This would avoid any hacks since processors always have an accurate picture of which block is the parent.

[colinodell]

Fix released in version 2.3.4!