Fix dynamic blocks not rendering in the frontend #11050

youknowriad · 2018-10-25T10:08:28Z

Regression in 4.1 when you have dynamic blocks in your post and a meta box (like yoast). The dynamic blocks get stripped from the frontend.

In this PR https://github.com/WordPress/gutenberg/pull/10035/files#r228107590 we changed the priority of the different the_content hooks but we forgot to update the hook removal properly?

Testing instructions

Install Yoast
Add a dynamic block (latest posts)
The block should show up in the frontend
Would be good to check that the excerpts are still stripping the dynamic blocks.

Related #8984

notnownikki

Tested with Yoast, publishing a post with a reusable block that has an embedded tweet, to make sure that everything gets rendered. 🚢

ocean90

Thanks! The fix is working for me as well.

Wondering if we can add a test for this so it doesn't happen again?

gziolo · 2018-10-25T10:22:24Z

If Travis is unhappy about coding styles, see #11051.

notnownikki · 2018-10-25T10:22:53Z

@ocean90 I'm working on a test now

mcsf · 2018-10-25T11:13:32Z

lib/blocks.php

@@ -312,7 +312,7 @@ function strip_dynamic_blocks_add_filter( $text ) {
 	 * @return string
 	 */
 	function strip_dynamic_blocks_remove_filter( $text ) {
-		remove_filter( 'the_content', 'strip_dynamic_blocks', 8 );
+		remove_filter( 'the_content', 'strip_dynamic_blocks', 6 );


Should we have a constant to share that 6 between the _add_ and _remove_ pair?

mcsf · 2018-10-25T11:30:02Z

The whole system for managing filter priority is fragile. :( Adding hooks in sequence would help (see example from core) but it doesn't work once we need to register temporary filters, such as the ones for excerpts.

There aren't as many filters as in the past now, but would it make sense to assign priorities in a central map?

$the_content_priority_strip_dynamic_blocks = 6;
$the_content_priority_do_blocks = 7;
$the_content_priority_gutenberg_wpautop = 8; // (do we still need `8`?)

youknowriad · 2018-10-25T11:31:38Z

@mcsf Sounds like a good idea to me.

Also ping @pento because this might have an impact on 5.0 as well

youknowriad · 2018-10-25T11:37:21Z

$the_content_priority_gutenberg_wpautop = 8; // (do we still need 8?)

That's a good question, and if I were to guess, it needs to run before do_blocks as this was the case before #10035 (@aduth is this true?)

We might need an e2e test for it.

mcsf · 2018-10-25T11:41:44Z

$the_content_priority_gutenberg_wpautop = 8; // (do we still need 8?)

That's a good question, and if I were to guess, it needs to run before do_blocks as this was the case before #10035 (@aduth is this true?)

I ask because, in the 5.0 branch, no priority is assigned. This discrepancy worries me. /cc @pento

youknowriad · 2018-10-25T12:17:13Z

test/e2e/specs/meta-boxes.test.js

@@ -35,4 +35,27 @@ describe( 'Meta boxes', () => {
 			page.keyboard.up( 'Meta' ),
 		] );
 	} );
+
+	it( 'Should render dynamic blocks when the meta box uses the excerpt for front end rendering', async () => {


❤️ Thanks for the test @notnownikki

youknowriad · 2018-10-25T12:41:07Z

https://wordpress.slack.com/archives/C02QB2JS7/p1540470819000100 This is probably related to the "autop" hook right?

lkraav · 2018-10-25T13:06:22Z

lib/blocks.php

@@ -292,7 +295,7 @@ function strip_dynamic_blocks( $content ) {
 	 * @return string
 	 */
 	function strip_dynamic_blocks_add_filter( $text ) {
-		add_filter( 'the_content', 'strip_dynamic_blocks', 6 ); // Before do_blocks().
+		add_filter( 'the_content', 'strip_dynamic_blocks', $the_content_priority_strip_dynamic_blocks );


@youknowriad for some reason I'm getting Undefined variable: the_content_priority_strip_dynamic_blocks here.

Scoping issues?

yes, too much JS in my head. it should be fixed now.

If we're global-ing already here + lengthy-named tracker variables across 2 files, is there some decent global class property available instead?

GutenbergVariables::$the_content_priority['strip_dynamic_block'] = 6, etc.

We're removing all gutenberg from variable names (that will make it into Core). I don't have strong opinion personally on how to set these variables. I assume this will be different in Core and this code will be removed from the plugin once 5.0 is released.

In the wild, I'm seeing plugins that need to do special content processing, currently have to hardcode things like remove_filter the_content gutenberg_wpautop 8, which means they instantly break when a release comes along that changes priorities.

Which is why some kind of a "well-known things" registry to pull from, would be much nicer for everybody.

But I understand this is a massive undertaking and can't be addressed here.

@lkraav fair point and I'd suggest opening a trac issue for it as, this code is already merged there for 5.0 beta.

I restored the hard-coded priorities for now. The variables were messing with the php unit tests. Any refactoring should happen separately.

designsimply · 2018-10-25T14:15:18Z

Noting that this problem was reported to Yoast at Yoast/wordpress-seo#11402 and I commented to note that this PR will fix the problem: Yoast/wordpress-seo#11402 (comment)

youknowriad · 2018-10-25T14:25:30Z

I added an e2e test to catch autop issues in the future.

mcsf · 2018-10-25T14:33:43Z

I ask because, in the 5.0 branch, no priority is assigned

This issue referenced in t45172.

lkraav · 2018-10-25T16:32:43Z

@youknowriad thanks. Are you planning a hotfix 4.1.1 here, or?

mcsf · 2018-10-25T17:13:53Z

@lkraav: yes

* Fix dynamic blocks not rendering in the frontend * Use variables for hook priorities * Test that dynamic blocks are rendered when meta boxes use the excerpt * Fix autop hook order * Fix linting * Fix scoping issues * Add an e2e test for the autop issue * copy/paste issues * Trying to fix travis * Trim content to avoid small differences between themes

…rnmobile/merge-blocks-on-backspace * 'master' of https://github.com/WordPress/gutenberg: Do not add isDirty prop to DOM (#11093) Format API (#10209) Remove 4.2 deprecated features (#10952) Update `@wordpress/hooks` README to include namespace mention (#11061) Feature: save lock control via actions (#10649) Fix usage of `preg_quote()` (#10998) Update plugin version to 4.1.1 (#11078) Improve preloading request code (#11007) Fix dynamic blocks not rendering in the frontend (#11050) Media & Text: Fixing vertical alignment of the image (#11025) Date: Mark getSettings as experimental (#10636) Improve handling of centered 1-col galleries with small images (#11040) Use better help text for ALT text input; fixes #8391. (#11052) # Conflicts: # packages/editor/src/components/rich-text/index.native.js

youknowriad added [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release labels Oct 25, 2018

youknowriad self-assigned this Oct 25, 2018

youknowriad requested a review from a team October 25, 2018 10:08

notnownikki approved these changes Oct 25, 2018

View reviewed changes

ocean90 approved these changes Oct 25, 2018

View reviewed changes

ocean90 mentioned this pull request Oct 25, 2018

Reusable blocks not showing up on frontend #10942

Closed

mcsf reviewed Oct 25, 2018

View reviewed changes

youknowriad mentioned this pull request Oct 25, 2018

strip_dynamic_blocks is removed from the wrong priority, dynamic blocks are not rendered #11056

Closed

youknowriad commented Oct 25, 2018

View reviewed changes

This was referenced Oct 25, 2018

Fix WordPress block resolution and embeds as reusable blocks #10035

Merged

Gutenberg wraps Div->A->Div with P #11038

Closed

lkraav reviewed Oct 25, 2018

View reviewed changes

designsimply mentioned this pull request Oct 25, 2018

Gutenberg Reusable Blocks don't display on front end. Yoast/wordpress-seo#11402

Closed

10 tasks

youknowriad added this to the 4.1 - UI freeze milestone Oct 25, 2018

youknowriad and others added 5 commits October 25, 2018 15:41

Fix dynamic blocks not rendering in the frontend

c45eaa8

Use variables for hook priorities

f1fb1eb

Test that dynamic blocks are rendered when meta boxes use the excerpt

fb382f8

Fix autop hook order

b605ed8

Fix linting

169f5f4

youknowriad added 2 commits October 25, 2018 15:41

Fix scoping issues

544c337

Add an e2e test for the autop issue

60e22bf

youknowriad force-pushed the fix/dynamic-blocks-with-metaboxes branch from 80014f4 to 60e22bf Compare October 25, 2018 14:41

youknowriad added 2 commits October 25, 2018 15:45

copy/paste issues

70be656

Trying to fix travis

c9899c3

youknowriad mentioned this pull request Oct 25, 2018

<br> tags appearing in random Parapgraph blocks #11037

Closed

Trim content to avoid small differences between themes

8152dca

youknowriad merged commit 4317761 into master Oct 25, 2018

youknowriad deleted the fix/dynamic-blocks-with-metaboxes branch October 25, 2018 16:05

youknowriad mentioned this pull request Oct 25, 2018

Gutenberg 4.1 custom HTML gets unwanted <br> #11031

Closed

mcsf mentioned this pull request Oct 25, 2018

Update plugin version to 4.1.1 #11078

Merged

antpb mentioned this pull request Oct 25, 2018

[4.1-RC2] BUG: Background color breaks mid-paragraph using nbsp #10954

Closed

earnjam mentioned this pull request Mar 20, 2020

Fix embedded blocks when converted to reusable blocks #21043

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dynamic blocks not rendering in the frontend #11050

Fix dynamic blocks not rendering in the frontend #11050

youknowriad commented Oct 25, 2018 •

edited

Loading

notnownikki left a comment

ocean90 left a comment

gziolo commented Oct 25, 2018

notnownikki commented Oct 25, 2018

mcsf Oct 25, 2018

mcsf commented Oct 25, 2018

youknowriad commented Oct 25, 2018

youknowriad commented Oct 25, 2018

mcsf commented Oct 25, 2018

youknowriad Oct 25, 2018

youknowriad commented Oct 25, 2018

lkraav Oct 25, 2018

youknowriad Oct 25, 2018

youknowriad Oct 25, 2018

lkraav Oct 25, 2018

youknowriad Oct 25, 2018

lkraav Oct 25, 2018

youknowriad Oct 25, 2018

youknowriad Oct 25, 2018

designsimply commented Oct 25, 2018 •

edited

Loading

youknowriad commented Oct 25, 2018

mcsf commented Oct 25, 2018

lkraav commented Oct 25, 2018

mcsf commented Oct 25, 2018

Fix dynamic blocks not rendering in the frontend #11050

Fix dynamic blocks not rendering in the frontend #11050

Conversation

youknowriad commented Oct 25, 2018 • edited Loading

notnownikki left a comment

Choose a reason for hiding this comment

ocean90 left a comment

Choose a reason for hiding this comment

gziolo commented Oct 25, 2018

notnownikki commented Oct 25, 2018

Choose a reason for hiding this comment

mcsf commented Oct 25, 2018

youknowriad commented Oct 25, 2018

youknowriad commented Oct 25, 2018

mcsf commented Oct 25, 2018

Choose a reason for hiding this comment

youknowriad commented Oct 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

designsimply commented Oct 25, 2018 • edited Loading

youknowriad commented Oct 25, 2018

mcsf commented Oct 25, 2018

lkraav commented Oct 25, 2018

mcsf commented Oct 25, 2018

youknowriad commented Oct 25, 2018 •

edited

Loading

designsimply commented Oct 25, 2018 •

edited

Loading