Skip to content
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.

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

HTML API: Only pass a single class name to add_class() #5325

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/wp-includes/block-supports/elements.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

/**
* Gets the elements class names.
* Gets the elements class name for a given block.
*
* @since 6.0.0
* @access private
Expand Down
21 changes: 17 additions & 4 deletions src/wp-includes/blocks/search.php
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in Slack, this will need changing in Gutenberg; it will then require a new release of the @wordpress/block-library npm so it can be finally sync'ed to Core. (There's currently no other way 😅 )

AFAICT, you've started work on this already in WordPress/gutenberg#54873. If we want to get this into WP 6.4, we'd need to land it fairly soon so it can be cherry-picked for one of the next rounds of npm releases (probably prior to Beta 3 on Oct 10).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the review. I don't see a need to push this into 6.4 as it doesn't currently cause any problems (beyond the fact that passing mutiple class names can cause problems), but it will need to be updated before we lock down adding multiple classes probably for 6.5

Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,17 @@ function render_block_core_search( $attributes, $content, $block ) {
$label->set_attribute( 'for', $input_id );
$label->add_class( 'wp-block-search__label' );
if ( $show_label && ! empty( $attributes['label'] ) ) {
if ( ! empty( $typography_classes ) ) {
$label->add_class( $typography_classes );
/*
* There are two oddities here:
* - the classes are joined in get_typography_classes_for_block_core_search()
* so it shouldn't be necessary here to re-explode them.
* - the class names are esc_attr()'d but then fed into the HTML API, which leads
* to double-escaping.
*
* @TODO: Fix these upstream to avoid double work and double escaping.
*/
foreach ( WP_CSS::class_list( $typography_classes ) as $class ) {
$label->add_class( $class );
}
} else {
$label->add_class( 'screen-reader-text' );
Expand All @@ -73,7 +82,9 @@ function render_block_core_search( $attributes, $content, $block ) {
$input_classes[] = $typography_classes;
}
if ( $input->next_tag() ) {
$input->add_class( implode( ' ', $input_classes ) );
foreach ( $input_classes as $class ) {
$input->add_class( $class );
}
$input->set_attribute( 'id', $input_id );
$input->set_attribute( 'value', get_search_query() );
$input->set_attribute( 'placeholder', $attributes['placeholder'] );
Expand Down Expand Up @@ -143,7 +154,9 @@ function render_block_core_search( $attributes, $content, $block ) {
$button = new WP_HTML_Tag_Processor( sprintf( '<button type="submit" %s>%s</button>', $inline_styles['button'], $button_internal_markup ) );

if ( $button->next_tag() ) {
$button->add_class( implode( ' ', $button_classes ) );
foreach ( $button_classes as $class ) {
$button->add_class( $class );
}
if ( 'expand-searchfield' === $attributes['buttonBehavior'] && 'button-only' === $attributes['buttonPosition'] ) {
$button->set_attribute( 'data-wp-bind--aria-label', 'selectors.core.search.ariaLabel' );
$button->set_attribute( 'data-wp-bind--aria-controls', 'selectors.core.search.ariaControls' );
Expand Down
58 changes: 58 additions & 0 deletions src/wp-includes/html-api/class-wp-css.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

class WP_CSS {
/**
* Generator function that allows iterating over the list of unique
* CSS class names found within a given already-HTML-decoded string.
*
* Names are provided in a lower-case form to aid processing in calling
* code since CSS class names are case-insensitive on US-ASCII letters.
*
* @since 6.4.0
*
* @param string $class_string String containing a list of CSS class names, e.g. from an HTML "class" attribute.
* @return Generator|void Generator to iterate over unique CSS class names, or null if not given a string.
*/
public static function class_list( $class_string ) {
if ( ! is_string( $class_string ) ) {
return;
}

$seen = array();

$at = 0;
while ( $at < strlen( $class_string ) ) {
// Skip past any initial boundary characters.
$at += strspn( $class_string, " \t\f\r\n", $at );
if ( $at >= strlen( $class_string ) ) {
return;
}

// Find the byte length until the next boundary.
$length = strcspn( $class_string, " \t\f\r\n", $at );
if ( 0 === $length ) {
return;
}

/*
* CSS class names are case-insensitive in the ASCII range.
*
* @see https://www.w3.org/TR/CSS2/syndata.html#x1
*/
$name = strtolower( substr( $class_string, $at, $length ) );
$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.
*/
if ( in_array( $name, $seen, true ) ) {
continue;
}

$seen[] = $name;
yield $name;
}
}
}
44 changes: 2 additions & 42 deletions src/wp-includes/html-api/class-wp-html-tag-processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -643,48 +643,8 @@ public function next_tag( $query = null ) {
* @since 6.4.0
*/
public function class_list() {
/** @var string $class contains the string value of the class attribute, with character references decoded. */
$class = $this->get_attribute( 'class' );

if ( ! is_string( $class ) ) {
return;
}

$seen = array();

$at = 0;
while ( $at < strlen( $class ) ) {
// Skip past any initial boundary characters.
$at += strspn( $class, " \t\f\r\n", $at );
if ( $at >= strlen( $class ) ) {
return;
}

// Find the byte length until the next boundary.
$length = strcspn( $class, " \t\f\r\n", $at );
if ( 0 === $length ) {
return;
}

/*
* CSS class names are case-insensitive in the ASCII range.
*
* @see https://www.w3.org/TR/CSS2/syndata.html#x1
*/
$name = strtolower( substr( $class, $at, $length ) );
$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.
*/
if ( in_array( $name, $seen, true ) ) {
continue;
}

$seen[] = $name;
yield $name;
foreach ( WP_CSS::class_list( $this->get_attribute( 'class' ) ) as $class ) {
yield $class;
}
}

Expand Down
1 change: 1 addition & 0 deletions src/wp-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@
require ABSPATH . WPINC . '/class-wp-oembed-controller.php';
require ABSPATH . WPINC . '/media.php';
require ABSPATH . WPINC . '/http.php';
require ABSPATH . WPINC . '/html-api/class-wp-css.php';
require ABSPATH . WPINC . '/html-api/class-wp-html-attribute-token.php';
require ABSPATH . WPINC . '/html-api/class-wp-html-span.php';
require ABSPATH . WPINC . '/html-api/class-wp-html-text-replacement.php';
Expand Down