Make WordPress Core

Opened 4 weeks ago

Last modified 5 days ago

#61430 accepted enhancement

Adding escaping function in wp_interactivity_data_wp_context PHPDoc

Reported by: mosne's profile mosne Owned by: audrasjb's profile audrasjb
Milestone: 6.6.1 Priority: normal
Severity: minor Version: 6.5
Component: Interactivity API Keywords: good-first-bug has-patch
Focuses: docs Cc:

Description

in the /wp-includes/interactivity-api/interactivity-api.php the following example is given in the PHPDoc:

<div <?php echo wp_interactivity_data_wp_context( array( 'isOpen' => true, 'count' => 0 ) ); ?>>

We should always follow best practices, so I propose adding proper escaping:

<div <?php echo wwp_kses_data( wp_interactivity_data_wp_context( array( 'isOpen' => true, 'count' => 0  ) ) ); ?>>

Change History (12)

#1 @mosne
4 weeks ago

P.S. In case the wp_interactivity_data_wp_context function is truly safe, we could add it to the coding standard exceptions.

#2 @audrasjb
4 weeks ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 6.6

As this is a Docs related enhancement, let's try milestoning it to 6.6.

#3 @audrasjb
4 weeks ago

  • Owner set to audrasjb
  • Status changed from new to accepted

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


4 weeks ago
#4

  • Keywords has-patch added; needs-patch removed

…ata_wp_context

Trac ticket: #61430

#5 @Rahe
4 weeks ago

Seems good to me :)

#6 follow-up: @jonsurrell
4 weeks ago

I think this function should be added as an exception. It returns an HTML attribute name and value escaped and ready to print. It doesn't seem like there's anything for kses to do here.

From wp_kses_data (emphasis mine):

$data string required
Content to filter, expected to not be escaped.


I did notice that the description is wrong here for wp_interactivity_data_wp_context:

This helper function simplifies the creation of data-wp-context directives by providing a way to pass an array of data, which encodes into a JSON string safe for direct use as a HTML attribute value.

Although the return description does hint that it returns the full attribute "a complete … directive":

Return
string A complete data-wp-context directive with a JSON encoded value representing the context array and the store namespace if specified.

It seems like that could be improved to clarify that it returns and HTML attribute suitable to be output to the page.

#7 in reply to: ↑ 6 @audrasjb
4 weeks ago

Replying to jonsurrell:

I think this function should be added as an exception. It returns an HTML attribute name and value escaped and ready to print. It doesn't seem like there's anything for kses to do here.

From wp_kses_data (emphasis mine):

$data string required
Content to filter, expected to not be escaped.

By the way, it's worth noting that the contributor who filled this ticket opened it after the plugin review team asked him to escape the result of this function.

That's why it seems like it made sense to enforce escaping in the example provided.

I did notice that the description is wrong here for wp_interactivity_data_wp_context:

This helper function simplifies the creation of data-wp-context directives by providing a way to pass an array of data, which encodes into a JSON string safe for direct use as a HTML attribute value.

Although the return description does hint that it returns the full attribute "a complete … directive":

Return
string A complete data-wp-context directive with a JSON encoded value representing the context array and the store namespace if specified.

It seems like that could be improved to clarify that it returns and HTML attribute suitable to be output to the page.

Great point!

@mukesh27 commented on PR #6819:


3 weeks ago
#9

Instead of adding escape function we should update docblock check comment here

This ticket was mentioned in Slack in #core by nhrrob. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


5 days ago

#12 @audrasjb
5 days ago

  • Milestone changed from 6.6 to 6.6.1

Moving to 6.6.1.

Note: See TracTickets for help on using tickets.