`render_block_context` filter works differently on top-level vs. inner blocks

[dlh]

The render_block_context filter has a different effect depending on whether the block being rendered is a top-level block or an inner block. When the block is a top-level block, then context provided with render_block_context is made available as context to its inner blocks. When the block is an inner block, the context provided with the filter isn't made available to its inner blocks.

By "top-level block," I mean a single block that's rendered with render_block(), such as within do_blocks(). By "inner block," I mean an inner block rendered within WP_Block::render().

I think this difference in behavior can be traced back to when the filter is applied.

The logic to make context available to inner blocks occurs within the WP_Block constructor. In render_block(), the value returned by the render_block_context filter is passed to the new WP_Block object, so the filtered value is the one made available to inner blocks.

Within WP_Block::render(), the WP_Block object for the inner block has already been instantiated (in WP_Block_List). The value returned by render_block_context is assigned directly to the inner block's $context property, so the logic in the constructor to provide that context to deeper inner blocks isn't invoked.

The linked PR contains a unit test demonstrating this behavior. The test is derived from the real situation in which we observed it: A custom block capable of passing query context to an inner core/post-template block via render_block_context. The post template block received the expected context when the custom block was a top-level block but not when it was an inner block, such as inside a group.

render_block_context was applied to inner blocks in #51612 along with related filters, and there's quite a bit of discussion there about when exactly to apply them, so the logic in place now certainly has reasons behind it.

Still, I would expect the filter to behave the same way with respect to inner blocks regardless of where in the tree the block being filtered is. Between the two options, I would expect the filter to behave like it does in render_block(): Context supplied with a filter is made available to inner blocks.

[gziolo]

Within WP_Block::render(), the WP_Block object for the inner block has already been instantiated (in WP_Block_List). The value returned by render_block_context is assigned directly to the inner block's $context property, so the logic in the constructor to provide that context to deeper inner blocks isn't invoked.

Great findings. I was always unsure whether some of these filters should get applied in the constructor before WP_Block_List is instatiated. I'm wondering whether we would need to apply the filter there, too.

By the way, I added some debug information on GitHub to illustrate better which path is responsible for calling filters depending on the scenario.

[obliviousharmony]

How well timed, I just debugged this same problem lol. I'm working on ​a PR refactoring how we're passing dynamic context in some of our blocks and ran into it. I've spent the better part of my weekend digging through all of the rendering logic for blocks and I've got a possible idea.

I'm sure there are reasons related to how inner blocks initially developed, however, It's a little weird that we make a distinction between a top-level block and an inner-block with respect to rendering. I think we should use render_block() for both inner and top-level blocks so that we can keep all of the hooks together. As for the problem itself, the caveat here is that the available context is only populated on __construct and there's no filtering before that point.

My first thought is to move render_block_data and render_block_context into the constructor but I think this might be a breaking change since it moves these filters to a different stage of the block rendering pipeline. Anyone relying on this being called between pre_render_block and render_block will be in for a shock. In my case, for example, I'm using pre_render_block to set the global post and then a low-priority render_block callback to reset it. While this change doesn't impact me, I'm very close conceptually to changing something that is then expected to be use as context.

My second thought is that, technically, none of these hooks require the block instance to exist. They operate on the $parsed_block array that is then passed to the WP_Block constructor. The caveat here is that WP_Block_List handles our instantiation and it does so using offsetGet. Anyone relying on this behavior before WP_Block::render() instantiates it will still need that functional block instance.

My third thought is that, since WP_Block_List holds onto the available context before the blocks are instantiated, we would need some way to filter it there to alter the context being given to the block.

---

The way that we're instantiating the block essentially forces either a breaking change or new filters. As much as I'd love to move these filters into the constructor, I think they have surrounded the block's render() behavior for too long to be safe to touch. I propose this:

