#51850 closed defect (bug) (fixed)
WP_Block: Inner blocks not being set
Reported by: | noisysocks | Owned by: | noisysocks |
---|---|---|---|
Milestone: | 5.6 | Priority: | normal |
Severity: | normal | Version: | 5.6 |
Component: | Editor | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
From https://wordpress.slack.com/archives/C02QB2JS7/p1606038913306200:
Gutenberg 9.4 expects $block→inner_blocks
to be set. WordPress's version of the WP_Block
class is passing $block->parsed_block['innerBlocks']
. Blocks such as Navigation return ''
when inner_blocks isn't set. ie My FSE navigation menus don't work anymore. It looks like some extra shimming is needed.
Change History (8)
This ticket was mentioned in Slack in #core-editor by noisysocks. View the logs.
4 years ago
#3
@
4 years ago
I tested this patch in my local development environment. My navigation menus are working again.
#4
@
4 years ago
This looks like a "logical companion" to WP_Block::__get()
. However in the PHP manual there are some pretty strong, valid objections to using these magic methods: https://www.php.net/manual/en/language.oop5.overloading.php#allnotes.
Looks like in WP 5.5 WP_Block::__get()
was used only to retrieve the block attributes: https://core.trac.wordpress.org/browser/tags/5.5/src/wp-includes/class-wp-block.php#L153, however in 5.6 it changed to get everything.
I'm probably missing something but why is that public __get()
needed? The $parsed_block
is passed in the constructor, so the get_attributes()
, get_inner_blocks()
, get_inner_content()
, etc. would work without additional loading, lazy or not. What is the problem with setting the properties in the constructor at the same time as $parsed_block
? (Looks like they will have to be set in the new WP_Block::reset()
.)
#5
follow-up:
↓ 6
@
4 years ago
See #51612 for some background context here. (Apologies, I should have linked to this ticket.)
We want to call pre_render_block
, render_block_data
, and render_block_context
in WP_Block::render()
as it is not intuitive for these "render" filters to be called in the constructor. WP_Block::$inner_blocks
, WP_Block::$inner_html
, and WP_Block::$inner_content
are all derived from the result of calling these filters.
This means we either have to:
1) Eagerly set the properties in the constructor and then re-set them in render()
; or
2) Make the properties lazy.
I opted for (2) because (1) involves doing unnecessary work in the constructor and WP_Block::render()
is a fairly warm path that is called when viewing a post.
Note that the dynamic properties are documented using @property
PHPDoc attributes and so auto completion will still work for these properties in most IDEs.
#6
in reply to:
↑ 5
@
4 years ago
Replying to noisysocks:
We want to call
pre_render_block
,render_block_data
, andrender_block_context
inWP_Block::render()
as it is not intuitive for these "render" filters to be called in the constructor.WP_Block::$inner_blocks
,WP_Block::$inner_html
, andWP_Block::$inner_content
are all derived from the result of calling these filters.
Thanks for the explanation :)
That seems to introduce two states of WP_Block
, one with data from before the filters are run, and another after the filters (after render()
). This seems to bring some back-compat problems: filters were run on the block data before WP_Block was instantiated, so the filtered, "correct" data was in the instance from the beginning. Now the "correct" data is available only after WP_Block::render()
has been run.
Then as far as I see the magic __get()
and __isset()
are needed to support getting pre-filtered (wrong?) data, or rather may get the wrong data depending on whether render() was run or not.
Going to reopen #51612 with the above questions.
#7
@
4 years ago
- Resolution set to fixed
- Status changed from assigned to closed
This is fixed by reverting the changes made in #51612 in [49694,49695].
https://github.com/WordPress/wordpress-develop/pull/756 contains a fix.