Style Engine: add condition to check for semicolons before adding them to CSS rules #41028
Conversation
| @@ -242,7 +242,8 @@ public function generate( $block_styles ) { | |||
| foreach ( $css_rules as $rule => $value ) { | |||
| $filtered_css = esc_html( safecss_filter_attr( "{$rule}: {$value}" ) ); | |||
| if ( ! empty( $filtered_css ) ) { | |||
| $css_output .= $filtered_css . '; '; | |||
| $css_output_suffix = str_ends_with( $filtered_css, ';' ) ? ' ' : '; '; | |||
There was a problem hiding this comment.
Thanks for putting this together @chad1008 🙇
I'm not convinced this change is required on core.
Block supports, even before the style engine, has been adding ; to inline styles.
See this line here: 6de1698#diff-d61feaa89a4d6e58f54a8f07a04644ea493ef9e203f41a284236abe28a1bd899R86
Furthermore, in between moving from creating inlines styles in block supports files and creating them here in the style engine, the core tests weren't modified and continued to pass.
🤔
It's a curious one though. I would love to know if this is reproducible on WordPress Core (with no plugins/CSS processing).
So far I cannot reproduce it.
Are there any steps to reproduce using WordPress + Gutenberg? So far I can't. |
|
After chatting about this a bit elsewhere, because this only happens in specific cases and specific parsers (and can't be repro'd without any plugins or CSS processing) I agree we don't need a change here. Closing the draft, but thanks for taking a look! 😄 |
related to: #40332
What?
Confirm that a CSS rule does not already end in a semicolon before adding one in the style engine
Why?
Depending on the what CSS parsers are in use on a site, it's possible semicolons will already be present on inline CSS styles, which can result in double semicolons (
attribute1: value1;; attribute2: value2;;) rendering when the page loads.How?
After confirming that styles are present, the rule suffix is conditionally set:
Note: this does not account for parsers that might also add a space character of their own. It may be worth extending the logic for that possibility as well, but I'll wait on feedback there first.
Testing Instructions
Screenshots or screencast