• New Filter: create_block_available_context: This filter goes in __construct() and allows developers to filter the context available to the block.
• New Filter: create_block_data: This filter also goes in __construct() and filters the $parsed_block it is given.
• Add $parent_block as an optional parameter to WP_Block::__construct() so that it can be available to the above filters.
• Move the $postId and $postType injection from render_block into a create_block_available_context hook and inject it if it's not already set.
  • This should be safe because render_block_context would still receive these without any knowledge of where they came from. Even if someone was filtering to change them, since it's set on construction (and they can't currently alter that behavior), their code would continue to work.
  • There's a small breaking change here since right now you can filer this to remove those from the block hierarchy. I don't know why you would do that though and it would probably break everything?

Technically we documented render_block_context as filtering the default context. There's a meaningful distinction between filtering the rendered context and the context provided to a block hierarchically. These changes should allow is to provide dynamic context options while still being able to override the context at render time.

What if we merge the parent context in inner blocks context in WP_Block::render() method before the render_block_context filter? If we did that the parent context is available in inner blocks context.

// Merge parent context.
if ( $parent_block->context ) {
        $inner_block->context = array_merge( $parent_block->context, $inner_block->context );
}

I did quick test in local and it solve the issue and unit test ✅

@mukeshpanchal27, are you talking about the following line when considering changes to the context?

​https://github.com/WordPress/wordpress-develop/blob/a78540b0881a195ec8488bb6bdf878114d75f8b8/src/wp-includes/class-wp-block.php#L496-L497

---

What we miss is that the context is set only for the top-level block just before the class gets initialized:

​https://github.com/WordPress/wordpress-develop/blob/a78540b0881a195ec8488bb6bdf878114d75f8b8/src/wp-includes/blocks.php#L2110-L2114

So the best way to address it would be to, move that to WP_Block::construct, but I have no idea what side-effects that would have.

I did a quick proof of concept, and it passed the newly added test, but there are some changes in two other tests that I need to investigate. The diff:

src/wp-includes/blocks.php

@@ -2087 +2087 @@
                 $context['postType'] = $post->post_type;
         }
 
-        /**
-         * Filters the default context provided to a rendered block.
-         *
-         * @since 5.5.0
-         * @since 5.9.0 The `$parent_block` parameter was added.
-         *
-         * @param array         $context      Default context.
-         * @param array         $parsed_block {
-         *     An associative array of the block being rendered. See WP_Block_Parser_Block.
-         *
-         *     @type string   $blockName    Name of block.
-         *     @type array    $attrs        Attributes from block comment delimiters.
-         *     @type array[]  $innerBlocks  List of inner blocks. An array of arrays that
-         *                                  have the same structure as this one.
-         *     @type string   $innerHTML    HTML from inside block comment delimiters.
-         *     @type array    $innerContent List of string fragments and null markers where
-         *                                  inner blocks were found.
-         * }
-         * @param WP_Block|null $parent_block If this is a nested block, a reference to the parent block.
-         */
-        $context = apply_filters( 'render_block_context', $context, $parsed_block, $parent_block );
+        $block = new WP_Block( $parsed_block, $context, null, $parent_block );
 
-        $block = new WP_Block( $parsed_block, $context );
-
         return $block->render();
 }
 

src/wp-includes/class-wp-block-list.php

@@ -42 +42 @@
         protected $registry;
 
         /**
+         * Reference to the parent block.
+         *
+         * @since 6.7.0
+         * @var WP_Block|null
+         * @access protected
+         */
+        protected $parent_block;
+
+        /**
          * Constructor.
          *
          * Populates object properties from the provided block instance argument.
          *
          * @since 5.5.0
+         * @since 6.7.0 Added the optional `$parent_block` argument.
          *
          * @param array[]|WP_Block[]     $blocks            Array of parsed block data, or block instances.
          * @param array                  $available_context Optional array of ancestry context values.
          * @param WP_Block_Type_Registry $registry          Optional block type registry.
+         * @param WP_Block|null          $parent_block      Optional. Optional. If this is a nested block, a reference to the parent block.
          */
