Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embed block fails when converted to reusable #14608

Closed
nicopujol opened this issue Mar 25, 2019 · 15 comments · Fixed by #37554
Closed

Embed block fails when converted to reusable #14608

nicopujol opened this issue Mar 25, 2019 · 15 comments · Fixed by #37554
Assignees
Labels
[Block] Embed Affects the Embed Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@nicopujol
Copy link

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

  • OS: Win 10-64
  • Browser Chrome
  • Version 73
@youknowriad youknowriad added [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Block] Embed Affects the Embed Block labels Mar 25, 2019
@youknowriad
Copy link
Contributor

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

cc @notnownikki

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Mar 25, 2019
@notnownikki
Copy link
Member

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 notnownikki self-assigned this Mar 25, 2019
@notnownikki
Copy link
Member

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
Copy link
Contributor

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

@notnownikki
Copy link
Member

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 notnownikki reopened this Mar 26, 2019
@notnownikki
Copy link
Member

I've opened a PR with a test to cover this at #14663

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

@noisysocks
Copy link
Member

noisysocks commented Apr 20, 2020

Duplicate of #4483 which has been closed in favour of https://core.trac.wordpress.org/ticket/48034.

@noisysocks noisysocks added the [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed label Apr 20, 2020
@noisysocks
Copy link
Member

Change of plan here. As mentioned in #21043 (comment), 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
Copy link
Contributor

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
Copy link

hrkhal commented Mar 11, 2021

@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
Copy link
Contributor

paaljoachim commented Mar 11, 2021

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

@paaljoachim paaljoachim reopened this Mar 11, 2021
@earnjam earnjam removed their assignment Mar 11, 2021
@dougwollison
Copy link
Contributor

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
Copy link

orvn commented Apr 8, 2021

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
Copy link

LachlanArthur commented May 5, 2021

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
Copy link
Contributor

paaljoachim commented May 5, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Embed Affects the Embed Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet