Make WordPress Core

Opened 2 months ago

Last modified 3 weeks ago

#65466 new defect (bug)

HTML API: Introduce helper function for parsing CSS class names from a (decoded) attribute value

Reported by: westonruter Owned by:
Priority: normal Milestone: 7.2
Component: HTML API Version: 6.4
Severity: normal Keywords:
Cc: Focuses:

Description

The HTML API has the WP_HTML_Tag_Processor::class_list() method which can be used to tokenize the class names from a class attribute according to the HTML spec. However, to use it, you have to first construct an HTML tag with the class attribute value encoded as HTML. For example (PHP Playground):

<?php
/**
 * Parses class names from a class list string.
 *
 * @return Generator<int, non-empty-string>
 */
function classnames_to_class_list( string $class_names ): Generator {
        $processor = new WP_HTML_Tag_Processor(
                sprintf( '<div class="%s"></div>', esc_attr( $class_names ) )
        );
        $processor->next_tag();
        foreach ( $processor->class_list() as $class_name ) {
                yield $class_name;
        }
}

print_r( iterator_to_array( classnames_to_class_list( ' foo bar baz ' ) ) );

Output:

Array
(
    [0] => foo
    [1] => bar
    [2] => baz
)

We can avoid the overhead of encoding and parsing the HTML.

In fact, this was done recently in:

  • r62475 for wp_render_elements_class_name() (gutenberg_render_elements_class_name())
  • r62359 for wp_render_custom_css_class_name() (gutenberg_render_custom_css_class_name())

The two functions are duplicating the same underlying logic for parsing the class list according to the HTML spec, as identified in a PR comment.

There additional cases which could make use of this helper:

  • wp_render_block_style_variation_class_name() (gutenberg_render_block_style_variation_class_name())
  • wp_get_block_style_variation_name_from_registered_style() (gutenberg_get_block_style_variation_name_from_registered_style())
  • gutenberg_get_block_style_variation_name_from_class()
  • WP_Duotone_Gutenberg::restore_image_outer_container()

Both of these functions have bugs in parsing class names, either by not tokenizing correctly at a token boundary (e.g. by using the \b word boundary in regex) or by incorrectly splitting by spaces, when other whitespace characters may separate tokens.

An initial implementation has been sketched out by @jonsurrell in a PR comment.

Change History (5)

#1 @dmsnell
2 months ago

for parsing the class list according to the HTML spec

this part worries me, particularly because we know from the examples that we are not working with HTML data, but some implicit data type in JSON which is a list of class names.

to that end, we definitely don’t want to decode HTML character references, and we may want to be careful about assuming whether or not to treat case-variations of the same class name as duplicates or distinct.

if the JSON semantic is "space separates class names" then it could even be better that we use strok( $decoded_class_list, ' ' ); (against my own previous advice, I’m sure).

and maybe this is roughly another perspective on needing something like a string set, which deduplicates on byte-identical inputs.


it would seem important to me to make sure that the appearance of this function does not lead to confusion between the semantic act of parsing class names from HTML strings and adding class names from some custom implicit and undocumented but already decoded setting.

#2 @jonsurrell
2 months ago

There does seem to be utility in a function like this. I do share some concerns that there's potential for confusion and mis-use. The fact that different implementations of this functionality exist in WordPress Core right now (" ", \S, \b separators) suggests there's remove for improvement.

it could even be better that we use strok( $decoded_class_list, ' ' );

I think I understand where this is coming from. The proposed function deals with a narrow part of HTML (whitespace separated lists) while ignoring some other parts (HTML character reference decoding or some DOMTokenList set behaviors like deduplication).

As long as this function is useful and its scope is clear, it's worth considering. Let's survey the examples already mentioned:

All of these cases revolve around the className block attribute. On one hand, we should be able to normalize this block attribute to a " " separated list instead of HTML-whitespace separated. On the other hand, it does seem clear that this is a plaintext representation of the HTML attribute value, where the HTML whitespace separators " \t\f\r\n" should work fine. Perhaps both would be good, both _recognizing and respecting_ the HTML separators, but also normalizing to " " when storing or parsing this attribute.


For reference, this is the implementation I shared:

<?php
/**
 * Splits a string containing multiple class names yielding individual values.
 *
 * This operates on _plaintext_. DO NOT USE WITH HTML ENCODED VALUES. It will
 * not perform any decoding. Only use on plaintext inputs!
 * 
 * Named for classnames for discoverability as most common use case, but suitable
 * for any HTML whitespace delimited list (DOMTokenList).
 * 
 * For HTML encoded values, use {@see WP_HTML_Tag_Processor}.
 *
 * Splits on HTML ASCII whitespace. This matches the behavior of DOMTokenList
 * and is suitable for class lists (…add more things that use DOMTokenList).
 * 
 * @todo add examples to documentation…
 *
 * @param string $classnames Plaintext space-delimited list.
 * @return Generator<string> Individual items.
 */
function classnames_to_class_list( $classnames ): Generator {
    if ( ! is_string( $classnames ) ) {
        // _doing_it_wrong?
        return;
    }
        $delimiters = " \t\n\f\r";
        $length     = strlen( $classnames );
        $offset     = 0;

        while ( $offset < $length ) {
                $offset += strspn( $classnames, $delimiters, $offset );

                if ( $offset >= $length ) {
                        break;
                }

                $token_length = strcspn( $classnames, $delimiters, $offset );

                yield substr( $classnames, $offset, $token_length );

                $offset += $token_length;
        }
}

#3 @jonsurrell
8 weeks ago

WP_Duotone_Gutenberg::restore_image_outer_container uses the Tag Processor to process block content, but explodes the attribute value on " " to work with individual class names. (The HTML API class helpers should be used in this case, it's not a candidate for the proposed method and should be updated).

Addressing this.

#4 @dmsnell
8 weeks ago

the thought struck me while looking through the CSS spec that we could have some luck using the terminology for the semantic values rather than the lexical values. we’re dealing with various input sources and meanings, but this is a task whose goal is mostly to split a set of space-separated tokens

a shift to this layer in the abstraction could redeem the parsing issues without introducing the conflation of “a JSON string value whose space-separated names are CSS class names” and “an HTML class attribute”

that leaves some things feeling awkwardly-named, but we have a lot of awkwardly named things.

#5 @wildworks
3 weeks ago

  • Milestone 7.17.2

Since we are already in the 7.1 Beta phase, let's release the new API in 7.2.

Note: See TracTickets for help on using tickets.