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 545af0650386b..3e9aa8640c77e 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 @@ -2796,8 +2796,11 @@ private function get_enqueued_attribute_value( string $comparable_name ) { $enqueued_text = $this->lexical_updates[ $comparable_name ]->text; - // Removed attributes erase the entire span. - if ( '' === $enqueued_text ) { + /* + * Removed attributes erase the entire span, unless a space is needed + * to prevent a preceding slash from becoming a self-closing flag. + */ + if ( '' === $enqueued_text || ' ' === $enqueued_text ) { return null; } @@ -2991,7 +2994,8 @@ public function get_attribute_names_with_prefix( $prefix ): ?array { continue; } - if ( '' === $update->text ) { + // A removal replaces the attribute with nothing, or with a space when a slash precedes it. + if ( '' === $update->text || ' ' === $update->text ) { $removals[ $update_name ] = true; } elseif ( ! isset( $this->attributes[ $update_name ] ) && str_starts_with( $update_name, $comparable ) ) { $additions[] = $update_name; @@ -4798,20 +4802,62 @@ public function remove_attribute( $name ): bool { * replacement: `` * * Result:
+ * + * Inside a tag, `/` is ignored unless `>` immediately follows it, which + * makes it the self-closing flag. A `/` directly before an attribute is + * always one of these ignored separators: tag names and attribute names + * end at `/`, and when a `/` appears in an unquoted value, the characters + * after it belong to the value, not to a new attribute. Removing the + * attribute could leave that `/` directly before the `>`, so a space + * replaces the attribute instead. + * + * Example – remove the attribute attr from : + * + * ^--^ + * replacement: ` ` + * + * Result: + * + * get_enqueued_attribute_value() and get_attribute_names_with_prefix() + * read a replacement of a single space as a removal. */ + $attribute_token = $this->attributes[ $name ]; + $replacement = '/' === $this->html[ $attribute_token->start - 1 ] ? ' ' : ''; + $this->lexical_updates[ $name ] = new WP_HTML_Text_Replacement( - $this->attributes[ $name ]->start, - $this->attributes[ $name ]->length, - '' + $attribute_token->start, + $attribute_token->length, + $replacement ); + $duplicate_attributes = $this->duplicate_attributes[ $name ] ?? array(); + $duplicates_already_removed = false; + + /* + * Duplicate removals are always enqueued as a batch. If the first is + * already present, all are present, and none should be enqueued again. + */ + if ( count( $duplicate_attributes ) > 0 ) { + $first_duplicate = $duplicate_attributes[0]; + foreach ( $this->lexical_updates as $update ) { + if ( $first_duplicate->start === $update->start && $first_duplicate->length === $update->length ) { + $duplicates_already_removed = true; + break; + } + } + } + // 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, - '' - ); + if ( ! $duplicates_already_removed ) { + foreach ( $duplicate_attributes as $attribute_token ) { + $replacement = '/' === $this->html[ $attribute_token->start - 1 ] ? ' ' : ''; + + $this->lexical_updates[] = new WP_HTML_Text_Replacement( + $attribute_token->start, + $attribute_token->length, + $replacement + ); + } } return true; diff --git a/tests/phpunit/tests/html-api/wpHtmlProcessor.php b/tests/phpunit/tests/html-api/wpHtmlProcessor.php index 58e7ef18cba41..60ea50485027f 100644 --- a/tests/phpunit/tests/html-api/wpHtmlProcessor.php +++ b/tests/phpunit/tests/html-api/wpHtmlProcessor.php @@ -608,6 +608,60 @@ public function test_unquoted_slash_attribute_does_not_self_close_foreign_conten ); } + /** + * Ensures that removing attributes preserves foreign-content tree semantics. + * + * @ticket 65967 + * + * @covers ::remove_attribute + * + * @dataProvider data_remove_attribute_preserves_foreign_content_tree + * + * @param string $html HTML containing the attribute to remove. + * @param string $tag_name Foreign element tag name to modify. + * @param bool $expected_has_self_closing_flag Expected self-closing flag state. + * @param bool $expected_expects_closer Expected closer expectation. + * @param array $expected_text_breadcrumbs Expected breadcrumbs for the following text. + */ + public function test_remove_attribute_preserves_foreign_content_tree( $html, $tag_name, $expected_has_self_closing_flag, $expected_expects_closer, $expected_text_breadcrumbs ) { + $processor = WP_HTML_Processor::create_fragment( $html ); + $this->assertTrue( $processor->next_tag( $tag_name ), "Failed to find the {$tag_name} tag: check test setup." ); + $this->assertTrue( $processor->remove_attribute( 'attr' ), 'Could not remove the target attribute.' ); + + $processor = WP_HTML_Processor::create_fragment( $processor->get_updated_html() ); + $this->assertTrue( $processor->next_tag( $tag_name ), "Failed to find the updated {$tag_name} tag." ); + $this->assertNull( $processor->get_attribute( 'attr' ), 'The updated tag retained the removed attribute.' ); + $this->assertSame( $expected_has_self_closing_flag, $processor->has_self_closing_flag(), 'Removing the attribute changed the self-closing flag state.' ); + $this->assertSame( $expected_expects_closer, $processor->expects_closer(), 'Removing the attribute changed the closer expectation.' ); + + $this->assertTrue( $processor->next_token(), 'Failed to find text following the updated tag.' ); + $this->assertSame( $expected_text_breadcrumbs, $processor->get_breadcrumbs(), 'Removing the attribute changed the text node ancestry.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_remove_attribute_preserves_foreign_content_tree() { + return array( + 'SVG element without self-closing flag' => array( + 'inside', + 'G', + false, + true, + array( 'HTML', 'BODY', 'SVG', 'G', '#text' ), + ), + 'MathML element with self-closing flag' => array( + 'outside', + 'MI', + true, + false, + array( 'HTML', 'BODY', 'MATH', '#text' ), + ), + ); + } + /** * Ensures that expects_closer works for void-like elements in foreign content. * diff --git a/tests/phpunit/tests/html-api/wpHtmlTagProcessor-bookmark.php b/tests/phpunit/tests/html-api/wpHtmlTagProcessor-bookmark.php index beb4cc60f6d95..625dfdead79dc 100644 --- a/tests/phpunit/tests/html-api/wpHtmlTagProcessor-bookmark.php +++ b/tests/phpunit/tests/html-api/wpHtmlTagProcessor-bookmark.php @@ -181,6 +181,31 @@ public function test_removing_long_attributes_doesnt_break_seek() { ); } + /** + * @ticket 65967 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * @covers WP_HTML_Tag_Processor::seek + * @covers WP_HTML_Tag_Processor::set_bookmark + */ + public function test_repeated_duplicate_attribute_removal_does_not_break_following_bookmark() { + $processor = new WP_HTML_Tag_Processor( '
' ); + $this->assertTrue( $processor->next_tag( 'div' ), 'Could not find the DIV tag: check test setup.' ); + $this->assertTrue( $processor->set_bookmark( 'div' ), 'Could not bookmark the DIV tag.' ); + $this->assertTrue( $processor->next_tag( 'path' ), 'Could not find the PATH tag: check test setup.' ); + $this->assertTrue( $processor->set_bookmark( 'path' ), 'Could not bookmark the PATH tag.' ); + + $this->assertTrue( $processor->seek( 'div' ), 'Could not seek back to the DIV tag.' ); + $this->assertTrue( $processor->remove_attribute( 'a' ), 'Could not remove the duplicated attribute.' ); + $this->assertTrue( $processor->remove_attribute( 'a' ), 'Could not remove the duplicated attribute again.' ); + $this->assertNull( $processor->get_attribute( 'a' ), 'The duplicated attribute was not removed.' ); + $processor->get_updated_html(); + + $this->assertTrue( $processor->seek( 'path' ), 'Could not seek to the PATH tag after removing the attributes.' ); + $this->assertSame( 'PATH', $processor->get_tag(), 'The PATH bookmark no longer points to its tag.' ); + $this->assertSame( 'x', $processor->get_attribute( 'id' ), 'The PATH bookmark points to the wrong tag.' ); + } + /** * @ticket 56299 * diff --git a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php index 1bfaa01bf0453..21631b251fafc 100644 --- a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php +++ b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php @@ -560,6 +560,38 @@ public function test_get_attribute_names_with_prefix_immediately_reflects_remove ); } + /** + * Ensures that an attribute removed via remove_attribute() is no longer reported + * by get_attribute_names_with_prefix() when a slash precedes the attribute. + * + * @ticket 65967 + * + * @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix + */ + public function test_get_attribute_names_with_prefix_immediately_reflects_removed_attribute_after_slash() { + $processor = new WP_HTML_Tag_Processor( '' ); + $processor->next_tag(); + + $this->assertSame( + array( 'data-x' ), + $processor->get_attribute_names_with_prefix( 'data-' ), + 'Failed to report the existing attribute: check test setup.' + ); + + $processor->remove_attribute( 'data-x' ); + + $this->assertNull( + $processor->get_attribute( 'data-x' ), + 'Expected get_attribute() to report the removed attribute as missing.' + ); + + $this->assertSame( + array(), + $processor->get_attribute_names_with_prefix( 'data-' ), + 'Expected no custom data attributes after removing the only one.' + ); + } + /** * Ensures that when the `class` attribute is newly added via add_class(), that * it’s reported by get_attribute_names_with_prefix() immediately after being added. @@ -1540,6 +1572,238 @@ public function test_remove_attribute_with_an_existing_attribute_name_removes_it ); } + /** + * @ticket 65967 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * + * @dataProvider data_remove_attribute_preserves_self_closing_flag_state + * + * @param string $html HTML containing the attribute to remove. + * @param int $removal_count Number of times to remove the attribute. + * @param string $expected Expected HTML after removing the attribute. + * @param bool $expected_has_self_closing_flag Expected self-closing flag state. + */ + public function test_remove_attribute_preserves_self_closing_flag_state( $html, $removal_count, $expected, $expected_has_self_closing_flag ) { + $processor = new WP_HTML_Tag_Processor( $html ); + $processor->next_tag( 'g' ); + $this->assertSame( $expected_has_self_closing_flag, $processor->has_self_closing_flag(), 'Test setup has the wrong self-closing flag state.' ); + + for ( $i = 0; $i < $removal_count; $i++ ) { + $processor->remove_attribute( 'attr' ); + $this->assertNull( $processor->get_attribute( 'attr' ) ); + } + + $this->assertSame( $expected, $processor->get_updated_html() ); + $this->assertSame( $expected_has_self_closing_flag, $processor->has_self_closing_flag(), 'Removing the attribute changed the self-closing flag state.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_remove_attribute_preserves_self_closing_flag_state() { + return array( + 'Attribute preceded by a slash' => array( 'ok', 1, 'ok', false ), + 'Attribute before self-closing flag' => array( 'ok', 1, 'ok', true ), + 'Duplicate attribute removed twice' => array( 'ok', 2, 'ok', false ), + ); + } + + /** + * Ensures that removing an attribute does not change anything else about the tag. + * + * 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 ``. + * + * @ticket 65967 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * + * @dataProvider data_remove_attribute_preserves_tag_semantics + * + * @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_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(); + $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(); + $this->assertSame( $expected, $updated_html, 'Updated HTML did not match the expected output.' ); + + $this->assert_svg_g_tag_semantics( $updated_html, $had_self_closing_flag, $expected_attributes ); + } + + /** + * Data provider. + * + * @return array + */ + 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' ), + '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' ), + '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 only `/` separates the removed attribute from the tag name. + * + * New attributes are inserted directly after the tag name, so in `` + * the insertion and the removal are separated only by the `/`. + * + * @ticket 65967 + * + * @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 list $ops Operations to apply, each a method name followed by its arguments. + * @param string $expected Expected HTML after applying the operations. + * @param array $expected_attributes Expected attributes on the updated G tag. + */ + public function test_set_and_remove_attribute_at_tag_name_end( string $html, array $ops, string $expected, array $expected_attributes ): void { + $processor = new WP_HTML_Tag_Processor( $html ); + $processor->next_tag( 'G' ); + $had_self_closing_flag = $processor->has_self_closing_flag(); + foreach ( $ops as $op ) { + $method = array_shift( $op ); + $processor->$method( ...$op ); + } + + $updated_html = $processor->get_updated_html(); + $this->assertSame( $expected, $updated_html, 'Updated HTML did not match the expected output.' ); + + $this->assert_svg_g_tag_semantics( $updated_html, $had_self_closing_flag, $expected_attributes ); + } + + /** + * Data provider. + * + * @return array, string, array}> + */ + public static function data_set_and_remove_attribute_at_tag_name_end(): array { + return array( + 'Set then remove' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + array( 'b' => 'x' ), + ), + 'Remove then set' => array( + 'ok', + array( array( 'remove_attribute', 'a' ), array( 'set_attribute', 'b', 'x' ) ), + 'ok', + array( 'b' => 'x' ), + ), + 'Set two then remove' => array( + 'ok', + array( array( 'set_attribute', 'c', 'y' ), array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + array( + 'b' => 'x', + 'c' => 'y', + ), + ), + 'Add class then remove' => array( + 'ok', + array( array( 'add_class', 'c' ), array( 'remove_attribute', 'a' ) ), + 'ok', + array( 'class' => 'c' ), + ), + 'Set then remove, self-closing' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + array( 'b' => 'x' ), + ), + 'Set then remove, attribute follows' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ) ), + 'ok', + array( + 'b' => 'x', + 'c' => true, + ), + ), + 'Set, remove both' => array( + 'ok', + array( array( 'set_attribute', 'b', 'x' ), array( 'remove_attribute', 'a' ), array( 'remove_attribute', 'c' ) ), + 'ok', + array( 'b' => 'x' ), + ), + ); + } + + /** + * Asserts that the tag following SVG in the given HTML is a G tag with the + * expected self-closing flag and attributes, followed by the text "ok". + * + * @param string $html HTML of the form `ok`. + * @param bool $expected_has_self_closing_flag Expected self-closing flag state. + * @param array $expected_attributes Expected attribute values by name. + */ + private function assert_svg_g_tag_semantics( string $html, bool $expected_has_self_closing_flag, array $expected_attributes ): void { + $processor = new WP_HTML_Tag_Processor( $html ); + $processor->next_tag(); + $processor->next_tag(); + $this->assertSame( 'G', $processor->get_tag(), 'The update changed the tag name.' ); + $this->assertSame( + $expected_has_self_closing_flag, + $processor->has_self_closing_flag(), + $expected_has_self_closing_flag + ? 'The update dropped the self-closing flag.' + : 'The update introduced a self-closing flag.' + ); + + $actual_attributes = array(); + foreach ( $processor->get_attribute_names_with_prefix( '' ) as $name ) { + $actual_attributes[ $name ] = $processor->get_attribute( $name ); + } + ksort( $expected_attributes ); + ksort( $actual_attributes ); + $this->assertSame( $expected_attributes, $actual_attributes, 'The updated tag has the wrong attributes.' ); + + $processor->next_token(); + $this->assertSame( 'ok', $processor->get_modifiable_text(), 'The update changed the content following the tag.' ); + } + /** * @ticket 58119 *