Skip to content

Commit 9038612

Browse files
sirrealsirreal
committed
Respect comments when appending style declarations
1 parent 711ad9e commit 9038612

2 files changed

Lines changed: 144 additions & 4 deletions

File tree

‎src/wp-includes/html-api/class-wp-html-style-attribute-processor.php‎

Lines changed: 111 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,16 +241,17 @@ public function append_declaration( string $property_name, string $value, bool $
241241
$old_declaration_count = count( $this->declarations );
242242
$trimmed_style = rtrim( $this->style, self::WHITESPACE );
243243
$insert_at = strlen( $trimmed_style );
244-
$separator = $this->has_queued_append_at( $insert_at ) ? ' ' : '';
244+
$separator = $this->get_append_separator( $insert_at );
245+
$declaration_text = $this->serialize_declaration( $property_name, $value, $important );
245246

246-
if ( '' === $separator && '' !== trim( $trimmed_style, self::WHITESPACE ) ) {
247-
$separator = ( ';' === substr( $trimmed_style, -1 ) ) ? ' ' : '; ';
247+
if ( ! $this->is_parseable_append( $insert_at, $separator, $declaration_text, $old_declaration_count ) ) {
248+
return false;
248249
}
249250

250251
$this->queue_lexical_update(
251252
$insert_at,
252253
0,
253-
$separator . $this->serialize_declaration( $property_name, $value, $important ),
254+
$separator . $declaration_text,
254255
null
255256
);
256257

@@ -762,6 +763,112 @@ private function has_queued_append_at( int $insert_at ): bool {
762763
return false;
763764
}
764765

766+
/**
767+
* Gets the separator to use before an appended declaration.
768+
*
769+
* Comments and whitespace are CSS parser trivia. They do not determine
770+
* whether the existing declaration list needs a semicolon before appending.
771+
*
772+
* @param int $insert_at Byte offset at which the declaration will be inserted.
773+
* @return string Separator text.
774+
*/
775+
private function get_append_separator( int $insert_at ): string {
776+
if ( $this->has_queued_append_at( $insert_at ) ) {
777+
return ' ';
778+
}
779+
780+
$last_token = $this->get_last_top_level_non_ignored_token_before( $insert_at );
781+
782+
if ( null === $last_token ) {
783+
return $insert_at > 0 ? ' ' : '';
784+
}
785+
786+
return WP_CSS_Token_Processor::TOKEN_SEMICOLON === $last_token['type'] ? ' ' : '; ';
787+
}
788+
789+
/**
790+
* Gets the last top-level non-ignored token before a byte offset.
791+
*
792+
* @param int $insert_at Byte offset before which to scan.
793+
* @return array{type:string, value:string|null, start:int, length:int, end:int}|null Token metadata.
794+
*/
795+
private function get_last_top_level_non_ignored_token_before( int $insert_at ): ?array {
796+
$count = count( $this->tokens );
797+
$index = 0;
798+
$last_token = null;
799+
800+
while ( $index < $count ) {
801+
$token = $this->tokens[ $index ];
802+
803+
if ( $token['start'] >= $insert_at ) {
804+
break;
805+
}
806+
807+
if ( $this->is_ignored_token( $token ) ) {
808+
++$index;
809+
continue;
810+
}
811+
812+
$last_token = $token;
813+
814+
if ( WP_CSS_Token_Processor::TOKEN_FUNCTION === $token['type'] ) {
815+
list( $index ) = $this->consume_simple_block( $index, WP_CSS_Token_Processor::TOKEN_RIGHT_PAREN );
816+
continue;
817+
}
818+
819+
if ( WP_CSS_Token_Processor::TOKEN_LEFT_PAREN === $token['type'] ) {
820+
list( $index ) = $this->consume_simple_block( $index, WP_CSS_Token_Processor::TOKEN_RIGHT_PAREN );
821+
continue;
822+
}
823+
824+
if ( WP_CSS_Token_Processor::TOKEN_LEFT_BRACKET === $token['type'] ) {
825+
list( $index ) = $this->consume_simple_block( $index, WP_CSS_Token_Processor::TOKEN_RIGHT_BRACKET );
826+
continue;
827+
}
828+
829+
if ( WP_CSS_Token_Processor::TOKEN_LEFT_BRACE === $token['type'] ) {
830+
list( $index ) = $this->consume_simple_block( $index, WP_CSS_Token_Processor::TOKEN_RIGHT_BRACE );
831+
continue;
832+
}
833+
834+
++$index;
835+
}
836+
837+
return $last_token;
838+
}
839+
840+
/**
841+
* Checks whether an appended declaration will parse as a new declaration.
842+
*
843+
* Malformed existing declarations can leave the append point inside an
844+
* unclosed component value. In that case preserving the original text and
845+
* appending a top-level declaration are incompatible.
846+
*
847+
* @param int $insert_at Byte offset at which the declaration will be inserted.
848+
* @param string $separator Separator text.
849+
* @param string $declaration_text Serialized declaration text.
850+
* @param int $old_declaration_count Number of declarations before appending.
851+
* @return bool Whether the appended declaration is parseable.
852+
*/
853+
private function is_parseable_append( int $insert_at, string $separator, string $declaration_text, int $old_declaration_count ): bool {
854+
$expected_start = $insert_at + strlen( $separator );
855+
$expected_after = $expected_start + strlen( $declaration_text );
856+
$candidate_style = substr( $this->style, 0, $insert_at ) . $separator . $declaration_text . substr( $this->style, $insert_at );
857+
$candidate = new self( $candidate_style );
858+
859+
if ( count( $candidate->declarations ) !== $old_declaration_count + 1 ) {
860+
return false;
861+
}
862+
863+
foreach ( $candidate->declarations as $declaration ) {
864+
if ( $expected_start === $declaration['start'] && $expected_after === $declaration['after'] ) {
865+
return true;
866+
}
867+
}
868+
869+
return false;
870+
}
871+
765872
/**
766873
* Merges overlapping lexical updates into non-overlapping replacements.
767874
*

‎tests/phpunit/tests/html-api/wpHtmlStyleAttributeProcessor.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,39 @@ public function test_append_declaration_preserves_multiple_appends_in_order() {
208208
$this->assertSame( 'color: red; background: white;', $processor->get_updated_style() );
209209
}
210210

211+
/**
212+
* @covers ::append_declaration
213+
* @covers ::get_updated_style
214+
*/
215+
public function test_append_declaration_treats_comments_as_trivia_for_separators() {
216+
$processor = new WP_HTML_Style_Attribute_Processor( '/*keep*/' );
217+
218+
$this->assertTrue( $processor->append_declaration( 'color', 'red' ) );
219+
$this->assertSame( '/*keep*/ color: red;', $processor->get_updated_style() );
220+
221+
$processor = new WP_HTML_Style_Attribute_Processor( 'color: red;/*keep*/' );
222+
223+
$this->assertTrue( $processor->append_declaration( 'background', 'white' ) );
224+
$this->assertSame( 'color: red;/*keep*/ background: white;', $processor->get_updated_style() );
225+
226+
$processor = new WP_HTML_Style_Attribute_Processor( 'color: var(--x;/*keep*/);' );
227+
228+
$this->assertTrue( $processor->append_declaration( 'background', 'white' ) );
229+
$this->assertSame( 'color: var(--x;/*keep*/); background: white;', $processor->get_updated_style() );
230+
}
231+
232+
/**
233+
* @covers ::append_declaration
234+
* @covers ::get_updated_style
235+
*/
236+
public function test_append_declaration_rejects_unclosed_component_value_append_points() {
237+
$style = 'color: var(--x;/*keep*/';
238+
$processor = new WP_HTML_Style_Attribute_Processor( $style );
239+
240+
$this->assertFalse( $processor->append_declaration( 'background', 'white' ) );
241+
$this->assertSame( $style, $processor->get_updated_style() );
242+
}
243+
211244
/**
212245
* @covers ::append_declaration
213246
* @covers ::next_declaration

0 commit comments

Comments
 (0)