From 8c67e7bbfaaa51340ccbd11fe05ad6eac7e21808 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Thu, 20 Aug 2026 16:54:01 +0400 Subject: [PATCH 1/5] HTML API: Add tests for remove_attribute() preserving the self-closing flag. A `/` inside a tag is only a self-closing flag when immediately followed by `>`. When an attribute is directly preceded by a `/`, e.g. ``, removing the attribute leaves ``, introducing a self-closing flag that wasn't in the input and changing the tag's semantics. These tests currently fail and document the expected behavior. --- .../tests/html-api/wpHtmlTagProcessor.php | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php index 7f59239608d6b..89e197d7355ec 100644 --- a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php +++ b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php @@ -1595,6 +1595,65 @@ public static function data_html_with_duplicated_attributes() { ); } + /** + * Ensures that removing an attribute does not introduce a self-closing flag. + * + * A `/` inside a tag is only a self-closing flag when immediately followed by `>`. + * Anywhere else it's ignored as a separator. Removing an attribute that was + * directly preceded by a `/` could leave that `/` adjacent to the `>`, which + * would change the meaning of the tag, e.g. `` becoming ``. + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * + * @dataProvider data_remove_attribute_preserves_self_closing_flag + * + * @param string $html HTML containing a tag whose attribute will be removed. + * @param string $attribute_to_remove Name of the attribute to remove. + * @param string $expected Expected HTML after removing the attribute. + */ + public function test_remove_attribute_preserves_self_closing_flag( string $html, string $attribute_to_remove, string $expected ) { + $processor = new WP_HTML_Tag_Processor( $html ); + $processor->next_tag( 'G' ); + $had_self_closing_flag = $processor->has_self_closing_flag(); + + $this->assertTrue( $processor->remove_attribute( $attribute_to_remove ), 'Failed to remove the attribute.' ); + $updated_html = $processor->get_updated_html(); + $this->assertSame( $expected, $updated_html, 'Updated HTML did not match the expected output.' ); + + // Re-parse the output to confirm the tag's semantics are unchanged. + $processor = new WP_HTML_Tag_Processor( $updated_html ); + $processor->next_tag( 'G' ); + $this->assertNull( $processor->get_attribute( $attribute_to_remove ), 'Attribute still present in the updated HTML.' ); + $this->assertSame( + $had_self_closing_flag, + $processor->has_self_closing_flag(), + $had_self_closing_flag + ? 'Removing the attribute dropped the self-closing flag.' + : 'Removing the attribute introduced a self-closing flag.' + ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_remove_attribute_preserves_self_closing_flag() { + return array( + 'Slash before attribute' => array( 'ok', 'attr', 'ok' ), + 'Slash before attribute, no whitespace' => array( 'ok', 'attr', 'ok' ), + 'Multiple slashes before attribute' => array( 'ok', 'attr', 'ok' ), + 'Slash separating attributes, remove last' => array( 'ok', 'b', 'ok' ), + 'Slash separating attributes, remove first' => array( 'ok', 'a', 'ok' ), + 'Slash after quoted value, remove last' => array( 'ok', 'b', 'ok' ), + 'Slash before duplicated attributes' => array( 'ok', 'a', 'ok' ), + 'Slash and whitespace before attribute' => array( 'ok', 'attr', 'ok' ), + 'Self-closing flag is preserved' => array( 'ok', 'attr', 'ok' ), + 'Self-closing flag after quoted value' => array( 'ok', 'attr', 'ok' ), + 'Self-closing flag, slash before attribute' => array( 'ok', 'attr', 'ok' ), + ); + } + /** * @ticket 56299 * From 9b8a4bcc6d39f31c2ed88b63c8ee7872d8a02ea9 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Thu, 20 Aug 2026 16:56:30 +0400 Subject: [PATCH 2/5] HTML API: Prevent remove_attribute() from introducing a self-closing flag. Inside a tag, a `/` is ignored unless it's immediately followed by `>`, in which case it's the self-closing flag. When an attribute is directly preceded by a `/`, e.g. ``, removing just the attribute's own span left ``, introducing a self-closing flag that wasn't in the input and changing the semantics of the tag. Removals now also consume any `/` characters immediately preceding the attribute. This is safe because a `/` can only appear before an attribute as a separator: tag names and attribute names end at `/`, and inside an unquoted value it's part of the value rather than preceding an attribute. The same logic applies when removing duplicated attributes. --- .../html-api/class-wp-html-tag-processor.php | 55 +++++++++++++++---- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/src/wp-includes/html-api/class-wp-html-tag-processor.php b/src/wp-includes/html-api/class-wp-html-tag-processor.php index 88487fb068d11..ba60870d620a5 100644 --- a/src/wp-includes/html-api/class-wp-html-tag-processor.php +++ b/src/wp-includes/html-api/class-wp-html-tag-processor.php @@ -4786,24 +4786,57 @@ public function remove_attribute( $name ): bool { * * Result:
*/ - $this->lexical_updates[ $name ] = new WP_HTML_Text_Replacement( - $this->attributes[ $name ]->start, - $this->attributes[ $name ]->length, - '' - ); + $this->lexical_updates[ $name ] = $this->get_attribute_removal( $this->attributes[ $name ] ); // Removes any duplicated attributes if they were also present. - foreach ( $this->duplicate_attributes[ $name ] ?? array() as $attribute_token ) { - $this->lexical_updates[] = new WP_HTML_Text_Replacement( - $attribute_token->start, - $attribute_token->length, - '' - ); + foreach ( $this->duplicate_attributes[ $name ] ?? array() as $attribute_span ) { + $this->lexical_updates[] = $this->get_attribute_removal( $attribute_span ); } return true; } + /** + * Creates a text replacement which removes the given attribute from the document. + * + * Any `/` characters immediately preceding the attribute are removed along with it. + * Inside a tag, `/` is ignored unless it's immediately followed by `>`, in which case + * it's the self-closing flag. Leaving a `/` behind could therefore change the meaning + * of the tag if it ends up adjacent to the `>` after the attribute is removed. + * + * Example: + * + * + * ^----^ + * start end + * replacement: `` + * + * Result: + * + * Had only `attr` been removed the result would have been ``, which has a + * self-closing flag that wasn't in the input. + * + * @since 7.2.0 + * + * @param WP_HTML_Attribute_Token|WP_HTML_Span $attribute Attribute to remove, or the span it occupies. + * @return WP_HTML_Text_Replacement Replacement which removes the attribute. + */ + private function get_attribute_removal( $attribute ): WP_HTML_Text_Replacement { + $start = $attribute->start; + $end = $start + $attribute->length; + + /* + * A `/` can only precede an attribute as a separator: tag names and attribute + * names end at `/`, and it's part of an unquoted attribute value when found + * there. It's safe to consume every `/` leading up to the attribute. + */ + while ( $start > 0 && '/' === $this->html[ $start - 1 ] ) { + --$start; + } + + return new WP_HTML_Text_Replacement( $start, $end - $start, '' ); + } + /** * Adds a new class name to the currently matched tag. * From 9cf76f944179081051f62769a3e386f3568ff1d7 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 21 Aug 2026 08:34:57 +0400 Subject: [PATCH 3/5] HTML API: Extend remove_attribute() semantics tests to neighboring syntax. Broaden the self-closing flag test to assert that the tag name, the remaining attributes, and the following content are all unchanged, and add cases where removing an attribute along with the `/` preceding it would otherwise merge the tag name or a neighboring attribute with what follows, e.g. `` becoming ``. Also add a test for combining set_attribute() and remove_attribute() on the same tag when the removal extends back to the end of the tag name, which is where new attributes are inserted. These tests currently fail. --- .../tests/html-api/wpHtmlTagProcessor.php | 119 ++++++++++++++++-- 1 file changed, 108 insertions(+), 11 deletions(-) diff --git a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php index 89e197d7355ec..40761c6a3f0af 100644 --- a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php +++ b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php @@ -1596,25 +1596,33 @@ public static function data_html_with_duplicated_attributes() { } /** - * Ensures that removing an attribute does not introduce a self-closing flag. + * Ensures that removing an attribute does not change anything else about the tag. * - * A `/` inside a tag is only a self-closing flag when immediately followed by `>`. - * Anywhere else it's ignored as a separator. Removing an attribute that was - * directly preceded by a `/` could leave that `/` adjacent to the `>`, which - * would change the meaning of the tag, e.g. `` becoming ``. + * Inside a tag, `/` is ignored unless immediately followed by `>`, where it's the + * self-closing flag; otherwise it merely separates attributes. Removing only the + * attribute's own span can leave a stray `/` which changes the meaning of the tag, + * e.g. `` becoming ``. Conversely, removing too much can merge the + * tag name or a neighboring attribute into whatever follows, e.g. `` + * becoming ``. * * @covers WP_HTML_Tag_Processor::remove_attribute * - * @dataProvider data_remove_attribute_preserves_self_closing_flag + * @dataProvider data_remove_attribute_preserves_tag_semantics * - * @param string $html HTML containing a tag whose attribute will be removed. + * @param string $html HTML containing a G tag whose attribute will be removed. * @param string $attribute_to_remove Name of the attribute to remove. * @param string $expected Expected HTML after removing the attribute. */ - public function test_remove_attribute_preserves_self_closing_flag( string $html, string $attribute_to_remove, string $expected ) { + public function test_remove_attribute_preserves_tag_semantics( string $html, string $attribute_to_remove, string $expected ) { $processor = new WP_HTML_Tag_Processor( $html ); $processor->next_tag( 'G' ); $had_self_closing_flag = $processor->has_self_closing_flag(); + $expected_attributes = array(); + foreach ( $processor->get_attribute_names_with_prefix( '' ) as $name ) { + if ( strtolower( $attribute_to_remove ) !== $name ) { + $expected_attributes[ $name ] = $processor->get_attribute( $name ); + } + } $this->assertTrue( $processor->remove_attribute( $attribute_to_remove ), 'Failed to remove the attribute.' ); $updated_html = $processor->get_updated_html(); @@ -1622,8 +1630,9 @@ public function test_remove_attribute_preserves_self_closing_flag( string $html, // Re-parse the output to confirm the tag's semantics are unchanged. $processor = new WP_HTML_Tag_Processor( $updated_html ); - $processor->next_tag( 'G' ); - $this->assertNull( $processor->get_attribute( $attribute_to_remove ), 'Attribute still present in the updated HTML.' ); + $processor->next_tag(); + $processor->next_tag(); + $this->assertSame( 'G', $processor->get_tag(), 'Removing the attribute changed the tag name.' ); $this->assertSame( $had_self_closing_flag, $processor->has_self_closing_flag(), @@ -1631,6 +1640,13 @@ public function test_remove_attribute_preserves_self_closing_flag( string $html, ? 'Removing the attribute dropped the self-closing flag.' : 'Removing the attribute introduced a self-closing flag.' ); + $actual_attributes = array(); + foreach ( $processor->get_attribute_names_with_prefix( '' ) as $name ) { + $actual_attributes[ $name ] = $processor->get_attribute( $name ); + } + $this->assertSame( $expected_attributes, $actual_attributes, 'Removing the attribute changed the other attributes.' ); + $processor->next_token(); + $this->assertSame( 'ok', $processor->get_modifiable_text(), 'Removing the attribute changed the content following the tag.' ); } /** @@ -1638,7 +1654,7 @@ public function test_remove_attribute_preserves_self_closing_flag( string $html, * * @return array[] */ - public static function data_remove_attribute_preserves_self_closing_flag() { + public static function data_remove_attribute_preserves_tag_semantics() { return array( 'Slash before attribute' => array( 'ok', 'attr', 'ok' ), 'Slash before attribute, no whitespace' => array( 'ok', 'attr', 'ok' ), @@ -1648,9 +1664,90 @@ public static function data_remove_attribute_preserves_self_closing_flag() { 'Slash after quoted value, remove last' => array( 'ok', 'b', 'ok' ), 'Slash before duplicated attributes' => array( 'ok', 'a', 'ok' ), 'Slash and whitespace before attribute' => array( 'ok', 'attr', 'ok' ), + 'Slash inside unquoted value' => array( 'ok', 'attr', 'ok' ), + 'Slash ending unquoted value' => array( 'ok', 'attr', 'ok' ), + 'Slash ending unquoted value, remove other' => array( 'ok', 'b', 'ok' ), 'Self-closing flag is preserved' => array( 'ok', 'attr', 'ok' ), 'Self-closing flag after quoted value' => array( 'ok', 'attr', 'ok' ), 'Self-closing flag, slash before attribute' => array( 'ok', 'attr', 'ok' ), + 'Attribute directly after quoted value' => array( 'ok', 'a', 'ok' ), + 'Attribute directly after quoted value, middle' => array( 'ok', 'a', 'ok' ), + 'Equals directly after quoted value' => array( 'ok', 'a', 'ok' ), + 'Quote directly after quoted value' => array( 'ok', 'a', 'ok' ), + 'Slash directly after quoted value' => array( 'ok', 'a', 'ok' ), + 'Whitespace after quoted value' => array( 'ok', 'a', 'ok' ), + ); + } + + /** + * Ensures that attributes added and removed on the same tag don't interfere + * when the removal extends all the way back to the end of the tag name. + * + * New attributes are inserted directly after the tag name, which is also where + * a removal starts when the removed attribute directly follows the tag name + * with only `/` between them, e.g. ``. + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * @covers WP_HTML_Tag_Processor::set_attribute + * + * @dataProvider data_set_and_remove_attribute_at_tag_name_end + * + * @param string $html HTML containing a G tag. + * @param array $ops Operations to apply, each a method name followed by its arguments. + * @param string $expected Expected HTML after applying the operations. + */ + public function test_set_and_remove_attribute_at_tag_name_end( string $html, array $ops, string $expected ) { + $processor = new WP_HTML_Tag_Processor( $html ); + $processor->next_tag( 'G' ); + foreach ( $ops as $op ) { + $method = array_shift( $op ); + $processor->$method( ...$op ); + } + $this->assertSame( $expected, $processor->get_updated_html() ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_set_and_remove_attribute_at_tag_name_end() { + return array( + 'Set then remove' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + ), + 'Remove then set' => array( + 'ok', + array( array( 'remove_attribute', 'a' ), array( 'set_attribute', 'b', 'x' ) ), + 'ok', + ), + 'Set two then remove' => array( + 'ok', + array( array( 'set_attribute', 'c', 'y' ), array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + ), + 'Add class then remove' => array( + 'ok', + array( array( 'add_class', 'c' ), array( 'remove_attribute', 'a' ) ), + 'ok', + ), + 'Set then remove, self-closing' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + ), + 'Set then remove, attribute follows' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + ), + 'Set, remove both' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ), array( 'remove_attribute', 'c' ) ), + 'ok', + ), ); } From 6495adf9fde7b7119e43d91341137f0b26f81370 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 21 Aug 2026 08:37:05 +0400 Subject: [PATCH 4/5] HTML API: Keep neighboring syntax intact when removing slashes with an attribute. Consuming the `/` characters before a removed attribute introduced two problems of its own: 1. Only a quoted attribute value can be directly followed by another attribute, e.g. ``. Removing `/a="x"` outright merged the tag name with the following attribute: ``. Likewise `` became ``. When the consumed slashes were the only separator and an attribute follows, a single space now takes their place. 2. When the removed attribute directly follows the tag name, e.g. ``, the removal now starts at the same offset where new attributes are inserted by set_attribute(). Lexical updates were sorted by start and then by text, which ordered the removal (empty text) before the insertion; applying the removal first advanced the copy cursor past the insertion point and produced corrupted output such as ` b="x"/a>`. Zero-length insertions now sort before consuming replacements at the same offset. --- .../html-api/class-wp-html-tag-processor.php | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/html-api/class-wp-html-tag-processor.php b/src/wp-includes/html-api/class-wp-html-tag-processor.php index ba60870d620a5..25dd7ef2f60bb 100644 --- a/src/wp-includes/html-api/class-wp-html-tag-processor.php +++ b/src/wp-includes/html-api/class-wp-html-tag-processor.php @@ -2769,6 +2769,19 @@ private static function sort_start_ascending( WP_HTML_Text_Replacement $a, WP_HT return $by_start; } + /* + * An insertion (a replacement of zero length) must be applied before + * any replacement which starts at the same offset but consumes bytes. + * Otherwise the consuming replacement would advance the cursor past + * the insertion point before the insertion is applied. + * + * For example, new attributes are inserted directly after the tag name, + * and removing the attribute in `` also starts there. + */ + if ( ( 0 === $a->length ) !== ( 0 === $b->length ) ) { + return 0 === $a->length ? -1 : 1; + } + $by_text = isset( $a->text, $b->text ) ? strcmp( $a->text, $b->text ) : 0; if ( 0 !== $by_text ) { return $by_text; @@ -4816,6 +4829,22 @@ public function remove_attribute( $name ): bool { * Had only `attr` been removed the result would have been ``, which has a * self-closing flag that wasn't in the input. * + * When those `/` characters are the only thing separating the attribute from what + * precedes it, and another attribute immediately follows, a space is left in their + * place so that the surrounding syntax doesn't merge. + * + * Example: + * + * + * ^--------^ + * start end + * replacement: ` ` + * + * Result: + * + * Had the `/` been removed outright the result would have been ``, changing + * the tag name. + * * @since 7.2.0 * * @param WP_HTML_Attribute_Token|WP_HTML_Span $attribute Attribute to remove, or the span it occupies. @@ -4834,7 +4863,20 @@ private function get_attribute_removal( $attribute ): WP_HTML_Text_Replacement { --$start; } - return new WP_HTML_Text_Replacement( $start, $end - $start, '' ); + /* + * Only a quoted attribute value can be directly followed by another attribute, + * e.g. ``. If the `/` separating the removed attribute from the tag + * name or from a preceding attribute is consumed, then a space must take its + * place to keep the tag name or preceding attribute from merging with the + * following one, e.g. ``. + */ + $needs_separator = ( + $start !== $attribute->start && + isset( $this->html[ $end ] ) && + 0 === strspn( $this->html, " \t\f\r\n/>", $end, 1 ) + ); + + return new WP_HTML_Text_Replacement( $start, $end - $start, $needs_separator ? ' ' : '' ); } /** From 0da5474baf44e1db689d064ec6c0d1953554f916 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Fri, 21 Aug 2026 09:14:04 +0400 Subject: [PATCH 5/5] HTML API: Add return types and array shapes to new remove_attribute() tests. --- .../tests/html-api/wpHtmlTagProcessor.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php index 40761c6a3f0af..6c0ea4e6dba26 100644 --- a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php +++ b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php @@ -1613,7 +1613,7 @@ public static function data_html_with_duplicated_attributes() { * @param string $attribute_to_remove Name of the attribute to remove. * @param string $expected Expected HTML after removing the attribute. */ - public function test_remove_attribute_preserves_tag_semantics( string $html, string $attribute_to_remove, string $expected ) { + public function test_remove_attribute_preserves_tag_semantics( string $html, string $attribute_to_remove, string $expected ): void { $processor = new WP_HTML_Tag_Processor( $html ); $processor->next_tag( 'G' ); $had_self_closing_flag = $processor->has_self_closing_flag(); @@ -1652,9 +1652,9 @@ public function test_remove_attribute_preserves_tag_semantics( string $html, str /** * Data provider. * - * @return array[] + * @return array */ - public static function data_remove_attribute_preserves_tag_semantics() { + public static function data_remove_attribute_preserves_tag_semantics(): array { return array( 'Slash before attribute' => array( 'ok', 'attr', 'ok' ), 'Slash before attribute, no whitespace' => array( 'ok', 'attr', 'ok' ), @@ -1692,11 +1692,11 @@ public static function data_remove_attribute_preserves_tag_semantics() { * * @dataProvider data_set_and_remove_attribute_at_tag_name_end * - * @param string $html HTML containing a G tag. - * @param array $ops Operations to apply, each a method name followed by its arguments. - * @param string $expected Expected HTML after applying the operations. + * @param string $html HTML containing a G tag. + * @param list $ops Operations to apply, each a method name followed by its arguments. + * @param string $expected Expected HTML after applying the operations. */ - public function test_set_and_remove_attribute_at_tag_name_end( string $html, array $ops, string $expected ) { + public function test_set_and_remove_attribute_at_tag_name_end( string $html, array $ops, string $expected ): void { $processor = new WP_HTML_Tag_Processor( $html ); $processor->next_tag( 'G' ); foreach ( $ops as $op ) { @@ -1709,9 +1709,9 @@ public function test_set_and_remove_attribute_at_tag_name_end( string $html, arr /** * Data provider. * - * @return array[] + * @return array, string}> */ - public static function data_set_and_remove_attribute_at_tag_name_end() { + public static function data_set_and_remove_attribute_at_tag_name_end(): array { return array( 'Set then remove' => array( 'ok',