Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix search block html handling for label and button text #38649

Merged
merged 2 commits into from
Feb 9, 2022

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Feb 9, 2022

Description

Fixes #38644

The search block was outputting escaped html in its label and button text, when it should have been outputting the tags.

In this PR I've swapped out esc_html for wp_kses.

Testing Instructions

  1. Add a search block to a post
  2. Use formatting on the label and button text (e.g. bold, italic)
  3. Preview the post

Expected - the formatting should be displayed correctly

Screenshots

Before

Screen Shot 2022-02-09 at 10 01 05 am

After

Screen Shot 2022-02-09 at 10 39 20 am

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@talldan talldan added [Type] Bug An existing feature does not function as intended [Block] Search Affects the Search Block - used to display a search field labels Feb 9, 2022
@talldan talldan requested a review from ajitbohra as a code owner February 9, 2022 02:40
@talldan talldan self-assigned this Feb 9, 2022
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion re globally allowed attributes.

Any reason you're not using wp_kses_post() and the default set of tags? It saves maintaining a separate allow list if the spec gets more tags.

),
'strong' => array(),
);

Copy link
Contributor

@peterwilsoncc peterwilsoncc Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$wp_kses_formatting_tags = array_map( '_wp_add_global_attributes', $wp_kses_formatting_tags );

Will allow advanced users to add classes, IDs and other pretty safe attributes. See

https://github.com/WordPress/wordpress-develop/blob/f745cc551bb0a54d7df72ec97d23e3359c56048d/src/wp-includes/kses.php#L2559-L2571

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've switched it over to wp_kses_post now, and it looks like it implicitly has the global attributes:
https://github.com/WordPress/wordpress-develop/blob/f745cc551bb0a54d7df72ec97d23e3359c56048d/src/wp-includes/kses.php#L736

Thanks for the advice!

@talldan talldan added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 9, 2022
@talldan
Copy link
Contributor Author

talldan commented Feb 9, 2022

Any reason you're not using wp_kses_post() and the default set of tags? It saves maintaining a separate allow list if the spec gets more tags.

I unashamedly copied the Navigation Link block's code. 😄

It looks like that wasn't the best code to copy. I found this discussion about it - #19477 (comment).

I'll make a separate PR to update any other blocks that might be using wp_kses that could be using wp_kses_post.

@talldan talldan merged commit d3eeb30 into trunk Feb 9, 2022
@talldan talldan deleted the fix/search-block-html-handling branch February 9, 2022 05:16
@github-actions github-actions bot added this to the Gutenberg 12.6 milestone Feb 9, 2022
@Mamaduka Mamaduka removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 16, 2022
Mamaduka pushed a commit that referenced this pull request Feb 16, 2022
* Fix search block html handling

* Switch to wp_kses_post
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Search Affects the Search Block - used to display a search field [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search Block: Formatting applied to button text encoded in frontend
4 participants