WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.49k stars 4.19k forks source link

Embed block fails when converted to reusable #14608

Closed nicopujol closed 2 years ago

nicopujol commented 5 years ago

Describe the bug I added an embed URL at the end of the post here and it works fine as an embed block. However, as soon as I turn it into a reusable block to add on other posts, the output displays only the text URL and loses all rich content. I tested it several times back and forth. Functionality returns when reverting to standard block.

To Reproduce Steps to reproduce the behavior:

  1. Working as a standard block https://www.nicolaspujol.com/fast-mimicking-diet-fmd-recipes/
  2. Failing as a reusable block https://www.nicolaspujol.com/endive-garbanzo-bean-salad-with-oil-free-chimichurri-sauce/

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

youknowriad commented 5 years ago

I'm pretty sure there's an issue about it but I can't find it for some reason.

cc @notnownikki

notnownikki commented 5 years ago

This was a filter priority problem, oembed was running before the reusable block was rendered. We had fixed this previously, but I guess there wasn't a test... sounds like it needs an e2e test! I'll pick this up.

notnownikki commented 5 years ago

Ok folks, this is a core issue now, as all the code for registering dynamic blocks and rendering them seems to have been moved out of the plugin and into core.

There was an issue opened in the core trac a couple of weeks ago https://core.trac.wordpress.org/ticket/46457

youknowriad commented 5 years ago

Thanks for looking @notnownikki let's close in favor of this trac ticket.

notnownikki commented 5 years ago

I'm going to reopen this because although the problem is in core, the tests for reusable blocks are only in the plugin code, so we'll need to cover the tests here. I'll write an e2e test to cover it and track things in this issue too.

notnownikki commented 5 years ago

I've opened a PR with a test to cover this at https://github.com/WordPress/gutenberg/pull/14663

It'll require the core fix before we can merge, but it'll make sure it gets caught in the future.

noisysocks commented 4 years ago

Duplicate of https://github.com/WordPress/gutenberg/issues/4483 which has been closed in favour of https://core.trac.wordpress.org/ticket/48034.

noisysocks commented 4 years ago

Change of plan here. As mentioned in https://github.com/WordPress/gutenberg/pull/21043#issuecomment-611318826, changing the order of when the do_blocks will most definitely break plugins and themes. Instead let's fix this by making the Embed block a dynamic block. I'm re-opening this issue to track this work.

paaljoachim commented 3 years ago

Testing with WordPress 5.7 Beta 3. Twenty Twenty One Chrome

Added an embed. Saved it as a Reusable block. Here is the result.

Screen Shot 2021-02-21 at 18 28 03

It looks like it is working correctly. I will go ahead and close this issue. If I am mistaken the issue can of course be reopened.

hrkhal commented 3 years ago

@noisysocks / @paaljoachim This still isn't fixed in 5.7. While the embed blocks within a reusable block preview as expected in the editor they are still output as links on the front end.

The lack of embed support invalidates the entire concept of what reusable blocks are.

paaljoachim commented 3 years ago

Thank you Michael for seeing this! Reopening issue. I had not earlier checked the frontend.

Testing with: WordPress 5.7 Gutenberg plugin: 10.1.1 Twenty Twenty One.

Backend. Youtube embed converted to a Reusable block.

Screen Shot 2021-03-11 at 16 35 15

Frontend. Shows a text link and not an embed.

Screen Shot 2021-03-11 at 16 35 28

@ntsekouras

dougwollison commented 3 years ago

I'm currently adding this filter to work around the issue; it checks if an embed block has an iframe present, and if not runs WP_Embed::autoembed on the innerContent entry. It's not ideal (could also be more robust) but since there's no way to hook into and filter the render_block_core_block contents before/after it goes through do_blocks, I can't think of a saner way that doesn't risk breaking other stuff that's already been filtered (which this one probably arleady risks).

function ensure_autoembed_on_embed_blocks( $block ) {
    global $wp_embed;

    if ( $block['blockName'] === 'core/embed' && strpos( $block['innerContent'][ 0 ], '</iframe>' ) === false ) {
        $block['innerContent'][ 0 ] = $wp_embed->autoembed( $block['innerContent'][ 0 ] );
    }

    return $block;
}
add_filter( 'render_block_data', 'ensure_autoembed_on_embed_blocks', 10, 1 );
orvn commented 3 years ago

The method above of checking the for an iframe tag in the block content and then running it through the embed shortcode again did not work. This other solution didn't help either, but it did point to some ideas.

It really seemed like a sanitization issue, so I looked further into the page template to see how our theme was calling the content.

In our case, we were using

$page_content = get_post_field('post_content', $post->ID); 

(in our case things are very componentized, and this variable is later used as an arg in a component which outputs the markup)

$page_content = apply_filters('the_content', get_post_field('post_content', $post->ID));

This solves the issue in our case, because we apply all the filters, rather than just outputting the content. In the case of other people's theme implementation, it could also happen because the content is being output with get_the_content() instead of the_content(), since the former is just the variable value, with no filters applied.

I'm not sure if this solves the issue for everyone, but it's good to eliminate issues with the way the content gets echoed into the page template first anyway.

LachlanArthur commented 3 years ago

I worked around this by changing the priority of the do_blocks function to run before the \WP_Embed::run_shortcode function.

// Fix embeds inside Gutenberg saved blocks.
// Move priority of `do_blocks` to be earlier than `\WP_Embed::run_shortcode`.
add_action( 'init', function() {
    global $wp_embed;

    if (
        has_filter( 'the_content', 'do_blocks' ) === 9 &&
        has_filter( 'the_content', [ $wp_embed, 'run_shortcode' ] ) === 8
    ) {
        remove_filter( 'the_content', 'do_blocks', 9 );
        add_filter( 'the_content', 'do_blocks', 7 );
    }
} );

It only applies with this exact configuration of priorities, so it should "deactivate" if the filters are modified in the future.

paaljoachim commented 3 years ago

Hi @LachlanArthur Perhaps you could open a PR and suggest this solution? Thanks!