Skip to content

HTML API: Deduplicate class names with a keyed set #15223

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 1 commit into
base: trunk
Choose a base branch
from
Draft

HTML API: Deduplicate class names with a keyed set #15223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 16 additions & 11 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 @@ -1220,12 +1220,12 @@ public function class_list() {
$at += $length;

/*
* It's expected that the number of class names for a given tag is relatively small.
* Given this, it is probably faster overall to scan an array for a value rather
* than to use the class name as a key and check if it's a key of $seen.
* `$seen` is keyed by class name. PHP stores a canonical integer string
* such as "123" as the integer key 123, and no other string maps to that
* key, so distinct names stay distinct.
*/
if ( ! in_array( $name, $seen, true ) ) {
$seen[] = $name;
if ( ! isset( $seen[ $name ] ) ) {
$seen[ $name ] = true;
yield $name;
}

Expand Down Expand Up @@ -2492,19 +2492,24 @@ private function class_name_updates_to_attributes_updates(): void {
*/
$modified = false;

/*
* `$seen` and `$to_remove` are keyed by class name. PHP stores a canonical
* integer string such as "123" as the integer key 123, and no other string
* maps to that key, so distinct names stay distinct.
*/
$seen = array();
$to_remove = array();
$is_quirks = self::QUIRKS_MODE === $this->compat_mode;
if ( $is_quirks ) {
foreach ( $this->classname_updates as $updated_name => $action ) {
if ( self::REMOVE_CLASS === $action ) {
$to_remove[] = strtolower( $updated_name );
$to_remove[ strtolower( $updated_name ) ] = true;
}
}
} else {
foreach ( $this->classname_updates as $updated_name => $action ) {
if ( self::REMOVE_CLASS === $action ) {
$to_remove[] = $updated_name;
$to_remove[ $updated_name ] = true;
}
}
}
Expand All @@ -2529,17 +2534,17 @@ private function class_name_updates_to_attributes_updates(): void {
$at += $name_length;

// If this class is marked for removal, remove it and move on to the next one.
if ( in_array( $comparable_class_name, $to_remove, true ) ) {
if ( isset( $to_remove[ $comparable_class_name ] ) ) {
$modified = true;
continue;
}

// If a class has already been seen then skip it; it should not be added twice.
if ( in_array( $comparable_class_name, $seen, true ) ) {
if ( isset( $seen[ $comparable_class_name ] ) ) {
continue;
}

$seen[] = $comparable_class_name;
$seen[ $comparable_class_name ] = true;

/*
* Otherwise, append it to the new "class" attribute value.
Expand All @@ -2562,7 +2567,7 @@ private function class_name_updates_to_attributes_updates(): void {
// Add new classes by appending those which haven't already been seen.
foreach ( $this->classname_updates as $name => $operation ) {
$comparable_name = $is_quirks ? strtolower( $name ) : $name;
if ( self::ADD_CLASS === $operation && ! in_array( $comparable_name, $seen, true ) ) {
if ( self::ADD_CLASS === $operation && ! isset( $seen[ $comparable_name ] ) ) {
$modified = true;

$class .= strlen( $class ) > 0 ? ' ' : '';
Expand Down
30 changes: 30 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 @@ -2691,6 +2691,7 @@ public function test_class_list_null_bytes_replaced() {
/**
* @ticket 59209
* @ticket 65846
* @ticket 65967
*
* @covers WP_HTML_Tag_Processor::class_list
*
Expand Down Expand Up @@ -2742,6 +2743,8 @@ public static function data_class_list(): array {
'Decimal reference without semicolon' => array( "<div class='a&#32b'>", array( 'a', 'b' ) ),
'Hex reference without semicolon' => array( "<div class='a&#x20g'>", array( 'a', 'g' ) ),
'Bare ampersand' => array( "<div class='&'>", array( '&' ) ),
'Numeric-looking names stay distinct' => array( "<div class='123 0123 1e3 -0 0'>", array( '123', '0123', '1e3', '-0', '0' ) ),
'Numeric duplicates collapse' => array( "<div class='123 123'>", array( '123' ) ),
);
}

Expand Down Expand Up @@ -2800,6 +2803,33 @@ public static function data_html_with_variations_of_class_values_and_sought_clas
);
}

/**
* Ensures that add_class() and remove_class() match class names which are
* canonical integer strings, such as "123", and only those.
*
* @ticket 65967
*
* @covers WP_HTML_Tag_Processor::add_class
* @covers WP_HTML_Tag_Processor::remove_class
*/
public function test_add_class_and_remove_class_match_numeric_class_names() {
$processor = new WP_HTML_Tag_Processor( '<div class="123 abc 123">' );
$processor->next_tag();
$processor->remove_class( '123' );

$this->assertSame( '<div class="abc">', $processor->get_updated_html(), 'Failed to remove class name "123".' );

$processor = new WP_HTML_Tag_Processor( '<div class="123 0123 1e3">' );
$processor->next_tag();
$processor->add_class( '123' );

$this->assertSame( '<div class="123 0123 1e3">', $processor->get_updated_html(), 'Added a second copy of class name "123".' );

$processor->remove_class( '0123' );

$this->assertSame( '<div class="123 1e3">', $processor->get_updated_html(), 'Did not remove class name "0123" alone.' );
}

/**
* Ensures that the invalid comment closing syntax "--!>" properly closes a comment.
*
Expand Down
Loading