-        public function __construct( $blocks, $available_context = array(), $registry = null ) {
+        public function __construct( $blocks, $available_context = array(), $registry = null, $parent_block = null ) {
                 if ( ! $registry instanceof WP_Block_Type_Registry ) {
                         $registry = WP_Block_Type_Registry::get_instance();
                 }
@@ -60 +71 @@
                 $this->blocks            = $blocks;
                 $this->available_context = $available_context;
                 $this->registry          = $registry;
+                $this->parent_block      = $parent_block;
         }
 
         /**
@@ -93 +105 @@
                 $block = $this->blocks[ $offset ];
 
                 if ( isset( $block ) && is_array( $block ) ) {
-                        $block = new WP_Block( $block, $this->available_context, $this->registry );
+                        $block = new WP_Block( $block, $this->available_context, $this->registry, $this->parent_block );
 
                         $this->blocks[ $offset ] = $block;
                 }

src/wp-includes/class-wp-block.php

@@ -112 +112 @@
          * its registered type will be assigned to the block's `context` property.
          *
          * @since 5.5.0
+         * @since 6.7.0 Added the optional `$parent_block` argument.
          *
          * @param array                  $block             {
          *     An associative array of a single parsed block object. See WP_Block_Parser_Block.
@@ -125 +126 @@
          * }
          * @param array                  $available_context Optional array of ancestry context values.
          * @param WP_Block_Type_Registry $registry          Optional block type registry.
+         * @param WP_Block|null          $parent_block      Optional. If this is a nested block, a reference to the parent block.
          */
-        public function __construct( $block, $available_context = array(), $registry = null ) {
+        public function __construct( $block, $available_context = array(), $registry = null, $parent_block = null ) {
                 $this->parsed_block = $block;
                 $this->name         = $block['blockName'];
 
@@ -138 +140 @@
 
                 $this->block_type = $registry->get_registered( $this->name );
 
-                $this->available_context = $available_context;
+                /**
+                 * Filters the default context provided to a rendered block.
+                 *
+                 * @since 5.5.0
+                 * @since 5.9.0 The `$parent_block` parameter was added.
+                 *
+                 * @param array         $context      Default context.
+                 * @param array         $parsed_block {
+                 *     An associative array of the block being rendered. See WP_Block_Parser_Block.
+                 *
+                 *     @type string   $blockName    Name of block.
+                 *     @type array    $attrs        Attributes from block comment delimiters.
+                 *     @type array[]  $innerBlocks  List of inner blocks. An array of arrays that
+                 *                                  have the same structure as this one.
+                 *     @type string   $innerHTML    HTML from inside block comment delimiters.
+                 *     @type array    $innerContent List of string fragments and null markers where
+                 *                                  inner blocks were found.
+                 * }
+                 * @param WP_Block|null $parent_block If this is a nested block, a reference to the parent block.
+                 */
+                $this->available_context = apply_filters( 'render_block_context', $available_context, $block, $parent_block );
 
                 if ( ! empty( $this->block_type->uses_context ) ) {
                         foreach ( $this->block_type->uses_context as $context_name ) {
@@ -159 +181 @@
                                 }
                         }
 
-                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $registry );
+                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $registry, $parent_block );
                 }
 
                 if ( ! empty( $block['innerHTML'] ) ) {

@gziolo The POC is in right direction.

The Failed unit tests if we apply the POC.

1) Tests_Blocks_RenderReusableCommentTemplate::test_rendering_comment_template_sets_comment_id_context
commentId block context wasn't set correctly.
Failed asserting that 45 is identical to '45'.

/var/www/tests/phpunit/tests/blocks/renderCommentTemplate.php:575
/var/www/vendor/bin/phpunit:122

2) Tests_Blocks_wpBlock::test_block_filters_for_inner_blocks
Failed asserting that 3 is identical to 2.

/var/www/tests/phpunit/tests/blocks/wpBlock.php:799
/var/www/vendor/bin/phpunit:122

The second test needs to check why the filter is called an extra time, the first one needs to fix in Unit test IMO.

### Testing PR. DO NOT REVIEW.

The POC code used from ​https://github.com/WordPress/wordpress-develop/pull/7344#issuecomment-2363536005

If we refactor core code per POC the ancestors context will be available in available_context protected key.

For Column block i add two context. 1) how much column wpp-columns 2) column width wpp-col-width

[available_context:protected] => Array
        (
            [postId] => 6
            [postType] => page
            [wpp-columns] => 1
            [wpp-col-width] => 50%
        )
{{{ 

If we use `get_block_type_uses_context` filter then we get those context in `$block->context` public array key.

}}}php
/*
 * Inspiration taken from current block binding implementation.
 * https://github.com/WordPress/wordpress-develop/blob/6.6/src/wp-includes/class-wp-block-bindings-registry.php#L193-L204
 */
add_filter(
        'get_block_type_uses_context',
        function ( $uses_context, $block_type ) {
                if ( 'core/image' === $block_type->name ) {
                        // Use array_values to reset the array keys.
                        return array_values( array_unique( array_merge( $uses_context, array( 'wpp-columns','wpp-col-width' ) ) ) );
                }
                return $uses_context;
        },
        10,
        2
);
}}}


{{{php
[context] => Array
        (
            [postId] => 6
            [postType] => page
            [wpp-columns] => 1
            [wpp-col-width] => 50%
        )

As was mentioned in a comment in the Trac ticket, moving the render_block_context filter into the constructor would probably be considered backwards-incompatible because it changes the order in which the various block filters run. Additionally, it seems less desirable to me because it reinforces the pattern of having logic in the class constructor, a pattern that's partly responsible for the problem to begin with. Those are counterarguments to the approach, even though it might end up being the best approach anyway.

I wanted to offer another idea, similar to one mentioned in that same Trac comment, which is to use the filtered data to make a new WP_Block instance with all the expected context, then render that. See the diff below.

In terms of backwards compatibility, there are still risks, but I think there might be fewer. As far as I can tell, the only developer assumption that would be violated is that WP_Block_List will always return the exact same WP_Block instance for a given index. Someone could be relying on that assumption, but perhaps that's less likely than a developer relying on filters running in a consistent order.

src/wp-includes/class-wp-block.php

@@ -496 +496 @@
                                                 /** This filter is documented in wp-includes/blocks.php */
                                                 $inner_block->context = apply_filters( 'render_block_context', $inner_block->context, $inner_block->parsed_block, $parent_block );
 
-                                                $block_content .= $inner_block->render();
+                                                $this->inner_blocks[ $index ] = new WP_Block( $inner_block->parsed_block, $inner_block->context, $this->registry );
+
+                                                $block_content .= $this->inner_blocks[ $index ]->render();
                                         }
 
                                         ++$index;

I wanted to offer another idea, similar to one mentioned in that same Trac comment, which is to use the filtered data to make a new WP_Block instance with all the expected context, then render that. See the diff below.

Intriguing alternative. It replicates how the render_block works today, precisely:

​https://github.com/WordPress/wordpress-develop/blob/a78540b0881a195ec8488bb6bdf878114d75f8b8/src/wp-includes/blocks.php#L2110-L2114

Can you apply that change to this branch to ensure it passes all the tests? We can discuss further whether there are some opportunities to avoid recreating the instance of WP_Block when it isn't necessary for some block types.

In terms of backwards compatibility, there are still risks, but I think there might be fewer. As far as I can tell, the only developer assumption that would be violated is that WP_Block_List will always return the exact same WP_Block instance for a given index. Someone could be relying on that assumption, but perhaps that's less likely than a developer relying on filters running in a consistent order.

Could it be mitigated by introducing a new method on WP_Block like update_available_context ? A rough sketch of the idea:

function update_available_context( $context ) {
        $this->context = array();
        $this->available_context = $context;
        if ( ! empty( $this->block_type->uses_context ) ) {
                foreach ( $this->block_type->uses_context as $context_name ) {
                        if ( array_key_exists( $context_name, $this->available_context ) ) {
                                $this->context[ $context_name ] = $this->available_context[ $context_name ];
                        }
                }
        }

        if ( ! empty( $this->parsed_block['innerBlocks'] ) ) {
                $child_context = $this->available_context;

                if ( ! empty( $this->block_type->provides_context ) ) {
                        foreach ( $this->block_type->provides_context as $context_name => $attribute_name ) {
                                if ( array_key_exists( $attribute_name, $this->attributes ) ) {
                                        $child_context[ $context_name ] = $this->attributes[ $attribute_name ];
                                }
                        }
                }

                $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $this->registry );
        }
}

I might be missing some nuances, but it shouldn't differ from the original idea while retaining the existing WP_Block instance. One important consideration is that rendering happens bottom-up, while all objects get initialized in the top-down fashion, so that needs to be further investigated.

Could it be mitigated by introducing a new method on WP_Block like update_available_context ? A rough sketch of the idea:

How it help to solve the issue?

Could it be mitigated by introducing a new method on WP_Block like update_available_context ? A rough sketch of the idea:

How it help to solve the issue?

See the comment from @dlh01 ​https://github.com/WordPress/wordpress-develop/pull/7344#issuecomment-2375599875. It's the same idea with a twist:

src/wp-includes/class-wp-block.php

@@ -496 +496 @@
                                                 /** This filter is documented in wp-includes/blocks.php */
                                                 $inner_block->context = apply_filters( 'render_block_context', $inner_block->context, $inner_block->parsed_block, $parent_block );
 
-                                                $block_content .= $inner_block->render();
+                                                $inner_block->update_available_context( $inner_block->context );
+                                                $block_content .= $inner_block->render();
                                         }
 
                                         ++$index;

Here is the diff with the very quick draft that passes existing and the new tests added in this PR:

src/wp-includes/class-wp-block.php

@@ -138 +138 @@
 
                 $this->block_type = $registry->get_registered( $this->name );
 
+                $this->update_available_context( $block, $available_context );
+
+                if ( ! empty( $block['innerHTML'] ) ) {
+                        $this->inner_html = $block['innerHTML'];
+                }
+
+                if ( ! empty( $block['innerContent'] ) ) {
+                        $this->inner_content = $block['innerContent'];
+                }
+        }
+
+        public function update_available_context( $block, $available_context ) {
                 $this->available_context = $available_context;
 
                 if ( ! empty( $this->block_type->uses_context ) ) {
@@ -158 +170 @@
                                         }
                                 }
                         }
-
-                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $registry );
+                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $this->registry );
                 }
-
-                if ( ! empty( $block['innerHTML'] ) ) {
-                        $this->inner_html = $block['innerHTML'];
-                }
-
-                if ( ! empty( $block['innerContent'] ) ) {
-                        $this->inner_content = $block['innerContent'];
-                }
         }
 
         /**
@@ -505 +508 @@
                                                 /** This filter is documented in wp-includes/blocks.php */
                                                 $inner_block->context = apply_filters( 'render_block_context', $inner_block->context, $inner_block->parsed_block, $parent_block );
 
+                                                $inner_block->update_available_context( $inner_block->parsed_block, $inner_block->context );
                                                 $block_content .= $inner_block->render();
                                         }

@gziolo

Here is the diff with the very quick draft that passes existing and the new tests added in this PR

I like this approach at first glance. I'll plan on giving it a test but am unable to apply that diff (probably something I'm doing wrong).

@gziolo I was able to convert the diff above into an diff that I could apply to my git checkout by copying to a file and running git apply ~/path/to/file.diff (pasting here for others and to double check that I didn't miss anything:

src/wp-includes/class-wp-block.php

@@ -138 +138 @@
 
                 $this->block_type = $registry->get_registered( $this->name );
 
+                $this->update_available_context( $block, $available_context );
+
+                if ( ! empty( $block['innerHTML'] ) ) {
+                        $this->inner_html = $block['innerHTML'];
+                }
+
+                if ( ! empty( $block['innerContent'] ) ) {
+                        $this->inner_content = $block['innerContent'];
+                }
+        }
+
+        public function update_available_context( $block, $available_context ) {
+                $this->context = array();
+
                 $this->available_context = $available_context;
 
                 if ( ! empty( $this->block_type->uses_context ) ) {
@@ -159 +173 @@
                                 }
                         }
 
-                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $registry );
-                }
-
-                if ( ! empty( $block['innerHTML'] ) ) {
-                        $this->inner_html = $block['innerHTML'];
-                }
-
-                if ( ! empty( $block['innerContent'] ) ) {
-                        $this->inner_content = $block['innerContent'];
+                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $this->registry );
                 }
         }
 
@@ -505 +511 @@
                                                 /** This filter is documented in wp-includes/blocks.php */
                                                 $inner_block->context = apply_filters( 'render_block_context', $inner_block->context, $inner_block->parsed_block, $parent_block );
 
+                                                $inner_block->update_available_context( $inner_block->parsed_block, $inner_block->context );
+
                                                 $block_content .= $inner_block->render();
                                         }

Unfortunately, this looks like it is causing page level context to no get passed to innerBlocks, so blocks like the core/post-title block are not rendering correctly with this diff applied. I'm trying to debug it now.

I think I may have figured it out. When calling update_avialable_context() on a block that already has available context, if the new context being passed is empty, the old available context was being overwritten. Instead it should be merged. Here's a new diff:

src/wp-includes/class-wp-block.php

@@ -138 +138 @@
 
                 $this->block_type = $registry->get_registered( $this->name );
 
-                $this->available_context = $available_context;
+                $this->update_available_context( $block, $available_context );
+
+                if ( ! empty( $block['innerHTML'] ) ) {
+                        $this->inner_html = $block['innerHTML'];
+                }
+
+                if ( ! empty( $block['innerContent'] ) ) {
+                        $this->inner_content = $block['innerContent'];
+                }
+        }
+
+        public function update_available_context( $block, $available_context ) {
+                $this->context = array();
+
+                if ( null === $this->available_context ) {
+                        $this->available_context = $available_context;
+                } else {
+                        $this->available_context = array_merge( $this->available_context, $available_context );
+                }
 
                 if ( ! empty( $this->block_type->uses_context ) ) {
                         foreach ( $this->block_type->uses_context as $context_name ) {
@@ -159 +177 @@
                                 }
                         }
 
-                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $registry );
-                }
-
-                if ( ! empty( $block['innerHTML'] ) ) {
-                        $this->inner_html = $block['innerHTML'];
-                }
-
-                if ( ! empty( $block['innerContent'] ) ) {
-                        $this->inner_content = $block['innerContent'];
+                        $this->inner_blocks = new WP_Block_List( $block['innerBlocks'], $child_context, $this->registry );
                 }
         }
 
@@ -505 +515 @@
                                                 /** This filter is documented in wp-includes/blocks.php */
                                                 $inner_block->context = apply_filters( 'render_block_context', $inner_block->context, $inner_block->parsed_block, $parent_block );
 
+                                                $inner_block->update_available_context( $inner_block->parsed_block, $inner_block->context );
+
                                                 $block_content .= $inner_block->render();
                                         }

As I was thinking about the update_available_context() method, it occurred to me that there's a second difference between the two invocations of the render_block_context filter. @ObliviousHarmony's comment in Trac hinted at it, and perhaps it's on your mind as well.

When render_block_context runs for top-level blocks in render_block(), the filtered value becomes the $available_context parameter and is reduced down to the context that the block type uses. When render_block_context runs for inner blocks in WP_Block::render(), the result is applied directly to the $context property, meaning that inner blocks can be given context properties outside of what the block type supports. I've added a test to this branch to demonstrate this difference. I'm not sure, but the difference seems similarly unintentional.

Can you apply that change to this branch to ensure it passes all the tests? We can discuss further whether there are some opportunities to avoid recreating the instance of WP_Block when it isn't necessary for some block types.

Done and pushed to this branch.

One observation regarding the update_available_context() method as drafted here: If I'm reading correctly, it seems to me that the risk of violating developer assumptions is the same as it would be if a new WP_Block was instantiated as happens in 6a37f03b2b. The update_available_context() method would update the $inner_blocks property with a new WP_Block_List instance, and won't that will return new instances of WP_Block anyway?

The update_available_context() method would update the $inner_blocks property with a new WP_Block_List instance, and won't that return new instances of WP_Block anyway?

