Unexpected output from apply_block_hooks_to_content_from_post_object

[obenland]

When there are blocks hooked into 'core/post-content' => 'after', it creates unbalanced tags for remove_serialized_parent_block() in apply_block_hooks_to_content_from_post_object():

<!-- wp:post-content --><!-- wp:heading {"level":1} --><h1>Hello World!</h1><!-- /wp:heading --><!-- /wp:post-content --><!-- wp:tests/hooked-block /-->

The result I'm seeing is the hooked block being removed and leaving the closing wp:post-content tag:

<!-- wp:heading {"level":1} --><h1>Hello World!</h1><!-- /wp:heading --><!-- /wp:post-content -->

I'll attach a diff with failing tests to work against.

[obenland]

Diff to make tests fail

This applies https://core.trac.wordpress.org/attachment/ticket/63287/post-content-after.diff in order to see the test results

[jorbin]

Welcome back to trac @obenland!

I moved the patch to GH so we can all see the results of the automated tests.

[edent]

This also happens on non-block themes as well.

At the end of every post is <!-- /wp:post-content -->

I'm using the classic editor with a theme which doesn't use blocks.

[Bernhard Reiter]

I'll look into this today!

[Bernhard Reiter]

Right. So the crux is that the block wrapper is needed temporarily to allow for first/last child insertion into the Post Content block, as it translates between the post's ignoredHookedBlocks meta (i.e. reads from post meta and sets the corresponding block attribute on the wrapper block, and writes back to post meta based on the latter upon writing).

It's a different story for before/after insertion: If a block is inserted before or after the wrapper, it'll confuse remove_serialized_parent_block(), as @obenland pointed out.

I'll need to give this a bit more thought to figure out the correct solution. My intuition says that before/after insertion should be handled one layer higher (e.g. by the template that includes the Post Content block). In particular, I don't think that a hooked block inserted before/after a Post Content block should be stored inside its corresponding post object content. (Instead, it should probably end up next to the Post Content block, inside the containing markup.)

There are however some subtleties that we'll need to think through. For one, it raises the question where to store ignoredHookedBlocks: Continue to use the post meta, or use the containing markup's post content block delimiter?

It will probably also mean that we'll lose the ability to insert before or after post content in a non-block theme, but that's probably okay (as we wouldn't be dealing with a Post Content block strictly speaking, so there's no anchor block before or after which we could insert our hooked block).

[Bernhard Reiter]

This might be pesky enough of an issue to warrant a fix in 6.8.1.

WIP.

Needs additional test coverage:

• Hooked block inserted before/after Post Content block via hooked_block_types filter.
• Hooked block inserted in before/after position _inside_ of Post Content block.
• Hooked block inserted before/after Post Content block contained in other context (e.g. inside a template).

Props @obenland @aaronjorbin @gziolo.

Trac ticket: https://core.trac.wordpress.org/ticket/63287

Thank you @aaronjorbin and @obenland!

I've filed ​https://github.com/WordPress/wordpress-develop/pull/8700 to work on a fix.

We should probably check what happens if the user modifies a template in the Site Editor that contains a Post Content block that has a block hooked before or after it.

There are some reported coding style issues. Otherwise, all tests pass including newly added coverage for the advanced use case of using hooked_block_types filter. Nice work!

[Bernhard Reiter]

In 60173:

Block Hooks: Suppress insertion next to post content wrapper block.

As of [59523], Block Hooks are applied to post content. In order to allow for insertion of a hooked block as first_child or last_child of the containing Post Content block, we wrap the block's post content (as obtained from the DB) in a temporary <!-- wp:post-content --> wrapper block, apply the Block Hooks algorithm to the resulting markup, and remove the wrapper block. (The same technique is applied for the Synced Pattern block -- see [59543] -- as well as the Navigation block.)

However, this caused a problem when a hooked block was marked for insertion before before or after a Post Content block: The logic that's supposed to remove the temporary wrapper block after the Block Hooks algorithm runs erroneously removed that hooked block's delimiter instead of the wrapper block, producing garbled markup as a result.

This changeset fixes the issue by adding a hooked_block_types filter (with PHP_INT_MAX priority) that removes any blocks hooked before or after a Post Content block, if the current context is a post object. This prevents any blocks hooked that way from being "absorbed" into the corresponding post object's content; it retains the ability to hook blocks before and after a Post Content block in any other context (e.g. a template). (The same principle is applied to Synced Pattern and Navigation blocks.)

Props obenland, jorbin, gziolo, bernhard-reiter.
Fixes #63287.

[Bernhard Reiter]

Re-opening to seek a second Core Committer's approval for backporting to the 6.8 branch.

[edent]

This also fixes the issue for non-block themes. Thank you!

[jorbin]

In 60189:

Block Hooks: Suppress insertion next to post content wrapper block.

As of [59523], Block Hooks are applied to post content. In order to allow for insertion of a hooked block as first_child or last_child of the containing Post Content block, we wrap the block's post content (as obtained from the DB) in a temporary <!-- wp:post-content --> wrapper block, apply the Block Hooks algorithm to the resulting markup, and remove the wrapper block. (The same technique is applied for the Synced Pattern block -- see [59543] -- as well as the Navigation block.)

However, this caused a problem when a hooked block was marked for insertion before before or after a Post Content block: The logic that's supposed to remove the temporary wrapper block after the Block Hooks algorithm runs erroneously removed that hooked block's delimiter instead of the wrapper block, producing garbled markup as a result.

This changeset fixes the issue by adding a hooked_block_types filter (with PHP_INT_MAX priority) that removes any blocks hooked before or after a Post Content block, if the current context is a post object. This prevents any blocks hooked that way from being "absorbed" into the corresponding post object's content; it retains the ability to hook blocks before and after a Post Content block in any other context (e.g. a template). (The same principle is applied to Synced Pattern and Navigation blocks.)

Reviewed by Jorbin.
Merges [60173] to 6.8 branch.

Props obenland, jorbin, gziolo, bernhard-reiter.
Fixes #63287.