Custom Navigation blocks, appearing outside the UL on the frontend

[krugazul]

Description

When assigning a custom block to the navigation block, the render_block_core_navigation outputs the custom block outside of the UL, when on the frontend.

Trac Ticket - https://core.trac.wordpress.org/ticket/57992

Source

This array of nav "list items" and "items needing a list wrapper" is defined here

The custom block is included in the $inner_blocks variable. But when it loops through them, the custom block triggers the closing of the <ul> prematurely.

That is because a custom block, fails the list item check here.

Step-by-step reproduction instructions

Before you start, you will need a custom block that can be added to the core/navigation block.

1. Create a navigation block with 3 inner blocks. core/navigation-link, custom_block, core/navigation-link.
2. Save and view the menu on the frontend
3. Use the browser code inspector and select your custom block in the menu
4. You should see two UL elements, for the nav, 1 before your block, and 1 after.

Screenshots, screen recording, code snippet

Environment info

• WordPress Version 6.1.1
• Gutenberg (not installed)

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[getdave]

@draganescu I was wondering if we could use HTML Tag Processor to "detect" presence of <li> in the block content and act accordingly rather than rely on hard coded blocks whitelist.

[draganescu]

Hi @krugazul how is the custom block added as a possible child of navigation?

[getdave]

@draganescu The PR description was missing clear testing instructions as was the Core Trac ticket.

Here is an example from @costdev in WP Slack:

<?php

/**
 * Plugin Name: 1. Test 57992
 * Description: Tests 57992
 * Author:      WordPress Core Contributors
 * Author URI:  https://make.wordpress.org/core
 * License:     GPLv2 or later
 * Version:     1.0.0
 */

add_filter(
	'block_core_navigation_render_inner_blocks',
	function( $inner_blocks ) {
		$inner_blocks[] = new My_Block( array( 'blockName' => 'costdev/my-block' ) );
		return $inner_blocks;
	},
	999
);

class My_Block extends WP_Block {
	public function render( $options = array() ) {
		return 'My Block!';
	}
}

[draganescu]

This seems a bit weird because it actually shouldn't work so it's expected that it doesn't work :)

There is a list of allowed blocks for a reason, so if the block cannot be added in the editor it is not expected when rendering. The renderer should only meet blocks that can be added by the user. This is how it is today. If the allowed blocks is something that can be made extendable, that is something to address then.

I think not only the navigation block but all blocks that have an allowed list of inner blocks should not be forced to render things outside of that list and behave correctly.

[getdave]

@draganescu Do we need to improve the docs around block_core_navigation_render_inner_blocks then to state that it is not designed for adding new blocks not in the allowed list?

[draganescu]

Maybe 🤷🏻 - I don't think it's specific to the navigation block, it's basically an undocumented assumption. I have proposed in the past cc @gziolo that we bubble up allowed blocks into block.json so that it's obvious which blocks "should" be allowed, and maybe document this limitation there?

Either way hacking block content at render time is something we've hit in the past as well - I remember we removed recursively added navigation blocks and that broke the parser :)

[gziolo]

Maybe 🤷🏻 - I don't think it's specific to the navigation block, it's basically an undocumented assumption. I have proposed in the past cc @gziolo that we bubble up allowed blocks into block.json so that it's obvious which blocks "should" be allowed, and maybe document this limitation there?

As far as I remember, we landed #37998 that added the block_core_navigation_render_inner_blocks hook to resolve the extensibility issue for the Navigation block. However, it was clear that because the approach used is based on WP hooks, it can't be generalized for every possible block or visualized in the editor. Let's discuss potential solutions in #39439.