Skip to content

Style Engine: add condition to check for semicolons before adding them to CSS rules#41028

Closed
chad1008 wants to merge 1 commit into
trunkfrom
fix/style-engine-check-for-existing-semicolon
Closed

Style Engine: add condition to check for semicolons before adding them to CSS rules#41028
chad1008 wants to merge 1 commit into
trunkfrom
fix/style-engine-check-for-existing-semicolon

Conversation

@chad1008

Copy link
Copy Markdown
Contributor

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:

  • if the rule already ends in a semicolon, append a space character only
  • if the rule does not yet end in a semicolon, append a semicolon followed by a space character
    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

$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, ';' ) ? ' ' : '; ';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@ramonjd

ramonjd commented May 12, 2022

Copy link
Copy Markdown
Member

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.

Are there any steps to reproduce using WordPress + Gutenberg? So far I can't.

@chad1008

Copy link
Copy Markdown
Contributor Author

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! 😄

@chad1008 chad1008 closed this May 16, 2022
@johnbillion johnbillion deleted the fix/style-engine-check-for-existing-semicolon branch February 10, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants