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: Respect "multiple": false #7443

Closed
wants to merge 13 commits into from
Closed

Block Hooks: Respect "multiple": false #7443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
dcbdba9
Block Hooks: Respect `"multiple": false`
ockham Sep 27, 2024
7affa65
Polish a little
ockham Sep 27, 2024
c1319fc
Make anonymous function static
ockham Sep 27, 2024
034146f
Correctly format multi-line comment
ockham Sep 27, 2024
a3c1b82
Slightly rephrase comment
ockham Sep 27, 2024
92d8920
Another multi-line comment
ockham Sep 27, 2024
257225c
Revert "Use references instead of indices in loop"
ockham Sep 27, 2024
fab6700
Prune only after finishing loop
ockham Sep 30, 2024
0b563b0
Optimize a bit more
ockham Sep 30, 2024
05db29b
Remove unnecessary function args from filter
ockham Sep 30, 2024
a3d11b9
Move single_instance_blocks_present_in_content into filter, make static
ockham Sep 30, 2024
95c4140
Typo
ockham Sep 30, 2024
3bf035b
Rephrase comment
ockham Sep 30, 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
72 changes: 70 additions & 2 deletions src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1056,9 +1056,77 @@ function apply_block_hooks_to_content( $content, $context, $callback = 'insert_h
$after_block_visitor = make_after_block_visitor( $hooked_blocks, $context, $callback );
}

$blocks = parse_blocks( $content );
$block_allows_multiple_instances = array();
/*
* Remove hooked blocks from `$hooked_block_types` if they have `multiple` set to false and
* are already present in `$content`.
*/
foreach ( $hooked_blocks as $anchor_block_type => $relative_positions ) {
foreach ( $relative_positions as $relative_position => $hooked_block_types ) {
foreach ( $hooked_block_types as $index => $hooked_block_type ) {
$hooked_block_type_definition =
WP_Block_Type_Registry::get_instance()->get_registered( $hooked_block_type );

$block_allows_multiple_instances[ $hooked_block_type ] =
block_has_support( $hooked_block_type_definition, 'multiple', true );

if (
! $block_allows_multiple_instances[ $hooked_block_type ] &&
has_block( $hooked_block_type, $content )
) {
unset( $hooked_blocks[ $anchor_block_type ][ $relative_position ][ $index ] );
}
}
if ( empty( $hooked_blocks[ $anchor_block_type ][ $relative_position ] ) ) {
unset( $hooked_blocks[ $anchor_block_type ][ $relative_position ] );
}
}
if ( empty( $hooked_blocks[ $anchor_block_type ] ) ) {
unset( $hooked_blocks[ $anchor_block_type ] );
}
}

/*
* We also need to cover the case where the hooked block is not present in
* `$content` at first and we're allowed to insert it once -- but not again.
*/
$suppress_single_instance_blocks = static function ( $hooked_block_types ) use ( &$block_allows_multiple_instances, $content ) {
static $single_instance_blocks_present_in_content = array();
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 the only place where I'm not entirely sure what are the consequences of using the static variable. However my understanding is that $suppress_single_instance_blocks should be instantiated once per apply_block_hooks_to_content call, so all the checks are applied in a specific context - a template, template part, pattern, post content or navigation.

Copy link
Member

Choose a reason for hiding this comment

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

In my testing, it works correctly as explained in #7443 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, IIUC, we're instantiating a new function here every time apply_block_hooks_to_content() is called, and for the lifetime of that $suppress_single_instance_blocks, we're using $single_instance_blocks_present_in_content as a cache. The next call to apply_block_hooks_to_content() then creates a new $suppress_single_instance_blocks, which in turn creates a new (empty) $single_instance_blocks_present_in_content.

(I added a var_dump( $single_instance_blocks_present_in_content ); on the line below this one and ran tests to verify this 😅 )

foreach ( $hooked_block_types as $index => $hooked_block_type ) {
if ( ! isset( $block_allows_multiple_instances[ $hooked_block_type ] ) ) {
$hooked_block_type_definition =
WP_Block_Type_Registry::get_instance()->get_registered( $hooked_block_type );

$block_allows_multiple_instances[ $hooked_block_type ] =
block_has_support( $hooked_block_type_definition, 'multiple', true );
}

return traverse_and_serialize_blocks( $blocks, $before_block_visitor, $after_block_visitor );
if ( $block_allows_multiple_instances[ $hooked_block_type ] ) {
continue;
}

// The block doesn't allow multiple instances, so we need to check if it's already present.
if (
in_array( $hooked_block_type, $single_instance_blocks_present_in_content, true ) ||
has_block( $hooked_block_type, $content )
) {
unset( $hooked_block_types[ $index ] );
} else {
// We can insert the block once, but need to remember not to insert it again.
$single_instance_blocks_present_in_content[] = $hooked_block_type;
}
}
return $hooked_block_types;
};
add_filter( 'hooked_block_types', $suppress_single_instance_blocks, PHP_INT_MAX );
$content = traverse_and_serialize_blocks(
parse_blocks( $content ),
$before_block_visitor,
$after_block_visitor
);
remove_filter( 'hooked_block_types', $suppress_single_instance_blocks, PHP_INT_MAX );

