Skip to content

HTML API: Avoid creating self-closing flags when removing attributes #12368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sirreal wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft

HTML API: Avoid creating self-closing flags when removing attributes #12368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2673ae4
HTML API: Avoid creating self-closing flags when removing attributes
sirreal Jun 30, 2026
6e6694f
HTML API: Consolidate self-closing attribute removal tests
sirreal Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 43 additions & 6 deletions src/wp-includes/html-api/class-wp-html-tag-processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2576,9 +2576,12 @@ 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 );
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 );
Expand Down Expand Up @@ -4544,24 +4547,58 @@ public function remove_attribute( $name ): bool {
*
* Result: <div />
*/
$this->lexical_updates[ $name ] = new WP_HTML_Text_Replacement(
$attribute_removal = $this->get_attribute_removal_span(
$this->attributes[ $name ]->start,
$this->attributes[ $name ]->length,
$this->attributes[ $name ]->length
);

$this->lexical_updates[ $name ] = new WP_HTML_Text_Replacement(
$attribute_removal->start,
$attribute_removal->length,
''
);

// 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_removal = $this->get_attribute_removal_span(
$attribute_token->start,
$attribute_token->length,
$attribute_token->length
);

$this->lexical_updates[] = new WP_HTML_Text_Replacement(
$attribute_removal->start,
$attribute_removal->length,
''
);
}

return true;
}

/**
* Returns the full span to remove for an attribute.
*
* Slash characters may be used as attribute separators, as in `<div a/b>`.
* If an attribute following those separators is removed without removing the
* slash, then the remaining slash can become a self-closing flag.
*
* @since 7.1.0
*
* @param int $start Byte offset where the attribute starts.
* @param int $length Byte length of the attribute.
* @return WP_HTML_Span Full span to remove.
*/
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.
*
Expand Down
68 changes: 68 additions & 0 deletions tests/phpunit/tests/html-api/wpHtmlProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,74 @@ public function test_unquoted_slash_attribute_does_not_self_close_foreign_conten
);
}

/**
* Ensures that removing attributes preserves foreign-content self-closing semantics.
*
* @covers ::remove_attribute
*
* @dataProvider data_remove_attribute_preserves_foreign_content_self_closing_semantics
*
* @param string $html HTML containing a foreign element with an attribute.
* @param string $tag_name Foreign element tag name to modify.
* @param string $attribute_to_remove Name of the attribute to remove.
* @param string $updated_html Expected updated HTML.
* @param string $normalized_html Expected normalized HTML after reparsing the updated HTML.
* @param bool $expected_has_self_closing_flag Expected self-closing flag state.
* @param bool $expected_expects_closer Expected closer expectation.
*/
public function test_remove_attribute_preserves_foreign_content_self_closing_semantics( $html, $tag_name, $attribute_to_remove, $updated_html, $normalized_html, $expected_has_self_closing_flag, $expected_expects_closer ) {
$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->assertSame(
$expected_has_self_closing_flag,
$processor->has_self_closing_flag(),
'Test setup has the wrong self-closing flag state.'
);
$this->assertSame(
$expected_expects_closer,
$processor->expects_closer(),
'Test setup has the wrong closer expectation.'
);

$this->assertTrue( $processor->remove_attribute( $attribute_to_remove ), 'Could not remove the target attribute.' );

$this->assertSame(
$updated_html,
$processor->get_updated_html(),
'Removing the attribute should preserve foreign-content self-closing semantics.'
);
$this->assertSame(
$expected_has_self_closing_flag,
$processor->has_self_closing_flag(),
'Removing the attribute should preserve the self-closing flag state.'
);
$this->assertSame(
$expected_expects_closer,
$processor->expects_closer(),
'Removing the attribute should preserve the closer expectation.'
);
$this->assertSame(
$normalized_html,
WP_HTML_Processor::normalize( $processor->get_updated_html() ),
'Updated HTML should preserve foreign-content self-closing semantics when parsed again.'
);
}

/**
* Data provider.
*
* @return array[]
*/
public static function data_remove_attribute_preserves_foreign_content_self_closing_semantics() {
return array(
'SVG G without self-closing flag' => array( '<svg><g /b></g></svg>', 'G', 'b', '<svg><g ></g></svg>', '<svg><g></g></svg>', false, true ),
'MathML MI without self-closing flag' => array( '<math><mi /b></mi></math>', 'MI', 'b', '<math><mi ></mi></math>', '<math><mi></mi></math>', false, true ),
'SVG G with self-closing flag' => array( '<svg><g =/>text</svg>', 'G', '=', '<svg><g />text</svg>', '<svg><g />text</svg>', true, false ),
'MathML MI with self-closing flag' => array( '<math><mi =/>text</math>', 'MI', '=', '<math><mi />text</math>', '<math><mi />text</math>', true, false ),
);
}

/**
* Ensures that expects_closer works for void-like elements in foreign content.
*
Expand Down
103 changes: 103 additions & 0 deletions tests/phpunit/tests/html-api/wpHtmlTagProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,109 @@
);
}

/**
* Ensures that removing attributes preserves self-closing flag state.
*
* @covers WP_HTML_Tag_Processor::remove_attribute
*
* @dataProvider data_remove_attribute_preserves_self_closing_flag_state
*
* @param string $html HTML containing an attribute to remove.
* @param string $attribute_to_remove Name of the attribute to remove.
* @param string $expected Expected updated HTML.
* @param bool $expected_has_self_closing_flag Expected self-closing flag state.
*/
public function test_remove_attribute_preserves_self_closing_flag_state( $html, $attribute_to_remove, $expected, $expected_has_self_closing_flag ) {
$processor = new WP_HTML_Tag_Processor( $html );
$this->assertTrue( $processor->next_tag(), 'Could not find the DIV tag: check test setup.' );
$this->assertSame(
$expected_has_self_closing_flag,
$processor->has_self_closing_flag(),
'Test setup has the wrong self-closing flag state.'
);

$processor->remove_attribute( $attribute_to_remove );

$this->assertSame(
$expected,
$processor->get_updated_html(),
'Removing the attribute should not change the self-closing flag state.'
);
$this->assertSame(
$expected_has_self_closing_flag,
$processor->has_self_closing_flag(),
'Removing the attribute should preserve the self-closing flag state.'
);
}

/**
* Data provider.
*
* @return array[]
*/
public static function data_remove_attribute_preserves_self_closing_flag_state() {
return array(
'Boolean attribute after slash separator' => array( '<div /b>', 'b', '<div >', false ),

Check warning on line 1152 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 5 space(s) between "'Boolean attribute after slash separator'" and double arrow, but found 13.
'Unquoted attribute after slash separator' => array( '<div /b=c>', 'b', '<div >', false ),

Check warning on line 1153 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 4 space(s) between "'Unquoted attribute after slash separator'" and double arrow, but found 12.
'Double-quoted attribute after slash separator' => array( '<div /b="c">', 'b', '<div >', false ),

Check warning on line 1154 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Expected 1 space between "'Double-quoted attribute after slash separator'" and double arrow; 7 found.
'Single-quoted attribute after slash separator' => array( "<div /b='c'>", 'b', '<div >', false ),

Check warning on line 1155 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Expected 1 space between "'Single-quoted attribute after slash separator'" and double arrow; 7 found.
'Equals-sign attribute after slash separator' => array( '<div /=>', '=', '<div >', false ),

Check warning on line 1156 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 1 space(s) between "'Equals-sign attribute after slash separator'" and double arrow, but found 9.
'Duplicate attribute after slash separator' => array( '<div b /b>', 'b', '<div >', false ),

Check warning on line 1157 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 3 space(s) between "'Duplicate attribute after slash separator'" and double arrow, but found 11.
'Attribute after multiple slash separators' => array( '<div //b>', 'b', '<div >', false ),

Check warning on line 1158 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 3 space(s) between "'Attribute after multiple slash separators'" and double arrow, but found 11.
'Boolean attribute before self-closing flag' => array( '<div b/>', 'b', '<div />', true ),

Check warning on line 1159 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 2 space(s) between "'Boolean attribute before self-closing flag'" and double arrow, but found 10.
'Equals-sign attribute before self-closing flag' => array( '<div =/>', '=', '<div />', true ),

Check warning on line 1160 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Expected 1 space between "'Equals-sign attribute before self-closing flag'" and double arrow; 6 found.
'Slash-separated attribute before self-closing flag' => array( '<div /b/>', 'b', '<div />', true ),

Check warning on line 1161 in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Expected 1 space between "'Slash-separated attribute before self-closing flag'" and double arrow; 2 found.
'Spaced boolean attribute before self-closing flag' => array( '<div b />', 'b', '<div />', true ),
'Spaced equals-sign attribute before self-closing flag' => array( '<div = />', '=', '<div />', true ),
);
}

/**
* Ensures that adding and removing attributes with the same insertion point doesn't reinsert the removed text.
*
* @covers WP_HTML_Tag_Processor::set_attribute
* @covers WP_HTML_Tag_Processor::add_class
* @covers WP_HTML_Tag_Processor::remove_attribute
*
* @dataProvider data_can_add_attribute_while_removing_slash_separated_attribute
*
* @param string $html HTML containing a slash-separated attribute.
* @param string $method Method for adding an attribute before removing another.
* @param string $expected Expected updated HTML.
*/
public function test_can_add_attribute_while_removing_slash_separated_attribute( $html, $method, $expected ) {
$processor = new WP_HTML_Tag_Processor( $html );
$this->assertTrue( $processor->next_tag(), 'Could not find the DIV tag: check test setup.' );

if ( 'set_attribute' === $method ) {
$processor->set_attribute( 'a', 'x' );
} else {
$processor->add_class( 'x' );
}

$processor->remove_attribute( 'b' );

$this->assertSame(
$expected,
$processor->get_updated_html(),
'Adding an attribute while removing another should not reinsert the removed slash-separated attribute.'
);
}

/**
* Data provider.
*
* @return array[]
*/
public static function data_can_add_attribute_while_removing_slash_separated_attribute() {
return array(
'Set attribute, no self-closing flag' => array( '<div/b>', 'set_attribute', '<div a="x">' ),
'Add class, no self-closing flag' => array( '<div/b>', 'add_class', '<div class="x">' ),
'Set attribute, self-closing flag' => array( '<div/b/>', 'set_attribute', '<div a="x"/>' ),
'Add class, self-closing flag' => array( '<div/b/>', 'add_class', '<div class="x"/>' ),
);
}

/**
* @ticket 56299
*
Expand Down
Loading