Improve image block lightbox escaping #51061
Conversation
juanfra
added
[Type] Enhancement
juanfra
changed the title
|
Flaky tests detected in 5f49779. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5113426255
|
There was a problem hiding this comment.
@juanfra You are escaping the code inside the variable when you probably need to escape where the variable is returned in HTML. I recommend changing that. For example:
$some_var = __( 'Testing', 'some-text-domain' );
echo '<p>' . esc_html( $some_var ) . '</p>';
|
@alexstine, thanks for the review! Yes, I'm sanitizing at that point (as late as possible) because of the heredoc |
|
@juanfra Why was the choice made here to use heredoc? Seems like for no more than what this outputs, a simple $output = '';
$output .= '<p>' . esc_html( $escaped_here ) . '</p>';
echo $output;I am fine with approving this but kind of curious as to why this pattern was used. Thanks |
|
@alexstine I am not entirely sure, I went through the PR where the original functionality was merged and I couldn't find any reference or discussion to the usage of heredoc for the return. |
|
@juanfra @alexstine Thanks for looking at this! I used the heredoc because it seemed to me that was the best way to clearly express the HTML structure and allow others to understand it at a glance, especially with all of the interactivity API directives. |
|
I just never could recall us using heredoc in Core but I guess it likely is. Pretty large codebase so it is very possible this is just the first time I've seen it. |
* Improve the image block lightbox escaping.
What?
Improve the image block lightbox escaping.
Why?
The functionality can be broken by using some special characters in the image alt.
Example:
Screen.Recording.2023-05-29.at.17.34.19.mov
How?
Escaping all attributes.
Testing Instructions
<html>in the alt text.