#57002 closed defect (bug) (wontfix)
safecss_filter_attr() removes the trailing semincolon
Reported by: | Chouby | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | trivial | Version: | 2.8.1 |
Component: | Formatting | Keywords: | has-patch has-unit-tests |
Focuses: | css | Cc: |
Description
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
Change History (7)
This ticket was mentioned in PR #3579 on WordPress/wordpress-develop by @hugod.
2 years ago
#2
- Keywords has-patch has-unit-tests added
Ticket https://core.trac.wordpress.org/ticket/57002
## Description
- Add tests for
safecss_filter_attr()
. - Add a
;
before returning filtered CSS insafecss_filter_attr()
.
2 years ago
#3
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.
#4
@
2 years ago
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 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.
#5
@
2 years ago
Hi @peterwilsoncc
Thanks for the feedback, I didn't think about it that way.
Should I close my PR?
#6
@
2 years ago
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
@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.
2 years ago
#7
Won't fix, see this comment https://core.trac.wordpress.org/ticket/57002#comment:4
Copy paste drama :/. Of course, one should read: