Block supports: Return from layout support before resolving global settings - #80771
Conversation
…ttings `gutenberg_render_layout_support_flag()` called `gutenberg_get_global_settings()` before checking whether the block could produce any layout output. Resolving global settings is not read-only. On a cold cache it queries the user's `wp_global_styles` post, and that query fires `the_posts`. A plugin whose `the_posts` callback renders blocks re-enters `render_block`, which re-enters the settings lookup, which runs the query again. Before #79104 the lookup sat below the no-layout early return, so rendering the global styles post content (one block with a `null` name) stopped the cycle after one iteration. Moving the lookup above that return removed the base case and the request could exhaust the PHP memory limit. The guard needed `$viewport_child_layouts`, which needs the responsive media queries, which come from settings, hence the move. But whether any responsive child layout exists at all can be answered from the style attribute alone, so check that first and return before the lookup. Fixes #80770
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts layout block support rendering to avoid resolving global settings for blocks that cannot produce layout output, preventing re-entrant render_block recursion triggered via the_posts during cold-cache global styles resolution.
Changes:
- Add a pre-check for any child layout data in
attrs.style(including responsive state keys) and return early when layout output is impossible. - Keep the existing
empty( $viewport_child_layouts )guard, but move the expensive global settings lookup below the new early-return conditions. - Add a PHPUnit regression test to ensure blocks with no layout support do not resolve user global styles data (and a control case to ensure the assertion isn’t due to a warm cache).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/block-supports/layout.php |
Reorders logic to bail out before global settings resolution when no layout output is possible, preventing recursion. |
phpunit/block-supports/layout-test.php |
Adds regression coverage for the early bailout and cache-cold behavior around global settings resolution. |
Comments suppressed due to low confidence (1)
phpunit/block-supports/layout-test.php:1255
- Please remove the
wp_theme_json_data_userfilter at the end of the test to avoid affecting subsequent tests in the same process.
$this->assertGreaterThan(
0,
$user_data_resolutions,
'Global settings should still be resolved for a block that supports layout.'
);
The pre-check that decides whether a block carries a responsive child layout scanned every top-level `style` entry. Any sub-array holding a `layout` key matched, so a block could resolve global settings when it had no responsive child layout at all. Style state keys are always prefixed with `@`, and the loop that consumes them below only ever sees `@mobile`, `@tablet`, and `@desktop` from `get_viewport_media_queries()`, so match on that prefix instead. Also covers the case the pre-check exists for: a block with no layout support whose only child layout sits under `@mobile` still gets its container content class and media query rule. That path had no PHP test.
The early return was guarded by a separate scan of the style attribute that matched any `@` prefixed key carrying a `layout` value. That left the function scanning twice and guarding twice, with the first guard an approximation of the second. Only the media query strings depend on global settings. The style state keys do not: `sanitize_viewport_settings()` only ever returns a subset of `WP_Theme_JSON_Gutenberg::DEFAULT_VIEWPORT_BREAKPOINTS`. So collect the child layouts from that constant, keep the original single guard, and resolve settings afterwards only to attach the media queries and drop states the theme has no breakpoint for. This restores the shape the function had before #79104, when the breakpoints came from a constant, and removes the prefix heuristic. Adds coverage for the drop: a child layout under a breakpoint the theme does not define adds no markup and emits no CSS.
The fix had grown to reorder the child layout collection and re-source its breakpoint keys, which is more logic than the bug needs and more than can be shown not to change rendering. Reduce it to the smallest guard that closes the recursion: a block with no layout support and no style attribute at all cannot produce layout output, so return before resolving global settings. The block that closes the cycle is the `wp_global_styles` post content parsed as a single block with no name and no attributes, so that condition always holds for it. Every block that renders anything today takes exactly the path it took before, because a block with no style attribute already returned unchanged further down. The collection loop and both existing guards are back to their trunk form. Drops the two responsive child layout tests along with it. They covered the mechanisms this commit removes, not the fix, and the behaviour they exercised predates this branch.
|
Tiny note: for manual testing I had to use the following instead as the included test snippet wound up doing an infinite loop on the Query Loop block, which is unrelated to this fix. Here's a Claude-generated snippet in case it helps anyone else test this fix without the unrelated side-effect of the Query Loop block rendering: <?php
/**
* Repro for https://github.com/WordPress/gutenberg/issues/80770
*
* Renders blocks from a `the_posts` callback, which is what re-enters the
* global settings lookup while it is still resolving.
*
* Scoped to the main query and the `wp_global_styles` query. Rendering every
* query's posts adds a second, unrelated infinite loop, since a Query Loop
* block runs its own WP_Query whose `the_posts` renders those posts, which
* render their own Query Loop. That loop exhausts memory on its own, on trunk
* and with the fix applied alike, which makes the actual bug hard to see.
*/
add_filter(
'the_posts',
function ( $posts, $query ) {
if ( ! $query->is_main_query() && 'wp_global_styles' !== $query->get( 'post_type' ) ) {
return $posts;
}
foreach ( $posts as $post ) {
do_blocks( $post->post_content );
}
return $posts;
},
10,
2
); |
andrewserong
left a comment
There was a problem hiding this comment.
Testing nicely for me, thanks for the quick fix!
Returning early when there's no layout support and nothing in the style attribute that would result in output seems like the right way to go to me 🎉
|
Thank you! |
…ttings (#80771) Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: fushar <fushar@git.wordpress.org>
|
I just cherry-picked this PR to the wp/7.1 branch to get it included in the next release: d2a421b |
|
I just cherry-picked this PR to the release/23.6 branch to get it included in the next release: 8b13137 |
…ttings (#80771) Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: fushar <fushar@git.wordpress.org>
What
Move the global settings lookup in
gutenberg_render_layout_support_flag()back below the early return for blocks that cannot produce layout output.Why
Resolving global settings is not read-only. On a cold cache it runs
WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()->get_user_data(), which fires aWP_Queryfor the user'swp_global_stylespost, which firesthe_posts. A plugin whosethe_postscallback renders blocks (a legitimate way to collect block asset dependencies) re-entersrender_block, which re-enters the settings lookup, which runs the query again.Before #79104 the lookup sat below the
! $block_supports_layoutreturn. Thewp_global_stylespost content is a single line of JSON with no block delimiters, soparse_blocks()yields one block withblockName === null, which has no block type and therefore no layout support. That return was the base case, and the cycle closed after one iteration.#79104 had to move the lookup up: the guard depended on
$viewport_child_layouts, which needs the responsive media queries, which now come fromsettings.viewport. The base case went with it, and the request can now exhaust the PHP memory limit.Fixes #80770.
What changed
lib/block-supports/layout.php: one early return, before the lookup.The block that closes the cycle is the
wp_global_stylespost content parsed as a single block with no name and no attributes, so that condition always holds for it.Nothing else in the function moves. The collection loop and both existing guards are untouched, and every block that renders anything today takes exactly the path it took before, because a block with no style attribute already returned unchanged twenty lines further down. The only other change in the file is whitespace realignment that phpcs requires once a statement is inserted into that assignment block.
phpunit/block-supports/layout-test.php: one test, asserting that a block with no layout support does not resolve user global styles data, with a control case proving that is not a warm cache passing by accident.The other
render_blockfilters that resolve settings (position.php,typography.php,states.php,block-visibility.php) already return before their lookup for a block with no name, so layout was the only one affected.Manual testing
Requires a site whose active theme has a user global styles post. Opening the site editor once creates it.
Check out
trunkand open the site editor once so thewp_global_stylespost exists.Add this mu-plugin:
Make sure at least one published post contains a block with layout support, for example a Group block.
Load the front page. It dies with
Allowed memory size ... exhausted.Check out this branch and reload the front page. It renders, and the callback's
do_blocks()calls complete.Confirm ordinary layout output is unchanged: a Group block with a content width still renders
is-layout-constrained, and a Grid child with a column span still gets itswp-container-content-*class.