safecss_filter_attr() removes the trailing semincolon

[Chouby]

A trailing semicolon is not mandatory at the end of CSS rules. It's however a best practice to add one.

However the output of safecss_filter_attr( 'margin-right: 20px;' ) is margin-right: 20px. I would expect that it keeps margin-right: 20px

[Chouby]

Copy paste drama :/. Of course, one should read:

I would expect that it keeps margin-right: 20px;

Ticket https://core.trac.wordpress.org/ticket/57002

## Description

• Add tests for safecss_filter_attr().
• Add a ; before returning filtered CSS in safecss_filter_attr().

I'm aware some tests are failing. This is due to the ; added in any cases...
I'll fix them once the ticket is validated.

[peterwilsoncc]

Thanks for the ticket.

I agree that it would be nicer if safecss_filter_attr() retained the trailing semi-colon but I worry that it's a little late to change the function.

If a plugin has the following code then the change would cause the style attribute to contain the string ;;.

<div
  style="<?php echo safecss_filter_attr( wp_unslash( $_GET[ 'user_css' ] ) ); ?>; more:css;" />
</div>

It's something browsers' error recovery should be able to handle but, as with the lack of trailing semi-colon, it's not very nice code.

[hugod]

Hi @peterwilsoncc
Thanks for the feedback, I didn't think about it that way.
Should I close my PR?

[peterwilsoncc]

@hugod Thanks for understanding, yes, let's close this off without a fix for now

To be clear, I absolutely agree with the premise that a trailing semi-colon would be best practice when style="safecss_filter_attr( /*styles */ )"; the risk of ending up with two consecutive semi-colons results in a relative decision about the nicer code.

Some choices require choosing the pragmatic over the ideal.

Won't fix, see this comment https://core.trac.wordpress.org/ticket/57002#comment:4