Navigation Block: block wrapper class name applied to both outer element and its child element

[talldan]

Description

If you inspect the markup of a navigation block with the overlay turned off (makes it easier to reproduce), the wp-block-navigation class name is applied twice, once to the outer <nav> and once to its child <ul>:

<nav class="wp-block-navigation is-layout-flex wp-block-navigation-is-layout-flex" aria-label="Navigation 2">
<ul class="wp-block-navigation__container  wp-block-navigation">

Blocks should only have the wrapper classname on the outermost element.

The bug seems to be caused by get_block_wrapper_attributes being used to compute the attributes of the <ul> element: https://github.com/WordPress/gutenberg/blob/49070907e6d0bdc19e47628e12b0754cd7ddcb6b/packages/block-library/src/navigation/index.php#L177-L182

A concern is that changing this might cause some issues, it'd require a dev note as it may break some styles, but I think it needs to be fixed to ensure themes don't end up with unsual style rules like ul.wp-block-navigation.

Step-by-step reproduction instructions

1. Add a naviation block
2. Turn off the overlay
3. Preview the frontend
4. Inspect the markup

Screenshots, screen recording, code snippet

No response

Environment info

Reproduced on latest trunk core/gutenberg

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]

@talldan Good spot. It seems clear to me that as the <ul> is not the "wrapper" it should not be using get_block_wrapper_attributes. I wish it was easier to track down why this happened. It seems like it could just have been introduced accidentally, but it is unfortunate.

[draganescu]

I've looked all the way back and it seems get_block_wrapper_attributes has always been used like this in the navigation block and - according to how the generated class name was a block support preceding the navigation block - it has always had the classname twice: once for the wrapper and once for the list element.

From my digging it seems it has not been introduced, it simply was there like this all the time.

[getdave]

What's the action step here? Remove from the <ul> and review any impact?

[draganescu]

🤷🏻 block like these are complicated, but Ideally yes @getdave we should see to only gave it appear once in the outermost wrapper like "normal" blocks have.