Make WordPress Core

Opened 5 weeks ago

Last modified 34 hours ago

#64603 new defect (bug)

get_block_wrapper_attributes: id and aria-label should not be merged

Reported by: wildworks's profile wildworks Owned by:
Milestone: 7.0 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by wildworks)

Originally reported in https://github.com/WordPress/gutenberg/issues/75251

props to @mamaduka, @adrock42.

If a dynamic block supports id or aria-label, those attributes are automatically injected into the block via the get_block_wrapper_attributes function. However, if the user explicitly passes them, they are merged, resulting in a space between them. This is not the correct format, especially for id.

Perhaps the id and aria-label attributes should respect explicit user-added values ​​instead of being merged.

In particular, the anchor support (id) added to all dynamic blocks in the 6.9 release may affect many consumers, so let's fix that in the 7.0 release.

Change History (6)

#1 @wildworks
5 weeks ago

  • Description modified (diff)

This ticket was mentioned in PR #10877 on WordPress/wordpress-develop by @wildworks.


5 weeks ago
#2

  • Keywords has-patch has-unit-tests added

@wildworks commented on PR #10877:


4 weeks ago
#4

Oh, I re-forked the wordpress-develop repo and it seems all my submitted open PRs were closed 😂 I submitted a new PR that is equivalent to this one. https://github.com/WordPress/wordpress-develop/pull/10922

@wildworks commented on PR #10922:


3 days ago
#5

Sorry for the late reply.

After some consideration, I decided to refactor the get_block_wrapper_attributes() function overall to make it more robust and secure.

  • Define callback functions that define how to merge or override each attribute.
  • Sanitize style and class attribute values
  • More flexibly handle redundant semicolons in style attributes
  • Added a unit test to directly assert the return value of get_block_wrapper_attributes().

#6 @r1k0
44 hours ago

Patch Testing Report

Patch Tested: https://github.com/WordPress/wordpress-develop/pull/10922

Environment

  • WordPress: 7.0-beta4-61919-src
  • PHP: 8.3.30
  • Server: nginx/1.29.5
  • Database: mysqli (Server: 8.4.8 / Client: mysqlnd 8.3.30)
  • Browser: Chrome 145.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.4
  • MU Plugins: None activated
  • Plugins:
    • Block Development Examples - Block Dynamic Rendering 64756b 0.1.0
    • Test Reports 1.2.1

Steps taken

  1. Download this dynamic block plugin https://github.com/WordPress/block-development-examples/releases/download/latest/block-dynamic-rendering-64756b.zip from WordPress block-development-examples.
  2. Upload the plugin and activate it.
  3. Head over to the plugin files, and add this code to the build/block.json under the "example": {},
      "attributes": {
              "anchor": {
                      "type": "string"
              }
      },
    
  4. In the same file, add "anchor": true inside the support object then save the file.
  5. Edit the build/render.php file and add this code just above the <p> tag
    <?php
    $block_wrapper_attributes = get_block_wrapper_attributes( [
            'id'         => $attributes[ 'anchor' ],
    ] );
    
    ?>
    
  6. In the same file, remove get_block_wrapper_attributes() in the <p> tag and add $block_wrapper_attributes. In the end it should look like this, then save the file.
    <p <?php echo wp_kses_data( $block_wrapper_attributes  ); ?>>
            <?php esc_html_e( 'Block with Dynamic Rendering – hello!!!', 'block-development-examples' ); ?>
    </p>
    
  7. Head over to your dashboard, and add a new post.
  8. In the editor, add the Block Dynamic Rendering block.
  9. Add my-anchor in HTML anchor input field, save the post and view the post in the frontend.
  10. Inspect the paragraph added by the block.
  11. ✅ Patch is solving the problem

Expected result

  • The id attribute now holds one value and not two (an original and a duplicate).

Additional Notes

  • I wasn't able to get merged values when it came to aria-label.

Screenshots/Screencast with results

Before:
https://i.ibb.co/zhm5QmBH/attr-before.png

After:
https://i.ibb.co/MxQCmX66/attr-after.png

Last edited 34 hours ago by r1k0 (previous) (diff)
Note: See TracTickets for help on using tickets.