Skip to content

Commit 608993d

Browse files
dmsnelldmsnell
committed
Editor: Skip parsing in prepend_to_selector() for simple selectors.
Splitting a CSS selector involves unnecessary computation and allocation when the selector could not possibly involve more than one selector. It was discovered in profiling that this function was being called 4000+ times in a single request, and adding a pre-condition before splitting saved meaningful time on a page render. This patch adds the pre-condition to verify that any commas present in a selector string can only be “comma tokens,” which split selectors at the top level, making it safe to fall back to simpler parsing that’s more efficient than general separator splitting. This commit synchronizes work from the following Gutenberg PRs: - WordPress/gutenberg#76556 - WordPress/gutenberg#79499 Developed in: WordPress#12306 Discussed in: https://core.trac.wordpress.org/ticket/65533 Follow-up to [62607]. Props dmsnell, josephscott. See #65533. git-svn-id: https://develop.svn.wordpress.org/trunk@62650 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 142e7ee commit 608993d

2 files changed

Lines changed: 330 additions & 2 deletions

File tree

‎src/wp-includes/class-wp-theme-json.php‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,8 +1296,32 @@ protected static function sanitize( $input, $valid_block_names, $valid_element_n
12961296
*/
12971297
protected static function append_to_selector( $selector, $to_append ) {
12981298
if ( ! str_contains( $selector, ',' ) ) {
1299-
return $selector . $to_append;
1299+
return trim( $selector, " \t\n" ) . $to_append;
13001300
}
1301+
1302+
/**
1303+
* Check for an opportunity to skip the more-costly selector splitting.
1304+
* This should be possible if there are no comments, strings, functions,
1305+
* URLs, escapes, or comment declaration openers (CDOs).
1306+
*
1307+
* Note that this means the fast-path will not apply for selectors like
1308+
* the following incomplete list:
1309+
*
1310+
* - `[class ~= "wide"]`
1311+
* - `.wp-block:is(.is-style-a, .is-style-b)`
1312+
* - `:nth-child(1)`
1313+
*
1314+
* These syntax forms all present opportunities where a comma may not
1315+
* separate selectors. If none of the start characters are present,
1316+
* there should be no way for a comma to mean anything other than a
1317+
* comma token. The exception are syntax errors, which are not handled here.
1318+
*
1319+
* @see https://www.w3.org/TR/css-syntax-3/#parse-comma-separated-list-of-component-values
1320+
*/
1321+
if ( strlen( $selector ) === strcspn( $selector, '/\'"(<\\' ) ) {
1322+
return preg_replace( '~[ \t\n]*,[ \t\n]*~', "{$to_append}, ", trim( $selector, " \t\n" ) ) . $to_append;
1323+
}
1324+
13011325
$new_selectors = array();
13021326
$selectors = static::split_selector_list( $selector );
13031327
foreach ( $selectors as $sel ) {
@@ -1321,8 +1345,32 @@ protected static function append_to_selector( $selector, $to_append ) {
13211345
*/
13221346
protected static function prepend_to_selector( $selector, $to_prepend ) {
13231347
if ( ! str_contains( $selector, ',' ) ) {
1324-
return $to_prepend . $selector;
1348+
return $to_prepend . trim( $selector, " \t\n" );
13251349
}
1350+
1351+
/**
1352+
* Check for an opportunity to skip the more-costly selector splitting.
1353+
* This should be possible if there are no comments, strings, functions,
1354+
* URLs, escapes, or comment declaration openers (CDOs).
1355+
*
1356+
* Note that this means the fast-path will not apply for selectors like
1357+
* the following incomplete list:
1358+
*
1359+
* - `[class ~= "wide"]`
1360+
* - `.wp-block:is(.is-style-a, .is-style-b)`
1361+
* - `:nth-child(1)`
1362+
*
1363+
* These syntax forms all present opportunities where a comma may not
1364+
* separate selectors. If none of the start characters are present,
1365+
* there should be no way for a comma to mean anything other than a
1366+
* comma token. The exception are syntax errors, which are not handled here.
1367+
*
1368+
* @see https://www.w3.org/TR/css-syntax-3/#parse-comma-separated-list-of-component-values
1369+
*/
1370+
if ( strlen( $selector ) === strcspn( $selector, '/\'"(<\\' ) ) {
1371+
return $to_prepend . preg_replace( '~[ \t\n]*,[ \t\n]*~', ", {$to_prepend}", trim( $selector, " \t\n" ) );
1372+
}
1373+
13261374
$new_selectors = array();
13271375
$selectors = static::split_selector_list( $selector );
13281376
foreach ( $selectors as $sel ) {

‎tests/phpunit/tests/theme/wpThemeJson.php‎

Lines changed: 280 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7731,4 +7731,284 @@ public function test_block_custom_states_ignores_non_whitelisted() {
77317731
$this->assertStringNotContainsString( '-current', $stylesheet_unsupported );
77327732
$this->assertStringNotContainsString( 'current-menu-item', $stylesheet_unsupported );
77337733
}
7734+
7735+
/**
7736+
* Tests that append_to_selector correctly appends to single and
7737+
* compound (comma-separated) selectors.
7738+
*
7739+
* @ticket 65533
7740+
*
7741+
* @dataProvider data_append_to_selector
7742+
*
7743+
* @param string $selector Original CSS selector.
7744+
* @param string $to_append Selector to append.
7745+
* @param string $expected Expected resulting selector.
7746+
*/
7747+
public function test_append_to_selector( $selector, $to_append, $expected ) {
7748+
$theme_json = new ReflectionClass( 'WP_Theme_JSON' );
7749+
7750+
$func = $theme_json->getMethod( 'append_to_selector' );
7751+
if ( PHP_VERSION_ID < 80100 ) {
7752+
$func->setAccessible( true );
7753+
}
7754+
7755+
$actual = $func->invoke( null, $selector, $to_append );
7756+
7757+
$this->assertSame( $expected, $actual );
7758+
}
7759+
7760+
/**
7761+
* Data provider.
7762+
*
7763+
* @return array<string, {selector: string, to_append: string, expeted: string}>
7764+
*/
7765+
public function data_append_to_selector() {
7766+
return array(
7767+
'single class selector' => array(
7768+
'selector' => '.inner',
7769+
'to_append' => ':hover',
7770+
'expected' => '.inner:hover',
7771+
),
7772+
'single element selector' => array(
7773+
'selector' => 'p',
7774+
'to_append' => ':hover',
7775+
'expected' => 'p:hover',
7776+
),
7777+
'single id selector' => array(
7778+
'selector' => '#main',
7779+
'to_append' => ':focus',
7780+
'expected' => '#main:focus',
7781+
),
7782+
'two comma-separated selectors without spaces' => array(
7783+
'selector' => 'h1,h2',
7784+
'to_append' => ':hover',
7785+
'expected' => 'h1:hover, h2:hover',
7786+
),
7787+
'three comma-separated selectors without spaces' => array(
7788+
'selector' => 'h1,h2,h3',
7789+
'to_append' => ':focus',
7790+
'expected' => 'h1:focus, h2:focus, h3:focus',
7791+
),
7792+
'comma-separated class selectors without spaces' => array(
7793+
'selector' => '.foo,.bar',
7794+
'to_append' => '.is-active',
7795+
'expected' => '.foo.is-active, .bar.is-active',
7796+
),
7797+
'append without leading pseudo' => array(
7798+
'selector' => '.child',
7799+
'to_append' => '.parent',
7800+
'expected' => '.child.parent',
7801+
),
7802+
'compound selector with class append no comma spaces' => array(
7803+
'selector' => '.a,.b',
7804+
'to_append' => '.parent',
7805+
'expected' => '.a.parent, .b.parent',
7806+
),
7807+
'descendant selector appended' => array(
7808+
'selector' => '.block .inner',
7809+
'to_append' => ':hover',
7810+
'expected' => '.block .inner:hover',
7811+
),
7812+
'descendant selectors comma-separated without spaces' => array(
7813+
'selector' => '.block .inner,.block .alt',
7814+
'to_append' => ':hover',
7815+
'expected' => '.block .inner:hover, .block .alt:hover',
7816+
),
7817+
'empty selector' => array(
7818+
'selector' => '',
7819+
'to_append' => ':hover',
7820+
'expected' => ':hover',
7821+
),
7822+
'empty append' => array(
7823+
'selector' => '.child',
7824+
'to_append' => '',
7825+
'expected' => '.child',
7826+
),
7827+
'both empty' => array(
7828+
'selector' => '',
7829+
'to_append' => '',
7830+
'expected' => '',
7831+
),
7832+
'attribute selector' => array(
7833+
'selector' => '[data-type="example"]',
7834+
'to_append' => ':hover',
7835+
'expected' => '[data-type="example"]:hover',
7836+
),
7837+
'pseudo-class selector' => array(
7838+
'selector' => ':where(.is-layout-flex)',
7839+
'to_append' => ':hover',
7840+
'expected' => ':where(.is-layout-flex):hover',
7841+
),
7842+
'many comma-separated selectors without spaces' => array(
7843+
'selector' => 'h1,h2,h3,h4,h5,h6',
7844+
'to_append' => ':hover',
7845+
'expected' => 'h1:hover, h2:hover, h3:hover, h4:hover, h5:hover, h6:hover',
7846+
),
7847+
'real world block element selector' => array(
7848+
'selector' => 'p',
7849+
'to_append' => '.is-style-custom',
7850+
'expected' => 'p.is-style-custom',
7851+
),
7852+
'real world compound block element selectors' => array(
7853+
'selector' => 'a,.wp-element-button',
7854+
'to_append' => '.is-style-custom',
7855+
'expected' => 'a.is-style-custom, .wp-element-button.is-style-custom',
7856+
),
7857+
'spaces after commas are normalized' => array(
7858+
'selector' => 'h1, h2, h3',
7859+
'to_append' => ':hover',
7860+
'expected' => 'h1:hover, h2:hover, h3:hover',
7861+
),
7862+
'spaces after commas normalized with class selectors' => array(
7863+
'selector' => '.foo, .bar',
7864+
'to_append' => '.is-active',
7865+
'expected' => '.foo.is-active, .bar.is-active',
7866+
),
7867+
'mixed whitespace around commas is normalized' => array(
7868+
'selector' => '.a , .b , .c',
7869+
'to_append' => ':hover',
7870+
'expected' => '.a:hover, .b:hover, .c:hover',
7871+
),
7872+
);
7873+
}
7874+
7875+
/**
7876+
* Tests that prepend_to_selector correctly prepends to single and
7877+
* compound (comma-separated) selectors.
7878+
*
7879+
* @ticket 65533
7880+
*
7881+
* @dataProvider data_prepend_to_selector
7882+
*
7883+
* @param string $selector Original CSS selector.
7884+
* @param string $to_prepend Selector to prepend.
7885+
* @param string $expected Expected resulting selector.
7886+
*/
7887+
public function test_prepend_to_selector( $selector, $to_prepend, $expected ) {
7888+
$theme_json = new ReflectionClass( 'WP_Theme_JSON' );
7889+
7890+
$func = $theme_json->getMethod( 'prepend_to_selector' );
7891+
if ( PHP_VERSION_ID < 80100 ) {
7892+
$func->setAccessible( true );
7893+
}
7894+
7895+
$actual = $func->invoke( null, $selector, $to_prepend );
7896+
7897+
$this->assertSame( $expected, $actual );
7898+
}
7899+
7900+
/**
7901+
* Data provider.
7902+
*
7903+
* @return array<string, {selector: string, to_prepend: string, expeted: string}>
7904+
*/
7905+
public function data_prepend_to_selector() {
7906+
return array(
7907+
'single class selector' => array(
7908+
'selector' => '.inner',
7909+
'to_prepend' => '.wrapper ',
7910+
'expected' => '.wrapper .inner',
7911+
),
7912+
'single element selector' => array(
7913+
'selector' => 'p',
7914+
'to_prepend' => '.wrapper ',
7915+
'expected' => '.wrapper p',
7916+
),
7917+
'single id selector' => array(
7918+
'selector' => '#main',
7919+
'to_prepend' => '.wrapper ',
7920+
'expected' => '.wrapper #main',
7921+
),
7922+
'two comma-separated selectors without spaces' => array(
7923+
'selector' => 'h1,h2',
7924+
'to_prepend' => '.wrapper ',
7925+
'expected' => '.wrapper h1, .wrapper h2',
7926+
),
7927+
'three comma-separated selectors without spaces' => array(
7928+
'selector' => 'h1,h2,h3',
7929+
'to_prepend' => '.some-class ',
7930+
'expected' => '.some-class h1, .some-class h2, .some-class h3',
7931+
),
7932+
'comma-separated class selectors without spaces' => array(
7933+
'selector' => '.foo,.bar',
7934+
'to_prepend' => '.prefix ',
7935+
'expected' => '.prefix .foo, .prefix .bar',
7936+
),
7937+
'prepend without trailing space' => array(
7938+
'selector' => '.child',
7939+
'to_prepend' => '.parent',
7940+
'expected' => '.parent.child',
7941+
),
7942+
'compound selector without trailing space no comma spaces' => array(
7943+
'selector' => '.a,.b',
7944+
'to_prepend' => '.parent',
7945+
'expected' => '.parent.a, .parent.b',
7946+
),
7947+
'descendant selector prepended' => array(
7948+
'selector' => '.block .inner',
7949+
'to_prepend' => '.scope ',
7950+
'expected' => '.scope .block .inner',
7951+
),
7952+
'descendant selectors comma-separated without spaces' => array(
7953+
'selector' => '.block .inner,.block .alt',
7954+
'to_prepend' => '.scope ',
7955+
'expected' => '.scope .block .inner, .scope .block .alt',
7956+
),
7957+
'empty selector' => array(
7958+
'selector' => '',
7959+
'to_prepend' => '.prefix ',
7960+
'expected' => '.prefix ',
7961+
),
7962+
'empty prepend' => array(
7963+
'selector' => '.child',
7964+
'to_prepend' => '',
7965+
'expected' => '.child',
7966+
),
7967+
'both empty' => array(
7968+
'selector' => '',
7969+
'to_prepend' => '',
7970+
'expected' => '',
7971+
),
7972+
'attribute selector' => array(
7973+
'selector' => '[data-type="example"]',
7974+
'to_prepend' => '.scope ',
7975+
'expected' => '.scope [data-type="example"]',
7976+
),
7977+
'pseudo-class selector' => array(
7978+
'selector' => ':where(.is-layout-flex)',
7979+
'to_prepend' => '.editor ',
7980+
'expected' => '.editor :where(.is-layout-flex)',
7981+
),
7982+
'many comma-separated selectors without spaces' => array(
7983+
'selector' => 'h1,h2,h3,h4,h5,h6',
7984+
'to_prepend' => '.content ',
7985+
'expected' => '.content h1, .content h2, .content h3, .content h4, .content h5, .content h6',
7986+
),
7987+
'real world block element selector' => array(
7988+
'selector' => 'p',
7989+
'to_prepend' => '.wp-block-group ',
7990+
'expected' => '.wp-block-group p',
7991+
),
7992+
'real world compound block element selectors' => array(
7993+
'selector' => 'a,.wp-element-button',
7994+
'to_prepend' => '.wp-block-group ',
7995+
'expected' => '.wp-block-group a, .wp-block-group .wp-element-button',
7996+
),
7997+
'spaces after commas are normalized' => array(
7998+
'selector' => 'h1, h2, h3',
7999+
'to_prepend' => '.some-class ',
8000+
'expected' => '.some-class h1, .some-class h2, .some-class h3',
8001+
),
8002+
'spaces after commas normalized with class selectors' => array(
8003+
'selector' => '.foo, .bar',
8004+
'to_prepend' => '.prefix ',
8005+
'expected' => '.prefix .foo, .prefix .bar',
8006+
),
8007+
'mixed whitespace around commas are normalized' => array(
8008+
'selector' => '.a , .b , .c',
8009+
'to_prepend' => '.pre ',
8010+
'expected' => '.pre .a, .pre .b, .pre .c',
8011+
),
8012+
);
8013+
}
77348014
}

0 commit comments

Comments
 (0)