Performance: Follow-up fixes to prepend_to_selector() optimization.

backport-changelog/7.1/12306.md

@@ -1,3 +1,4 @@
 https://github.com/WordPress/wordpress-develop/pull/12306
 
 * https://github.com/WordPress/gutenberg/pull/76556
+* https://github.com/WordPress/gutenberg/pull/79499

lib/class-wp-theme-json-gutenberg.php

@@ -1304,8 +1304,26 @@ protected static function prepend_to_selector( $selector, $to_prepend ) {
 			return $to_prepend . $selector;
 		}
 
-		// Gate fast path, won't work for all selectors
-		if ( ! str_contains( $selector, '(' ) ) {
+		/**
+		 * Check for an opportunity to skip the more-costly selector splitting.
+		 * This should be possible if there are no comments, strings, functions,
+		 * URLs, escapes, or comment declaration openers (CDOs).
+		 *
+		 * Note that this means the fast-path will not apply for selectors like
+		 * the following incomplete list:
+		 *
+		 *  - `[class ~= "wide"]`
+		 *  - `.wp-block:is(.is-style-a, .is-style-b)`
+		 *  - `:nth-child(1)`
+		 *
+		 * These syntax forms all present opportunities where a comma may not
+		 * separate selectors. If none of the start characters are present,
+		 * there should be no way for a comma to mean anything other than a
+		 * comma token. The exception are syntax errors, which are not handled here.
+		 *
+		 * @see https://www.w3.org/TR/css-syntax-3/#parse-comma-separated-list-of-component-values
+		 */
+		if ( strlen( $selector ) === strcspn( $selector, '/\'"(<\\' ) ) {
 			return $to_prepend . str_replace( ',', ',' . $to_prepend, $selector );
 		}
 
@@ -1319,38 +1337,156 @@ protected static function prepend_to_selector( $selector, $to_prepend ) {
 	}
 
 	/**
-	 * Splits a selector list by top-level commas.
+	 * Splits a selector list into separate selectors.
+	 *
+	 * While selectors are joined by commas, not all commas separate top-level selectors.
+	 * This method only separates top-level selectors, so some commas may appear inside
+	 * strings, nested selectors, and comments. Leading and trailing CSS whitespace is
+	 * trimmed from the returned list items.
+	 *
+	 * Non-selector content, such as comments, are retained in the list in the same item
+	 * as the selector content they follow.
+	 *
+	 * Example:
+	 *
+	 *     array( '.wp-block' )    === self::split_selector_list( '.wp-block' );
+	 *     array( '.one', '.two' ) === self::split_selector_list( '.one, .two' );
+	 *
+	 *     // Nested selector lists are retained within their containing selector.
+	 *     array( ':is(.a, .b)', 'c' ) === self::split_selector_list( ':is(.a, .b), .c' );
+	 *
+	 *     // Commas within strings do not separate selectors.
+	 *     $selectors   = self::split_selector_list( '[data-label="Save, continue"],.fallback' );
+	 *     $selectors === array( '[data-label="Save, continue"]', '.fallback' )
+	 *
+	 *     array( 'lang(zh, "*-hant")', '.foo' ) === self::split_selector_list( 'lang(zh, "*-hant"), .foo' );
+	 *
+	 *     // Identifiers may contain escaped commas.
+	 *     array( '.foo\,bar', '.baz' ) === self::split_selector_list( '.foo\,bar,.baz' );
+	 *
+	 *     // Comments stay with the selector they follow.
+	 *     array( '.a /* a, the first *\/', '.b' ) === self::split_selector_list( '.a /* a, the first *\/,.b' );
+	 *
+	 * @see https://www.w3.org/TR/selectors/#parse-selector
+	 * @see https://www.w3.org/TR/css-syntax-3/
 	 *
 	 * @param string $selector CSS selector list.
 	 * @return string[] Selectors.
 	 */
-	protected static function split_selector_list( $selector ) {
+	protected static function split_selector_list( $selector ): array {
 		if ( ! str_contains( $selector, ',' ) ) {
 			return array( $selector );
 		}
 
 		$selectors         = array();
-		$current_selector  = '';
-		$parentheses_depth = 0;
 		$selector_length   = strlen( $selector );
+		$parentheses_depth = 0;
+		$at                = 0;
+		$was_at            = 0;
 
-		for ( $i = 0; $i < $selector_length; $i++ ) {
-			$char = $selector[ $i ];
+		while ( $at < $selector_length ) {
+			$next_at = $at + strcspn( $selector, '/,\'"()<-\\', $at );
+			if ( $next_at >= $selector_length ) {
+				break;
+			}
 
-			if ( '(' === $char ) {
-				++$parentheses_depth;
-			} elseif ( ')' === $char && $parentheses_depth > 0 ) {
-				--$parentheses_depth;
-			} elseif ( ',' === $char && 0 === $parentheses_depth ) {
-				$selectors[]      = $current_selector;
-				$current_selector = '';
+			$next_cp = $selector[ $next_at ];
+
+			// Escaped syntax characters do not act as delimiters.
+			if ( '\\' === $next_cp ) {
+				$at = min( $next_at + 2, $selector_length );
+				continue;
+			}
+
+			/*
+			 * Start of a parenthesized expression, which maintains a stack of parentheses.
+			 * For the sake of this function, no selector list will be split inside parentheses.
+			 * Therefore it’s possible to jump ahead until this list completes.
+			 */
+			if ( '(' === $next_cp || ')' === $next_cp ) {
+				$parentheses_depth += '(' === $next_cp ? 1 : -1;
+				$at                 = $next_at + 1;
 				continue;
 			}
 
-			$current_selector .= $char;
+			// Start of a string, which will be incorporated into the selector in which it’s found.
+			if ( "'" === $next_cp || '"' === $next_cp ) {
+				$end_of_string = $next_at + 1;
+				while ( $end_of_string < $selector_length ) {
+					$end_of_string += strcspn( $selector, "{$next_cp}\\", $end_of_string );
+					if ( $end_of_string >= $selector_length ) {
+						break;
+					}
+
+					$end_cp = $selector[ $end_of_string ];
+
+					// Skip escaped characters.
+					if ( '\\' === $end_cp ) {
+						$end_of_string = $end_of_string + 2;
+						continue;
+					}
+
+					if ( $next_cp === $end_cp ) {
+						++$end_of_string;
+						break;
+					}
+
+					++$end_of_string;
+				}
+
+				$at = $end_of_string;
+				continue;
+			}
+
+			// Start of a comment, which will be incorporated into the selector in which it’s found.
+			if ( '/' === $next_cp && ( $next_at + 1 ) < $selector_length && '*' === $selector[ $next_at + 1 ] ) {
+				$comment_end_at = strpos( $selector, '*/', $next_at + 1 );
+				$is_terminated  = false !== $comment_end_at;
+				$after_comment  = $is_terminated ? $comment_end_at + 2 : strlen( $selector );
+				$at             = $after_comment;
+				continue;
+			}
+
+			// Start of a CDO or CDC, which will be incorporated into the selector in which it’s found.
+			if (
+				( '<' === $next_cp && 0 === substr_compare( $selector, '<!--', $next_at, 4 ) ) ||
+				( '-' === $next_cp && 0 === substr_compare( $selector, '-->', $next_at, 3 ) )
+			) {
+				$at = $next_at + ( '<' === $next_cp ? 4 : 3 );
+				continue;
+			}
+
+			// Everything else is either a comma token or part of a selector.
+			if ( ',' === $next_cp && 0 === $parentheses_depth ) {
+				/**
+				 * Trim each selector so that downstream code doesn’t see whitespace
+				 * as the first character in a selector and get confused.
+				 *
+				 * There is inconsistency in this because comments and other syntax
+				 * are included which are also not part of the selector itself, but
+				 * a tradeoff is made between removing common syntax which carries
+				 * no meaning and rarer syntax which leaves auxiliary information.
+				 *
+				 * > A newline, U+0009 CHARACTER TABULATION, or U+0020 SPACE.
+				 * > Note that U+000D CARRIAGE RETURN and U+000C FORM FEED are
+				 * > not included in this definition, as they are converted
+				 * > to U+000A LINE FEED during preprocessing.
+				 *
+				 * @see https://www.w3.org/TR/css-syntax/#whitespace
+				 * @see https://www.w3.org/TR/css-syntax/#newline
+				 */
+				$selectors[] = trim( substr( $selector, $was_at, $next_at - $was_at ), " \t\n" );
+				$at          = $next_at + 1;
+				$was_at      = $at;
+				continue;
+			}
+
+			$at = $next_at + 1;
 		}
 
-		$selectors[] = $current_selector;
+		if ( $was_at < $selector_length ) {
+			$selectors[] = substr( $selector, $was_at );
+		}
 
 		return $selectors;
 	}

phpunit/class-wp-theme-json-selector-list.php

@@ -0,0 +1,122 @@
+<?php
+/**
+ * Tests selector list helpers in WP_Theme_JSON_Gutenberg.
+ *
+ * @package gutenberg
+ *
+ * @covers WP_Theme_JSON_Gutenberg
+ */
+
+class WP_Theme_JSON_Gutenberg_Selector_List_Test extends WP_UnitTestCase {
+	/**
+	 * Invokes a protected static method on WP_Theme_JSON_Gutenberg.
+	 *
+	 * @param string $method_name Method name.
+	 * @param mixed ...$args Method arguments.
+	 * @return mixed Method result.
+	 */
+	private static function invoke_theme_json_method( $method_name, ...$args ) {
+		$theme_json = new ReflectionClass( 'WP_Theme_JSON_Gutenberg' );
+
+		$method = $theme_json->getMethod( $method_name );
+		if ( PHP_VERSION_ID < 80100 ) {
+			$method->setAccessible( true );
+		}
+
+		return $method->invokeArgs( null, $args );
+	}
+
+	/**
+	 * @dataProvider data_split_selector_list
+	 *
+	 * @param string $selector CSS selector list.
+	 * @param string[] $expected Expected selectors.
+	 */
+	public function test_split_selector_list( $selector, $expected ) {
+		$this->assertSame(
+			$expected,
+			self::invoke_theme_json_method( 'split_selector_list', $selector )
+		);
+	}
+
+	/**
+	 * @return array<string, array{selector: string, expected: string[]}>
+	 */
+	public function data_split_selector_list() {
+		return array(
+			'simple selector list'                  => array(
+				'selector' => 'h1,h2',
+				'expected' => array( 'h1', 'h2' ),
+			),
+			'preserves whitespace around selectors' => array(
+				'selector' => '.a ,  .b , .c',
+				'expected' => array( '.a ', '  .b ', ' .c' ),
+			),
+			'selector function list argument'       => array(
+				'selector' => ':where(.a, .b),.c',
+				'expected' => array( ':where(.a, .b)', '.c' ),
+			),
+			'nested selector functions'             => array(
+				'selector' => ':where(:not(.a, .b), .c),.d',
+				'expected' => array( ':where(:not(.a, .b), .c)', '.d' ),
+			),
+			'attribute string containing comma'     => array(
+				'selector' => '[data-label="Save, continue"],.fallback',
+				'expected' => array( '[data-label="Save, continue"]', '.fallback' ),
+			),
+			'escaped comma in identifier'           => array(
+				'selector' => '.foo\,bar,.baz',
+				'expected' => array( '.foo\,bar', '.baz' ),
+			),
+			'escaped closing parenthesis in selector function' => array(
+				'selector' => ':is(.a\), .b), .c',
+				'expected' => array( ':is(.a\), .b)', ' .c' ),
+			),
+			'quoted function argument before top-level comma' => array(
+				'selector' => ':lang(zh, "*-hant"),.foo',
+				'expected' => array( ':lang(zh, "*-hant")', '.foo' ),
+			),
+			'escaped quote and comma inside string' => array(
+				'selector' => '[data-x="\",inside"],.b',
+				'expected' => array( '[data-x="\",inside"]', '.b' ),
+			),
+			'comment containing comma'              => array(
+				'selector' => '.a/*,*/,.b',
+				'expected' => array( '.a/*,*/', '.b' ),
+			),
+		);
+	}
+
+	/**
+	 * @dataProvider data_prepend_to_selector_uses_safe_splitting
+	 *
+	 * @param string $selector CSS selector list.
+	 * @param string $expected Expected prepended selector.
+	 */
+	public function test_prepend_to_selector_uses_safe_splitting( $selector, $expected ) {
+		$this->assertSame(
+			$expected,
+			self::invoke_theme_json_method( 'prepend_to_selector', $selector, '.scope ' )
+		);
+	}
+
+	/**
+	 * @return array<string, array{selector: string, expected: string}>
+	 */
+	public function data_prepend_to_selector_uses_safe_splitting() {
+		return array(
+			'fast path for simple selector list' => array(
+				'selector' => 'h1,h2',
+				'expected' => '.scope h1,.scope h2',
+			),
+			'escaped comma does not trigger fast-path breakage' => array(
+				'selector' => '.foo\,bar,.baz',
+				'expected' => '.scope .foo\,bar,.scope .baz',
+			),
+			'quoted selector function argument is preserved' => array(
+				'selector' => ':lang(zh, "*-hant"),.foo',
+				'expected' => '.scope :lang(zh, "*-hant"),.scope .foo',
+			),
+		);
+	}
+}

phpunit/class-wp-theme-json-test.php

@@ -8228,7 +8228,7 @@ public function data_prepend_to_selector() {
 				'to_prepend' => '.pre ',
 				'expected'   => '.pre .a ,.pre   .b ,.pre  .c',
 			),
-			'where with internal commas no top-level comma' => array(
+			'where with internal commas and no top-level comma' => array(
 				'selector'   => ':where(.a, .b)',
 				'to_prepend' => '.scope ',
 				'expected'   => '.scope :where(.a, .b)',