[wp-trac] [WordPress Trac] #55966: safecss_filter_attr() returns empty if containing min()
WordPress Trac
noreply at wordpress.org
Tue Sep 6 05:36:12 UTC 2022
#55966: safecss_filter_attr() returns empty if containing min()
-------------------------------------------------+-------------------------
Reporter: uxl | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: 6.1
Component: Formatting | Version: 6.0
Severity: major | Resolution:
Keywords: has-patch has-unit-tests changes- | Focuses: css
requested |
-------------------------------------------------+-------------------------
Changes (by noisysocks):
* keywords: has-patch has-unit-tests => has-patch has-unit-tests changes-
requested
Comment:
Yes @ramonopoly definitely better to fix this for real in `wp_kses` than
using a filter.
Nice work on the patch @johnregan3! I applied [attachment:"55966.2.diff"]
locally and ran `phpunit --group kses`. All green.
The code looks good to me aside from one small thing: @johnregan3, could
you please add `'css' =>` and `'expected' =>` to the `array()`s in the
tests? That way the code is a little more self-documenting.
Current:
{{{#!php
// Allow min().
array(
'width: min(50%, 400px)',
'width: min(50%, 400px)',
),
}}}
Better:
{{{#!php
// Allow min().
array(
'css' => 'width: min(50%, 400px)',
'expected' => 'width: min(50%, 400px)',
),
}}}
I can't comment much on the proposed regex other than to say it doesn't
look unreasonable. If Gutenberg has been using this regex for some time as
@andrewserong says then that's really encouraging.
With that small bit of feedback above fixed I'd be happy to commit this.
Out of an abundance of caution, since `wp_kses` scares me, I'll ping
@peterwilsoncc and @hellofromTonya here for a second pair of eyes.
I'm adding `early` since this blocks https://github.com/WordPress
/wordpress-develop/pull/3199 which needs to land before beta 1.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/55966#comment:9>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list