Custom CSS: Decode HTML entities in nested selectors#75654
Custom CSS: Decode HTML entities in nested selectors#75654glendaviesnz wants to merge 1 commit intotrunkfrom
Conversation
Block serialisation may encode `&` as `&` in the stored CSS attribute, which breaks nested CSS selectors (e.g. `&:hover`). This decodes HTML entities before processing so that `process_blocks_custom_css()` handles nested selectors correctly.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
glendaviesnz
added
the
[Type] Bug
|
Flaky tests detected in 9fca8aa. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22124473321
|
|
I've tested with a bunch of selectors, e.g &::before { content: "test"; }
&::after { display: block; }
&::placeholder { color: gray; }
&:focus { outline: 2px solid blue; }
&:active { opacity: 0.8; }
&:focus-visible { outline: 3px solid; }
&:not(.disabled) { cursor: pointer; }
&:is(h1, h2, h3) { font-weight: bold; }
&:has(> p) { display: flex; color:yellow;}
&:nth-child(1n) { background: #eee; }
&.wp-block-group { background: blue; } /* appended class */
&[class=*] {color:red; } /* attribute selector */
&[class*="wp-block-group"] { color: red; }And things look fine in the editor and frontend. The frontend handles the following well: &::before { content: "©"; }
The editor not so much. I don't think this is strictly related to this change. I might be thinking to far into this, but also wondering if we need to support the following: &::before { content: "©"; }
I don't want to distract, this is an improvement on the is state. |
ramonjd
added
the
CSS Styling
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
Thanks for the fix @glendaviesnz
I think this change is fine in itself to address the bug, but I would like a confidence check from folks who have been thinking about string sanitization. E.g., WordPress/wordpress-develop#10641
It might not make Beta 1, but we can flag it for backporting to a RC
| // Decode HTML entities (e.g. & → &) that may be introduced | ||
| // during block serialisation so that nested selectors using & | ||
| // are processed correctly by process_blocks_custom_css(). | ||
| $custom_css = html_entity_decode( $custom_css, ENT_QUOTES | ENT_HTML5, 'UTF-8' ); |
There was a problem hiding this comment.
Ok, it looks like the defaul encoding is actuall \u0026 so it seems like it is something in our CSS sanitisation pipeline that may be causing this, so we may be able to apply a local fix for this, so switching to draft until I have clarified that.
There was a problem hiding this comment.
Thank you for pinging me on this. I'm not sure what the resolution was, but it seems like this change was not needed after all?
There was a problem hiding this comment.
but it seems like this change was not needed after all?
Seems like it, and thanks for getting back to us. Did you resolve it in the end @glendaviesnz?
| } | ||
|
|
||
| // Validate CSS doesn't contain HTML markup (same validation as global styles REST API). | ||
| if ( preg_match( '#</?\w+#', $custom_css ) ) { |
There was a problem hiding this comment.
The fix now causes <script> to be decoded and then rejected by this HTML check. Maybe this behavior is worth asserting explicitly. If the order of operations were ever accidentally swapped, this test would catch it.
public function test_custom_css_rejects_html_entity_encoded_tags() {
// Before this fix, <script> would bypass the HTML check
// because no literal '<' existed. Now decode runs first.
$parsed_block = array(
'blockName' => 'test/custom-css-encoded-amp',
'attrs' => array( 'style' => array( 'css' => '<script>alert(1)</script>' ) ),
);
$result = gutenberg_render_custom_css_support_styles( $parsed_block );
$this->assertArrayNotHasKey( 'className', $result['attrs'],
'HTML-entity-encoded script tags must be rejected after decoding.' );
}
|
Exploring a local fix for this it my be caused by a custom sanitization pipeline - so switched to draft while i clarify that |
3 participants
Summary
&as&in the stored custom CSS attribute, breaking nested CSS selectors like&:hoverand& .childhtml_entity_decode()before CSS processing soprocess_blocks_custom_css()handles nested selectors correctlyTest plan
&:hover,& .child-class) render correctly on the front end