return $content;
}

/**
Expand Down
101 changes: 101 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 @@ -27,6 +27,27 @@ public static function wpSetUpBeforeClass() {
),
)
);

register_block_type(
'tests/hooked-block-with-multiple-false',
array(
'block_hooks' => array(
'tests/other-anchor-block' => 'after',
),
'supports' => array(
'multiple' => false,
),
)
);

register_block_type(
'tests/dynamically-hooked-block-with-multiple-false',
array(
'supports' => array(
'multiple' => false,
),
)
);
}

/**
Expand All @@ -38,6 +59,8 @@ public static function wpTearDownAfterClass() {
$registry = WP_Block_Type_Registry::get_instance();

$registry->unregister( 'tests/hooked-block' );
$registry->unregister( 'tests/hooked-block-with-multiple-false' );
$registry->unregister( 'tests/dynamically-hooked-block-with-multiple-false' );
}

/**
Expand Down Expand Up @@ -67,4 +90,82 @@ public function test_apply_block_hooks_to_content_inserts_hooked_block() {
$actual
);
}

/**
* @ticket 61902
*/
public function test_apply_block_hooks_to_content_respect_multiple_false() {
$context = new WP_Block_Template();
$context->content = '<!-- wp:tests/hooked-block-with-multiple-false /--><!-- wp:tests/other-anchor-block /-->';

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

/**
* @ticket 61902
*/
public function test_apply_block_hooks_to_content_respect_multiple_false_after_inserting_once() {
$context = new WP_Block_Template();
$context->content = '<!-- wp:tests/other-anchor-block /--><!-- wp:tests/other-block /--><!-- wp:tests/other-anchor-block /-->';

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

/**
* @ticket 61902
*/
public function test_apply_block_hooks_to_content_respect_multiple_false_with_filter() {
$filter = function ( $hooked_block_types, $relative_position, $anchor_block_type ) {
if ( 'tests/yet-another-anchor-block' === $anchor_block_type && 'after' === $relative_position ) {
$hooked_block_types[] = 'tests/dynamically-hooked-block-with-multiple-false';
}

return $hooked_block_types;
};

$context = new WP_Block_Template();
$context->content = '<!-- wp:tests/dynamically-hooked-block-with-multiple-false /--><!-- wp:tests/yet-another-anchor-block /-->';

add_filter( 'hooked_block_types', $filter, 10, 3 );
$actual = apply_block_hooks_to_content( $context->content, $context, 'insert_hooked_blocks' );
remove_filter( 'hooked_block_types', $filter, 10 );

$this->assertSame(
'<!-- wp:tests/dynamically-hooked-block-with-multiple-false /--><!-- wp:tests/yet-another-anchor-block /-->',
$actual
);
}

/**
* @ticket 61902
*/
public function test_apply_block_hooks_to_content_respect_multiple_false_after_inserting_once_with_filter() {
$filter = function ( $hooked_block_types, $relative_position, $anchor_block_type ) {
if ( 'tests/yet-another-anchor-block' === $anchor_block_type && 'after' === $relative_position ) {
$hooked_block_types[] = 'tests/dynamically-hooked-block-with-multiple-false';
}

return $hooked_block_types;
};

$context = new WP_Block_Template();
$context->content = '<!-- wp:tests/yet-another-anchor-block /--><!-- wp:tests/other-block /--><!-- wp:tests/yet-another-anchor-block /-->';

add_filter( 'hooked_block_types', $filter, 10, 3 );
$actual = apply_block_hooks_to_content( $context->content, $context, 'insert_hooked_blocks' );
remove_filter( 'hooked_block_types', $filter, 10 );

$this->assertSame(
'<!-- wp:tests/yet-another-anchor-block /--><!-- wp:tests/dynamically-hooked-block-with-multiple-false /--><!-- wp:tests/other-block /--><!-- wp:tests/yet-another-anchor-block /-->',
$actual
);
}
}
Loading