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 ace3e14bea565..4c109d34e747b 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 @@ -791,6 +791,12 @@ class WP_HTML_Tag_Processor { * boundaries, however, so these should never be exposed outside of this * class or any classes which intentionally expand its functionality. * + * Any code which creates these lexical updates must also ensure that no + * two updates replace overlapping spans of the document: the cursor and + * bookmark position accounting when applying the updates assumes that + * every update replaces a distinct span. Only zero-length insertions may + * share their offset with another update's span. + * * These are enqueued while editing the document instead of being immediately * applied to avoid processing overhead, string allocations, and string * copies when applying many updates to a single document. @@ -2629,9 +2635,26 @@ private function apply_attributes_updates( int $shift_this_point ): int { $accumulated_shift_for_given_point += $shift; } - $output_buffer .= substr( $this->html, $bytes_already_copied, $diff->start - $bytes_already_copied ); + /* + * Updates may share a document offset, e.g. a new attribute inserted + * at the end of the tag name and a removed attribute whose span starts + * there because it was separated from the tag name by only solidus + * characters. The copied-bytes cursor must never move backward, or + * already-replaced spans of the document would be copied again. + * + * Only zero-length insertions may share their offset with another + * update's span; the sort places them after a removal of that span, + * so the inserted text lands where the removed span was. Updates + * with positive length must never overlap each other: the cursor + * and bookmark position accounting below assume that every update + * replaces a distinct span of the document. + */ + if ( $diff->start > $bytes_already_copied ) { + $output_buffer .= substr( $this->html, $bytes_already_copied, $diff->start - $bytes_already_copied ); + } + $output_buffer .= $diff->text; - $bytes_already_copied = $diff->start + $diff->length; + $bytes_already_copied = max( $bytes_already_copied, $diff->start + $diff->length ); } $this->html = $output_buffer . substr( $this->html, $bytes_already_copied ); @@ -4745,17 +4768,32 @@ public function remove_attribute( $name ): bool { * * Result:
*/ - $this->lexical_updates[ $name ] = new WP_HTML_Text_Replacement( + $removal_spans = array(); + $removal_spans[] = $this->get_attribute_removal_span( $this->attributes[ $name ]->start, - $this->attributes[ $name ]->length, + $this->attributes[ $name ]->length + ); + foreach ( $this->duplicate_attributes[ $name ] ?? array() as $attribute_token ) { + $removal_spans[] = $this->get_attribute_removal_span( + $attribute_token->start, + $attribute_token->length + ); + } + + $this->supersede_updates_in_removal_spans( $removal_spans ); + + $this->lexical_updates[ $name ] = new WP_HTML_Text_Replacement( + $removal_spans[0]->start, + $removal_spans[0]->length, '' ); // Removes any duplicated attributes if they were also present. - foreach ( $this->duplicate_attributes[ $name ] ?? array() as $attribute_token ) { + $removal_span_count = count( $removal_spans ); + for ( $i = 1; $i < $removal_span_count; $i++ ) { $this->lexical_updates[] = new WP_HTML_Text_Replacement( - $attribute_token->start, - $attribute_token->length, + $removal_spans[ $i ]->start, + $removal_spans[ $i ]->length, '' ); } @@ -4763,6 +4801,93 @@ public function remove_attribute( $name ): bool { return true; } + /** + * Removes enqueued lexical updates replacing spans within removed spans. + * + * Removing spans of the document supersedes updates already enqueued + * over intersecting spans: removal replaces the entire span, and no two + * updates may replace overlapping spans of the document, since bookmark + * and cursor position accounting assumes that every update replaces a + * distinct span. This also ensures that each removal is enqueued exactly + * once, no matter how many times an attribute is removed. + * + * Zero-length insertions replace no spans of the document and are + * unaffected; they may share their offset with a removed span. + * + * The given spans must be sorted by starting position and must not + * overlap each other, as is the case for attribute spans, which appear + * in document order. Each enqueued update is then checked against the + * spans with a single binary search: an update can only intersect the + * last span starting before the update's end. + * + * @since 7.1.0 + * + * @param WP_HTML_Span[] $removal_spans Non-overlapping spans of the document being + * removed, sorted by starting position. + */ + private function supersede_updates_in_removal_spans( array $removal_spans ): void { + $span_count = count( $removal_spans ); + + foreach ( $this->lexical_updates as $key => $update ) { + if ( 0 === $update->length ) { + continue; + } + + $update_end = $update->start + $update->length; + + // Find the last removal span starting before the end of the update. + $candidate = null; + $low = 0; + $high = $span_count - 1; + while ( $low <= $high ) { + $probe = intdiv( $low + $high, 2 ); + if ( $removal_spans[ $probe ]->start < $update_end ) { + $candidate = $removal_spans[ $probe ]; + $low = $probe + 1; + } else { + $high = $probe - 1; + } + } + + if ( null !== $candidate && $update->start < $candidate->start + $candidate->length ) { + unset( $this->lexical_updates[ $key ] ); + } + } + } + + /** + * Returns the full span of the document to remove for an attribute. + * + * Solidus characters ("/") may precede an attribute name, where they act + * like attribute separators and are not part of any syntax token. Such + * solidus characters must be removed with the attribute; otherwise a + * remaining solidus directly before the tag-closing ">" would become a + * self-closing flag and change the meaning of the surrounding HTML. + * + * Example: + * + * inside g + * + * Removing only `attr` would produce `inside g`, where + * the self-closing G element no longer contains the text that follows. + * + * @since 7.1.0 + * + * @param int $start Byte offset into the document where the attribute starts. + * @param int $length Byte length of the attribute. + * @return WP_HTML_Span Span of the document to remove for the attribute. + */ + private function get_attribute_removal_span( int $start, int $length ): WP_HTML_Span { + $tag_name_ends_at = $this->tag_name_starts_at + $this->tag_name_length; + + while ( $start > $tag_name_ends_at && '/' === $this->html[ $start - 1 ] ) { + --$start; + ++$length; + } + + return new WP_HTML_Span( $start, $length ); + } + /** * Adds a new class name to the currently matched tag. * diff --git a/tests/phpunit/tests/html-api/wpHtmlProcessor.php b/tests/phpunit/tests/html-api/wpHtmlProcessor.php index f3b051ca3639e..fd6f4ba0050ea 100644 --- a/tests/phpunit/tests/html-api/wpHtmlProcessor.php +++ b/tests/phpunit/tests/html-api/wpHtmlProcessor.php @@ -608,6 +608,77 @@ public function test_unquoted_slash_attribute_does_not_self_close_foreign_conten ); } + /** + * Ensures that removing an attribute does not change the structure of the document. + * + * A solidus before an attribute name is ignored when parsing, but if removing + * the attribute left the solidus directly before the tag-closing ">", the tag + * would gain a self-closing flag. In foreign content this changes whether the + * element contains the content that follows it. + * + * @ticket 65372 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + */ + public function test_remove_attribute_does_not_self_close_foreign_content(): void { + $processor = WP_HTML_Processor::create_fragment( 'ok' ); + + $this->assertTrue( $processor->next_tag( 'G' ), 'Failed to find the G tag: check test setup.' ); + $this->assertFalse( $processor->has_self_closing_flag(), 'The G tag must not have a self-closing flag: check test setup.' ); + $this->assertTrue( $processor->remove_attribute( 'attr' ), 'Failed to remove the attribute.' ); + + $updated_html = $processor->get_updated_html(); + $this->assertSame( 'ok', $updated_html, 'Failed to remove the attribute with its preceding solidus.' ); + + $processor = WP_HTML_Processor::create_fragment( $updated_html ); + $this->assertTrue( $processor->next_tag( 'G' ), 'Failed to find the G tag in the updated HTML.' ); + $this->assertFalse( + $processor->has_self_closing_flag(), + 'Failed to prevent the G tag from becoming self-closing when removing the attribute.' + ); + + $this->assertTrue( $processor->next_token(), 'Failed to find text following the G tag in the updated HTML.' ); + $this->assertSame( + array( 'HTML', 'BODY', 'SVG', 'G', '#text' ), + $processor->get_breadcrumbs(), + 'Failed to keep text following the G tag inside the G element.' + ); + } + + /** + * Ensures that removing an attribute preserves a genuine self-closing flag + * and the resulting document structure in foreign content. + * + * @ticket 65372 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + */ + public function test_remove_attribute_preserves_self_closing_foreign_content(): void { + $processor = WP_HTML_Processor::create_fragment( 'ok' ); + + $this->assertTrue( $processor->next_tag( 'MI' ), 'Failed to find the MI tag: check test setup.' ); + $this->assertTrue( $processor->has_self_closing_flag(), 'The MI tag must have a self-closing flag: check test setup.' ); + $this->assertFalse( $processor->expects_closer(), 'The self-closing MI tag must not expect a closer: check test setup.' ); + $this->assertTrue( $processor->remove_attribute( 'attr' ), 'Failed to remove the attribute.' ); + + $updated_html = $processor->get_updated_html(); + $this->assertSame( 'ok', $updated_html, 'Removing the attribute produced unexpected HTML.' ); + + $processor = WP_HTML_Processor::create_fragment( $updated_html ); + $this->assertTrue( $processor->next_tag( 'MI' ), 'Failed to find the MI tag in the updated HTML.' ); + $this->assertTrue( + $processor->has_self_closing_flag(), + 'Failed to preserve the self-closing flag of the MI tag when removing the attribute.' + ); + + $this->assertTrue( $processor->next_token(), 'Failed to find text following the MI tag in the updated HTML.' ); + $this->assertSame( + array( 'HTML', 'BODY', 'MATH', '#text' ), + $processor->get_breadcrumbs(), + 'Failed to keep text following the self-closing MI tag outside the MI element.' + ); + } + /** * Ensures that expects_closer works for void-like elements in foreign content. * diff --git a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php index 84d90a84190fc..2731f6fe384d8 100644 --- a/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php +++ b/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php @@ -1332,6 +1332,237 @@ public function test_remove_attribute_with_an_existing_attribute_name_removes_it ); } + /** + * Ensures that removing an attribute does not change how the rest of the tag is parsed. + * + * Solidus characters ("/") may appear before attribute names, where they + * act like attribute separators and are not part of any syntax token. If + * removing an attribute leaves a solidus directly before the tag-closing + * ">", the tag would gain a self-closing flag, changing the meaning of + * the surrounding HTML. + * + * @ticket 65372 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * + * @dataProvider data_remove_attribute_preserves_self_closing_flag + * + * @param string $html HTML containing an "attr" attribute to remove. + * @param string $expected_html Expected HTML after removing the attribute. + */ + public function test_remove_attribute_preserves_self_closing_flag( $html, $expected_html ) { + $processor = new WP_HTML_Tag_Processor( $html ); + $this->assertTrue( $processor->next_tag(), 'Failed to find the tag: check test setup.' ); + $had_self_closing_flag = $processor->has_self_closing_flag(); + + $this->assertTrue( $processor->remove_attribute( 'attr' ), 'Failed to remove the attribute.' ); + + $updated_html = $processor->get_updated_html(); + $this->assertSame( $expected_html, $updated_html, 'Attribute removal produced unexpected HTML.' ); + + $processor = new WP_HTML_Tag_Processor( $updated_html ); + $this->assertTrue( $processor->next_tag(), 'Failed to find the tag in the updated HTML.' ); + $this->assertSame( + $had_self_closing_flag, + $processor->has_self_closing_flag(), + 'Attribute removal changed the self-closing flag of the tag.' + ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_remove_attribute_preserves_self_closing_flag() { + return array( + 'Solidus before the attribute' => array( 'ok', 'ok' ), + 'Multiple solidi before the attribute' => array( 'ok', 'ok' ), + 'Solidus after the tag name' => array( 'ok', 'ok' ), + 'Solidus before attribute with value' => array( 'ok', 'ok' ), + 'Solidus after another attribute' => array( 'ok', 'ok' ), + 'Solidus before duplicate attribute' => array( 'ok', 'ok' ), + 'Solidus separated from the attribute' => array( 'ok', 'ok' ), + 'Self-closing tag' => array( 'ok', 'ok' ), + 'Self-closing tag after solidus' => array( 'ok', 'ok' ), + ); + } + + /** + * Ensures that removing an attribute with its preceding solidus does not + * conflict with new attributes inserted at the end of the tag name. + * + * When a removed attribute is separated from the tag name by only solidus + * characters, the removed span starts at the end of the tag name, the same + * place where new attributes are inserted. The updates must apply + * correctly in either order of operations, and bookmark positions must + * reflect the applied updates. + * + * @ticket 65372 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * @covers WP_HTML_Tag_Processor::set_attribute + * @covers WP_HTML_Tag_Processor::add_class + * @covers WP_HTML_Tag_Processor::seek + * + * @dataProvider data_set_attribute_or_add_class_while_removing_solidus_separated_attribute + * + * @param string $method Method used to add an attribute: "set_attribute" or "add_class". + * @param string $order Order of operations: "add first" or "remove first". + * @param string $expected_html Expected HTML after the updates are applied. + */ + public function test_can_set_attribute_while_removing_solidus_separated_attribute( $method, $order, $expected_html ) { + $processor = new WP_HTML_Tag_Processor( 'ok' ); + $this->assertTrue( $processor->next_tag(), 'Failed to find the tag: check test setup.' ); + + if ( 'remove first' === $order ) { + $this->assertTrue( $processor->remove_attribute( 'attr' ), 'Failed to remove the attribute.' ); + } + + if ( 'set_attribute' === $method ) { + $processor->set_attribute( 'id', 'test' ); + } else { + $processor->add_class( 'test' ); + } + + if ( 'add first' === $order ) { + $this->assertTrue( $processor->remove_attribute( 'attr' ), 'Failed to remove the attribute.' ); + } + + $this->assertTrue( $processor->next_tag( 'path' ), 'Failed to find the PATH tag: check test setup.' ); + $this->assertTrue( $processor->set_bookmark( 'path' ), 'Failed to set a bookmark on the PATH tag.' ); + + $this->assertSame( $expected_html, $processor->get_updated_html(), 'Applying the updates produced unexpected HTML.' ); + + $this->assertTrue( $processor->seek( 'path' ), 'Failed to seek to the bookmark.' ); + $this->assertSame( 'PATH', $processor->get_tag(), 'Seeking to the bookmark landed on the wrong location in the document.' ); + $this->assertSame( 'x', $processor->get_attribute( 'id' ), 'Failed to find the attribute of the tag at the bookmark.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_set_attribute_or_add_class_while_removing_solidus_separated_attribute() { + return array( + 'set_attribute, then remove' => array( 'set_attribute', 'add first', 'ok' ), + 'remove, then set_attribute' => array( 'set_attribute', 'remove first', 'ok' ), + 'add_class, then remove' => array( 'add_class', 'add first', 'ok' ), + 'remove, then add_class' => array( 'add_class', 'remove first', 'ok' ), + ); + } + + /** + * Ensures that repeating an attribute removal does not corrupt bookmarks. + * + * Removing an attribute also enqueues removals for its duplicates. + * Repeating the removal must not enqueue those removals again: every + * enqueued update shifts bookmark positions and the internal cursor + * when updates are applied, so repeated updates for the same span + * would shift them more than the document actually changed. + * + * @ticket 65372 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * @covers WP_HTML_Tag_Processor::seek + * + * @dataProvider data_repeated_attribute_removal_preserves_bookmarks + * + * @param string $html HTML containing duplicate "a" attributes and a PATH tag. + * @param string $expected_html Expected HTML after removing the attribute. + */ + public function test_repeated_attribute_removal_preserves_bookmarks( $html, $expected_html ) { + $processor = new WP_HTML_Tag_Processor( $html ); + $this->assertTrue( $processor->next_tag(), 'Failed to find the first tag: check test setup.' ); + $this->assertTrue( $processor->remove_attribute( 'a' ), 'Failed to remove the attribute.' ); + $this->assertTrue( $processor->remove_attribute( 'a' ), 'Failed to remove the attribute again.' ); + + $this->assertTrue( $processor->next_tag( 'path' ), 'Failed to find the PATH tag: check test setup.' ); + $this->assertTrue( $processor->set_bookmark( 'path' ), 'Failed to set a bookmark on the PATH tag.' ); + + $this->assertSame( $expected_html, $processor->get_updated_html(), 'Removing the attribute twice produced unexpected HTML.' ); + + $this->assertTrue( $processor->seek( 'path' ), 'Failed to seek to the bookmark.' ); + $this->assertSame( 'PATH', $processor->get_tag(), 'Seeking to the bookmark landed on the wrong location in the document.' ); + $this->assertSame( 'x', $processor->get_attribute( 'id' ), 'Failed to find the attribute of the tag at the bookmark.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_repeated_attribute_removal_preserves_bookmarks() { + return array( + 'Duplicate attributes' => array( '
ok', '
ok' ), + 'Duplicate attribute after a solidus' => array( 'ok', 'ok' ), + ); + } + + /** + * Ensures that removing an attribute removes the entire spans of the + * attribute and its duplicates even when other lexical updates target + * spans within them. + * + * Exactly one update must replace each removed span. Bookmark positions + * and the internal cursor are shifted by every enqueued update when + * updates are applied: a missing removal leaves a duplicate in the + * document, and updates over intersecting spans shift positions more + * than the document actually changed. An update enqueued within a + * removed span by other means is superseded by the removal. + * + * @ticket 65372 + * + * @covers WP_HTML_Tag_Processor::remove_attribute + * @covers WP_HTML_Tag_Processor::seek + * + * @dataProvider data_updates_targeting_removed_attribute_spans + * + * @param string $html HTML containing "a" attributes and a PATH tag. + * @param int $update_start Byte offset of the enqueued update. + * @param int $update_length Byte length of the enqueued update. + * @param string $update_text Replacement text of the enqueued update. + * @param string $expected_html Expected HTML after removing the attribute. + */ + public function test_remove_attribute_removes_attribute_spans_when_other_updates_target_them( $html, $update_start, $update_length, $update_text, $expected_html ) { + $processor = new class($html) extends WP_HTML_Tag_Processor { + public function enqueue_replacement( int $start, int $length, string $text ): void { + $this->lexical_updates[] = new WP_HTML_Text_Replacement( $start, $length, $text ); + } + }; + + $this->assertTrue( $processor->next_tag(), 'Failed to find the tag: check test setup.' ); + + $processor->enqueue_replacement( $update_start, $update_length, $update_text ); + + $this->assertTrue( $processor->remove_attribute( 'a' ), 'Failed to remove the attribute.' ); + + $this->assertTrue( $processor->next_tag( 'path' ), 'Failed to find the PATH tag: check test setup.' ); + $this->assertTrue( $processor->set_bookmark( 'path' ), 'Failed to set a bookmark on the PATH tag.' ); + + $this->assertSame( $expected_html, $processor->get_updated_html(), 'Removing the attribute produced unexpected HTML.' ); + + $this->assertTrue( $processor->seek( 'path' ), 'Failed to seek to the bookmark.' ); + $this->assertSame( 'PATH', $processor->get_tag(), 'Seeking to the bookmark landed on the wrong location in the document.' ); + $this->assertSame( 'x', $processor->get_attribute( 'id' ), 'Failed to find the attribute of the tag at the bookmark.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_updates_targeting_removed_attribute_spans() { + return array( + 'Removal of a duplicate, exact span' => array( 'ok', 5, 1, '', 'ok' ), + 'Replacement of a duplicate, exact span' => array( 'ok', 5, 1, 'b', 'ok' ), + 'Removal within extended duplicate span' => array( 'ok', 6, 1, '', 'ok' ), + 'Replacement within extended duplicate span' => array( 'ok', 6, 1, 'b', 'ok' ), + 'Replacement within extended attribute span' => array( 'ok', 3, 1, 'b', 'ok' ), + ); + } + /** * @ticket 58119 *