[wp-trac] [WordPress Trac] #55966: safecss_filter_attr() returns empty if containing min()

[WordPress Trac]

#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