Yes, that's a good observation. If there are inner blocks, then all of them would have a new instance recreated. So, it is not so much of a win to call the method on the existing instance. Overall, it's a very complex arrangement with all these parsed block.

When render_block_context runs for top-level blocks in render_block(), the filtered value becomes the $available_context parameter and is reduced down to the context that the block type uses. When render_block_context runs for inner blocks in WP_Block::render(), the result is applied directly to the $context property, meaning that inner blocks can be given context properties outside of what the block type supports. I've added a test to this branch to demonstrate this difference. I'm not sure, but the difference seems similarly unintentional.

Overall, my understanding was that the context injected could be outside of what the block supports. Example from the Query block, in particular Post Template block:

​https://github.com/WordPress/wordpress-develop/blob/328284c0865be63b906042be0585a022c4f2f5e1/src/wp-includes/blocks/post-template.php#L111-L124

This block doesn't define providesContext:

​https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/blocks/post-template/block.json

So, this is very concerning if it works differently depending on where the filters get called from. It needs to be further investigated. I hope we can sort out all these differences somehow.

[gziolo]

We missed the date to land it in WordPress 6.7. I will explicitly set 6.8 milestone to ensure we sort it out in the next weeks. I think we are getting close to finding an acceptable solution in the PR from @dlh: ​https://github.com/WordPress/wordpress-develop/pull/7344. More testing is needed to confirm that it won't produce unintended side effects.

[joemcgill]

We missed the date to land it in WordPress 6.7.

@gziolo since this is a bug, I think we could likely still land a fix for this during beta, which would be ideal given that there will likely be a large time gap before 6.8 is released (likely end of Q1, 2025 based on past trends).

Could we possibly update the PR currently in progress to include one of the approaches we've been iterating on so that it's easier to test?

Overall, my understanding was that the context injected could be outside of what the block supports. Example from the Query block, in particular Post Template block:

OK, fair enough. In that case, I think the inconsistency is just the other way around: When the block is a top-level block, it's not possible to provide context outside of what the block supports using render_block_context because the filtered value becomes the $available_context and is reduced down to the context that the block type uses. I've updated test_render_block_context_allowed_context() in this PR to demonstrate this behavior.

(Side note: The example from the Post Template block is extra weird because the blockName is set to core/null just above the highlighted lines. Even if the Post Template block defined providesContext, I don't think it would be referenced because the instantiated WP_Block wouldn't retrieve the Post Template block type.)

It seems that we're dealing with a couple of inconsistencies now: Context supplied with render_block_context isn't always made available to a block's inner blocks, nor is it always applied to the block itself.

Accordingly, I've reverted 6a37f03 from this PR (for now) because it addressed only the first of these. I thought that having an incomplete solution in place might get in the way of stepping back to reconsider the entire situation. Both new tests are failing again, as expected.

[gziolo]

It sounds reasonable to land a bug fix in WordPress 6.7 during the beta cycle if that would delay other efforts. I mentioned that on WordPress Slack, just wanted to quickly repeat that the biggest challenge is to agree on the best way forward to unify handling for this filter.

Testing PR. DO NOT REVIEW.

[mukesh27]

The ​PR #7522 solved the unit tests.

I've updated test_render_block_context_allowed_context() in this PR to demonstrate this behavior.

I just noticed that I forgot to push this update. It's there now. Sorry!

@mukeshpanchal27 Heads up that the tests in ​https://github.com/WordPress/wordpress-develop/pull/7344 have been updated to reflect the last few comments there.

Nice one @mukeshpanchal27. @dlh01 and @joemcgill, it looks like all the unit tests we could think of pass in this branch. How do you feel about the proposed solution? Is it good enough to move forward as part of the WP 6.7 release?

Thanks for the ping! I should be able to take a look at this Wednesday or Thursday.

The PR is ready for review. Address the feedbacks.

I don't have more code comments for now. As mentioned earlier, at this point, we need to confirm that no accidental regressions have been introduced, as this API is used with different assumptions to overcome the preexisting shortcomings.

Closing in favour of #7522

Outstanding comments like ​https://github.com/WordPress/wordpress-develop/pull/7522/files#r1793229135 would be good to address, but fairly minor.

Here we go ​https://github.com/WordPress/wordpress-develop/pull/7522/commits/33930ed2183e891d752f2019db169e2fd22dde98

Joe has accurately summarized my hesitancy about the change to the test as it relates to #7344. Here are a few other thoughts after reading the PR.

First: Both times that update_available_context() is called, the passed parameters are already present on the block instance. For example:

$inner_block->update_available_context( $inner_block->parsed_block, $inner_block->context );

This seems like a bit of a code smell. Why would it be necessary to offer the parameters if the block already has the data?

Second: This is something that was on my mind in #7344, and even now I can't quite put my finger on it, but I keep thinking that there's an inconsistency of some kind between the use of a new public method that's designed to update a block's state and the call to new WP_Block_List() within that method, since new WP_Block_List() will wipe out the previous WP_Block instances that were being used as inner blocks.

If update_available_context() was called on a child block before being called on a parent block, the updated context in the child block would be lost because the inner blocks on the parent would be replaced after new WP_Block_List() in the parent. I'm purely speculating here, but it seems like behavior that unexpectedly might get in the way of future features, or at least surprise a plugin developer who attempts to make use of the new public API. There's an appearance of stability (arguably) that isn't quite there.

Third: I'm not sure that I understand why update_available_context() would also update the inner_html and inner_content class properties. Neither of these properties relates to block context, as far as I can tell. Also, doesn't it make the block state inconsistent to update just those properties with the passed block? For example, the attributes class property, accessed just a few lines earlier, would still reference the parsed_block property, which isn't changed by update_available_context(). Different properties in the same WP_Block would end up referencing different parsed block arrays.

@dlh01

This seems like a bit of a code smell. Why would it be necessary to offer the parameters if the block already has the data?

This is necessary because these two properties are filtered outside of the instance. Since they are modified outside the block instance, it is crucial to pass the modified values back to the instance. Otherwise its state is incorrect, and that partly led to the bug this PR is aiming to address.

Third: I'm not sure that I understand why update_available_context() would also update the inner_html and inner_content class properties. Neither of these properties relates to block context, as far as I can tell.

This relates to my above answer. Both inner_html and inner_content depend on parsed_block, which is being filtered outside the instance. So if parsed_block changes, those changes need to be reflected in inner_html and inner_content.

Maybe there are better solutions than filtering these properties outside of the class instance, it could be worth exploring that. But in its current state, this approach is broken because it leads to outdated property values (in case the parsed block or block context are filtered), and that's fixed by this PR.

This is necessary because these two properties are filtered outside of the instance. Since they are modified outside the block instance, it is crucial to pass the modified values back to the instance. Otherwise its state is incorrect, and that partly led to the bug this PR is aiming to address.

I'm sorry, but I'm not following you. I get that the properties are being filtered outside of the instance, but the filter is being applied directly to the properties. Wouldn't it be possible for update_available_context() to accept no parameters and refer to the properties themselves?

This relates to my above answer. Both inner_html and inner_content depend on parsed_block, which is being filtered outside the instance. So if parsed_block changes, those changes need to be reflected in inner_html and inner_content. Maybe another method name would help. Or we could try having two separate methods, one to update data based on a modified parsed_block, another to update data based on a modified context.

That's a great point, and I hadn't thought about it. Still, it seems like a separate bug than the one raised in the ticket, since it has to do with a different filter (I assume you're referring to render_block_data). So, yes, a separate method or method name makes sense to me.

This is necessary because these two properties are filtered outside of the instance. Since they are modified outside the block instance, it is crucial to pass the modified values back to the instance. Otherwise its state is incorrect, and that partly led to the bug this PR is aiming to address.

I'm sorry, but I'm not following you. I get that the properties are being filtered outside of the instance, but the filter is being applied directly to the properties. Wouldn't it be possible for update_available_context() to accept no parameters and refer to the properties themselves?

Ah I see what you're saying, yes. Probably the method should be parameterless and simply update the other properties based on the current values of the updated properties.