Add a has_class method to the public WP_HTML_Tag_Processor API

[adamziel]

Part of #44410

What?

Adds a has_class method to WP_HTML_Tag_Processor to provide an idiomatic, nuance-aware way of testing for the presence of a CSS class name:

// <div class="is-foo"></div>
$tags->has_class( 'is-foo' ); // true

// <div class="is-foo"></div>
$tags->has_class( 'not-is-foo' ); // false

// <div class="is-bar"></div>
$tags->has_class( 'is-foo' ); // false

// is-bar\tis-foo\ris-another-class'></div>
$tags->has_class( 'is-foo' ); // true

Testing Instructions

• Confirm the unit tests pass
• Confirm the code is sound

cc @dmsnell @getdave @aristath

[dmsnell]

As a broad thought I'm a bit skeptical of this even though I suggested it.

For one, I feel like it's starting to get confusing the way we have string_contains_a_class_name. That's not semantic to HTML and worse, it's not accurate. It's less accurate than the existing code, which should be fine for all cases except where we have HTML-entity-encoded class names.

For example, the example in the docblock shouldn't find bold inside of <a class="button bold"></a> (or maybe the code works and the example is wrong?)

I'd like to be sure we really need this before we introduce a second way to query class names. Im worried that the normative use-case ->next_tag( [ 'class_name' => '…' ] ) will be supplanted by the imperative version instead, which will confuse people and not perform the way they want.

while ( $tags->next_tag() ) {
	if ( $tags->has_class( '…' ) ) {
		…
	}
}

If we do go this route I'd be curious about building it as a separate system than the one shared by the query engine. Build a proper decoded, though of course as we saw earlier that's much more complicated. It's something I think we could make a more justified case for out of the main loop, but again something I want to know that we need before we put it out there and encourage people to use it.

[adamziel]

@dmsnell I refactored this one to use preg_match instead of recycling parts of the query scanner – would you give it another look?

For one, I feel like it's starting to get confusing the way we have string_contains_a_class_name. That's not semantic to HTML and worse, it's not accurate. It's less accurate than the existing code, which should be fine for all cases except where we have HTML-entity-encoded class names.

What do you mean by not accurate? It used the same code as the query matcher, so should be accurate to that extent.

For example, the example in the docblock shouldn't find bold inside of <a class="button bold"></a> (or maybe the code works and the example is wrong?)

The code example was correct, it used the string offset arguments to limit the search to the button bold part of the string.

I'd like to be sure we really need this before we introduce a second way to query class names. Im worried that the normative use-case ->next_tag( [ 'class_name' => '…' ] ) will be supplanted by the imperative version instead, which will confuse people and not perform the way they want.

while ( $tags->next_tag() ) {
	if ( $tags->has_class( '…' ) ) {
		…
	}
}

If that happened then yeah, it would be problematic. I don't think it will happen, though. People can already ditch the query matcher and manually match the tag names, and I didn't see it happening. We'll know for sure after 6.2 is released and the WP_HTML_Tag_processor gets noticed in the wider community, but I remain optimistic.

If we do go this route I'd be curious about building it as a separate system than the one shared by the query engine. Build a proper decoded, though of course as we saw earlier that's much more complicated. It's something I think we could make a more justified case for out of the main loop, but again something I want to know that we need before we put it out there and encourage people to use it.

This may be worth a shot. At the same time, I'd wait for more feature requests to help shape the scope.

[dmsnell]

note that if $this->is_closing_tag() then get_attribute() will return null as well, so we may not need to double-check $this->is_closing_tag

[dmsnell]

Concerning being more wrong I'm glad it was just the example code and not the real code that was wrong.

Concerning the value of the API I'm still skeptical, especially as long as we're not handling entities properly.

I'd really like to know this is wanted and necessary before we add it. Counterpoint is something like most operations I've seen related to classes simply need to seek to a tag with a given class, and then they add or remove classes not based on whether a class exists, but whether or not some value in the attribute set exists.

[adamziel]

Let's keep it on a backburner, then, and revisit at some other time. cc @getdave @aristath

[adamziel]

I'll close this PR for now.