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.

Sign up for GitHub

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

Block Hooks: Apply to Post Content (on frontend and in editor) #7898

Closed
wants to merge 20 commits into from
+88 −25
Closed

Block Hooks: Apply to Post Content (on frontend and in editor) #7898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
0711c61
Block Hooks: Apply to Post Content (on frontend)
ockham Nov 25, 2024
b40dd94
Solve by using a default instead
ockham Nov 25, 2024
20a5470
Insert hooked block into editor
ockham Nov 26, 2024
90abff4
Update PHPDoc
ockham Nov 26, 2024
0b844b3
Set ignoredHookedBlocks in REST API response
ockham Nov 26, 2024
838a75e
Also apply to pages
ockham Nov 28, 2024
6793b55
Update PHPDoc
ockham Nov 28, 2024
2958c64
Tweak PHPDoc
ockham Nov 28, 2024
74b39fa
More PHPDoc tweaking
ockham Nov 28, 2024
007d7a4
Temporarily remove filter
ockham Nov 28, 2024
1d98a27
Bail early if no content is given
ockham Nov 28, 2024
c6c8481
Revert "Bail early if no content is given"
ockham Nov 28, 2024
f29b288
Tighten checks
ockham Nov 28, 2024
a5cb598
Add since PHPDoc
ockham Dec 3, 2024
886d375
Update ignoredHookedBlocks post meta on save
ockham Dec 3, 2024
89b835d
Add back check for post_type
ockham Dec 3, 2024
e14db01
Tweak @since PHPDoc
ockham Dec 3, 2024
7635d23
Tweak param description
ockham Dec 3, 2024
2b7d36a
Add comments to clarify removal/re-adding of `the_content` filter
ockham Dec 16, 2024
a7693b0
Add test case for context set to null
ockham Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 62 additions & 24 deletions src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ function insert_hooked_blocks( &$parsed_anchor_block, $relative_position, $hooke
* @param string $relative_position The relative position of the hooked blocks.
* Can be one of 'before', 'after', 'first_child', or 'last_child'.
* @param string $anchor_block_type The anchor block type.
* @param WP_Block_Template|WP_Post|array $context The block template, template part, `wp_navigation` post type,
* @param WP_Block_Template|WP_Post|array $context The block template, template part, post object,
* or pattern that the anchor block belongs to.
*/
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
Expand All @@ -935,7 +935,7 @@ function insert_hooked_blocks( &$parsed_anchor_block, $relative_position, $hooke
* @param string $hooked_block_type The hooked block type name.
* @param string $relative_position The relative position of the hooked block.
* @param array $parsed_anchor_block The anchor block, in parsed block array format.
* @param WP_Block_Template|WP_Post|array $context The block template, template part, `wp_navigation` post type,
* @param WP_Block_Template|WP_Post|array $context The block template, template part, post object,
* or pattern that the anchor block belongs to.
*/
$parsed_hooked_block = apply_filters( 'hooked_block', $parsed_hooked_block, $hooked_block_type, $relative_position, $parsed_anchor_block, $context );
Expand All @@ -951,7 +951,7 @@ function insert_hooked_blocks( &$parsed_anchor_block, $relative_position, $hooke
* @param string $hooked_block_type The hooked block type name.
* @param string $relative_position The relative position of the hooked block.
* @param array $parsed_anchor_block The anchor block, in parsed block array format.
* @param WP_Block_Template|WP_Post|array $context The block template, template part, `wp_navigation` post type,
* @param WP_Block_Template|WP_Post|array $context The block template, template part, post object,
* or pattern that the anchor block belongs to.
*/
$parsed_hooked_block = apply_filters( "hooked_block_{$hooked_block_type}", $parsed_hooked_block, $hooked_block_type, $relative_position, $parsed_anchor_block, $context );
Expand Down Expand Up @@ -1039,17 +1039,25 @@ function set_ignored_hooked_blocks_metadata( &$parsed_anchor_block, $relative_po
*
* @since 6.6.0
* @since 6.7.0 Injects the `theme` attribute into Template Part blocks, even if no hooked blocks are registered.
* @since 6.8.0 Have the `$context` parameter default to `null`, in which case `get_post()` will be called to use the current post as context.
* @access private
*
* @param string $content Serialized content.
* @param WP_Block_Template|WP_Post|array $context A block template, template part, `wp_navigation` post object,
* or pattern that the blocks belong to.
* @param callable $callback A function that will be called for each block to generate
* the markup for a given list of blocks that are hooked to it.
* Default: 'insert_hooked_blocks'.
* @param string $content Serialized content.
* @param WP_Block_Template|WP_Post|array|null $context A block template, template part, post object, or pattern
* that the blocks belong to. If set to `null`, `get_post()`
* will be called to use the current post as context.
* Default: `null`.
* @param callable $callback A function that will be called for each block to generate
* the markup for a given list of blocks that are hooked to it.
* Default: 'insert_hooked_blocks'.
* @return string The serialized markup.
*/
function apply_block_hooks_to_content( $content, $context, $callback = 'insert_hooked_blocks' ) {
function apply_block_hooks_to_content( $content, $context = null, $callback = 'insert_hooked_blocks' ) {
// Default to the current post if no context is provided.
if ( null === $context ) {
$context = get_post();
}
ockham marked this conversation as resolved.
Show resolved Hide resolved

$hooked_blocks = get_hooked_blocks();

$before_block_visitor = '_inject_theme_attribute_in_template_part_block';
Expand Down Expand Up @@ -1165,36 +1173,37 @@ function extract_serialized_parent_block( $serialized_block ) {
}

/**
* Updates the wp_postmeta with the list of ignored hooked blocks where the inner blocks are stored as post content.
* Currently only supports `wp_navigation` post types.
* Updates the wp_postmeta with the list of ignored hooked blocks
* where the inner blocks are stored as post content.
*
* @since 6.6.0
* @since 6.8.0 Support non-`wp_navigation` post types.
* @access private
*
* @param stdClass $post Post object.
* @return stdClass The updated post object.
*/
function update_ignored_hooked_blocks_postmeta( $post ) {
/*
* In this scenario the user has likely tried to create a navigation via the REST API.
* In this scenario the user has likely tried to create a new post object via the REST API.
* In which case we won't have a post ID to work with and store meta against.
*/
if ( empty( $post->ID ) ) {
return $post;
}

/*
* Skip meta generation when consumers intentionally update specific Navigation fields
* Skip meta generation when consumers intentionally update specific fields
* and omit the content update.
*/
if ( ! isset( $post->post_content ) ) {
return $post;
}

/*
* Skip meta generation when the post content is not a navigation block.
* Skip meta generation if post type is not set.
*/
if ( ! isset( $post->post_type ) || 'wp_navigation' !== $post->post_type ) {
if ( ! isset( $post->post_type ) ) {
return $post;
}

Expand All @@ -1208,8 +1217,14 @@ function update_ignored_hooked_blocks_postmeta( $post ) {
);
}

if ( 'wp_navigation' === $post->post_type ) {
$wrapper_block_type = 'core/navigation';
} else {
$wrapper_block_type = 'core/post-content';
}

$markup = get_comment_delimited_block_content(
'core/navigation',
$wrapper_block_type,
$attributes,
$post->post_content
);
Expand Down Expand Up @@ -1266,16 +1281,17 @@ function insert_hooked_blocks_and_set_ignored_hooked_blocks_metadata( &$parsed_a
}

/**
* Hooks into the REST API response for the core/navigation block and adds the first and last inner blocks.
* Hooks into the REST API response for the Posts endpoint and adds the first and last inner blocks.
*
* @since 6.6.0
* @since 6.8.0 Support non-`wp_navigation` post types.
*
* @param WP_REST_Response $response The response object.
* @param WP_Post $post Post object.
* @return WP_REST_Response The response object.
*/
function insert_hooked_blocks_into_rest_response( $response, $post ) {
if ( ! isset( $response->data['content']['raw'] ) || ! isset( $response->data['content']['rendered'] ) ) {
if ( empty( $response->data['content']['raw'] ) || empty( $response->data['content']['rendered'] ) ) {
gziolo marked this conversation as resolved.
Show resolved Hide resolved
return $response;
}

Expand All @@ -1287,22 +1303,44 @@ function insert_hooked_blocks_into_rest_response( $response, $post ) {
'ignoredHookedBlocks' => $ignored_hooked_blocks,
);
}

if ( 'wp_navigation' === $post->post_type ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about synced patterns wp/block? Should it be extensible? These post types are special do maybe it’s fine as is but a filter here could be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point. I guess the wp_block post type should map to core/block.

A filter might make sense here 👍 Something to map post types to block types. (I wonder if we already have a function or data structure to do that somewhere 🤔 )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've finally tested this with a synced pattern, and it's currently not inserting hooked blocks.

Simply adding elseif ( 'wp_block' === $post->post_type ) { $wrapper_block_type = 'core/block'; } didn't do the trick, so I'll have to look into this a bit more. However, since this PR has been open for a while and seems to be ready otherwise, I'll merge it and tackle synced patterns in a follow-up 😊

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. It was also discussed in https://github.com/WordPress/wordpress-develop/pull/7898/files#r1867470796 to add support for every post type. The challenge with Patterns is that they are handled out of the box if they come from files bundled from the theme or when registered with PHP. The last missing puzzle would be a synced version of the pattern.

$wrapper_block_type = 'core/navigation';
} else {
$wrapper_block_type = 'core/post-content';
}

$content = get_comment_delimited_block_content(
'core/navigation',
$wrapper_block_type,
$attributes,
$response->data['content']['raw']
);

$content = apply_block_hooks_to_content( $content, $post );
$content = apply_block_hooks_to_content(
$content,
$post,
'insert_hooked_blocks_and_set_ignored_hooked_blocks_metadata'
);
Comment on lines +1319 to +1323
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're now inserting hooked blocks and setting ignoredHookedBlocks metadata. Previously, we were only inserting hooked blocks (i.e. the default behavior of apply_block_hooks_to_content when no third argument is given).

This change also affects the Navigation block. We need to make sure that any hooked blocks inside of the Navigation block -- both as first/last child of core/navigation itself, but also after/before blocks inside of the Navigation block -- still work (both on the frontend and in the editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To test if the Navigation block still works, we can follow the testing instructions from WordPress/gutenberg#59021.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out there was an unrelated issue with this. I've filed a fix: #7941

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've committed #7941 to Core [62639], and rebased this PR to include the fix.


// Remove mock Navigation block wrapper.
// Remove mock block wrapper.
$content = remove_serialized_parent_block( $content );

$response->data['content']['raw'] = $content;

// `apply_block_hooks_to_content` is called above. Ensure it is not called again as a filter.
$priority = has_filter( 'the_content', 'apply_block_hooks_to_content' );
if ( false !== $priority ) {
remove_filter( 'the_content', 'apply_block_hooks_to_content', $priority );
}

/** This filter is documented in wp-includes/post-template.php */
$response->data['content']['rendered'] = apply_filters( 'the_content', $content );

// Restore the filter if it was set initially.
if ( false !== $priority ) {
add_filter( 'the_content', 'apply_block_hooks_to_content', $priority );
}

return $response;
}

Expand All @@ -1320,7 +1358,7 @@ function insert_hooked_blocks_into_rest_response( $response, $post ) {
* @access private
*
* @param array $hooked_blocks An array of blocks hooked to another given block.
* @param WP_Block_Template|WP_Post|array $context A block template, template part, `wp_navigation` post object,
* @param WP_Block_Template|WP_Post|array $context A block template, template part, post object,
* or pattern that the blocks belong to.
* @param callable $callback A function that will be called for each block to generate
* the markup for a given list of blocks that are hooked to it.
Expand Down Expand Up @@ -1377,7 +1415,7 @@ function make_before_block_visitor( $hooked_blocks, $context, $callback = 'inser
* @access private
*
* @param array $hooked_blocks An array of blocks hooked to another block.
* @param WP_Block_Template|WP_Post|array $context A block template, template part, `wp_navigation` post object,
* @param WP_Block_Template|WP_Post|array $context A block template, template part, post object,
* or pattern that the blocks belong to.
* @param callable $callback A function that will be called for each block to generate
* the markup for a given list of blocks that are hooked to it.
Expand Down
7 changes: 6 additions & 1 deletion src/wp-includes/default-filters.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@
add_filter( 'the_title', 'convert_chars' );
add_filter( 'the_title', 'trim' );

add_filter( 'the_content', 'apply_block_hooks_to_content', 8 ); // BEFORE do_blocks().
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting #7889 (comment):

apply_block_hooks_to_content parses blocks, traverses the block tree (to insert hooked blocks), and reserializes them. Note that it's followed immediately by do_blocks (on the line below), which also parses blocks.

This means that in theory, there's some potential for optimization (parse only once inside of do_blocks, and traverse to insert hooked blocks there).

In practice, I'm not sure if that's viable, or if that might cause some collisions; in particular if do_blocks is already used in other contexts where hooked blocks are already inserted (through a different mechanism), e.g. in templates or wp_navigation posts.

add_filter( 'the_content', 'do_blocks', 9 );
add_filter( 'the_content', 'wptexturize' );
add_filter( 'the_content', 'convert_smilies', 20 );
Expand Down Expand Up @@ -760,9 +761,13 @@
add_filter( 'rest_pre_insert_wp_template_part', 'inject_ignored_hooked_blocks_metadata_attributes' );

// Update ignoredHookedBlocks postmeta for wp_navigation post type.
add_filter( 'rest_pre_insert_page', 'update_ignored_hooked_blocks_postmeta' );
add_filter( 'rest_pre_insert_post', 'update_ignored_hooked_blocks_postmeta' );
add_filter( 'rest_pre_insert_wp_navigation', 'update_ignored_hooked_blocks_postmeta' );

// Inject hooked blocks into the wp_navigation post type REST response.
// Inject hooked blocks into the Posts endpoint REST response for some given post types.
ockham marked this conversation as resolved.
Show resolved Hide resolved
add_filter( 'rest_prepare_page', 'insert_hooked_blocks_into_rest_response', 10, 2 );
add_filter( 'rest_prepare_post', 'insert_hooked_blocks_into_rest_response', 10, 2 );
ockham marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +769 to +770
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows the precedent of rest_prepare_wp_navigation below. However, it has the unfortunate side effect that Block Hooks aren't automatically applied to user-defined CPTs; those would need to be added manually (via add_filter( 'rest_prepare_my_cpt', ... )). (AFAIK, there's no "generic" rest_prepare filter that could cover all post types.)

I wonder if we should run insert_hooked_blocks_into_rest_response from inside the Posts controller instead, in order to insert hooked blocks into every CPT that has show_in_rest set to true 🤔 cc/ @gziolo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be worthwhile to explore supporting other types than post, page, and wp_navigation.

add_filter( 'rest_prepare_wp_navigation', 'insert_hooked_blocks_into_rest_response', 10, 2 );

unset( $filter, $action );
20 changes: 20 additions & 0 deletions tests/phpunit/tests/blocks/applyBlockHooksToContent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,26 @@ public function test_apply_block_hooks_to_content_inserts_hooked_block() {
);
}

/**
* @ticket 61074
*/
public function test_apply_block_hooks_to_content_with_context_set_to_null() {
$content = '<!-- wp:tests/anchor-block /-->';

/*
* apply_block_hooks_to_content() will fall back to the global $post object (via get_post())
* if the $context parameter is null. However, we'd also like to ensure that the function
* works as expected even when get_post() returns null.
*/
$this->assertNull( get_post() );

$actual = apply_block_hooks_to_content( $content, null, 'insert_hooked_blocks' );
$this->assertSame(
'<!-- wp:tests/anchor-block /--><!-- wp:tests/hooked-block /-->',
$actual
);
}

/**
* @ticket 61902
*/
Expand Down
Loading