Opened 3 months ago
Closed 2 months ago
#62427 closed defect (bug) (fixed)
HTML API: Next tag tag name query is case sensitive
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.7.2 | Priority: | normal |
Severity: | normal | Version: | 6.6 |
Component: | HTML API | Keywords: | has-patch has-unit-tests dev-feedback |
Focuses: | Cc: |
Description
The HTML API ::next_tag
method accepts a tag_name
query. The tag name matching should be case-insensitive. Tag names are generally UPPERCASE in the processors, a lower or mixed case name doesn't make much sense.
<?php // This is OK $processor = WP_HTML_Processor::create_fragment( '<DIV>' ); assert( $processor->next_tag( 'div' ) ); // This is OK $processor = WP_HTML_Processor::create_fragment( '<DIV>' ); assert( $processor->next_tag( array( 'breadcrumbs' => array( 'div' ) ) ) ); // This does not work but should $processor = WP_HTML_Processor::create_fragment( '<DIV>' ); assert( $processor->next_tag( array( 'tag_name' => 'div' ) ) ); // AssertionError!
This is also supported in the tag processor case-insensitively:
<?php $processor = new WP_HTML_Tag_Processor( '<section><DIV>' ); assert( $processor->next_tag( array( 'tag_name' => 'div' ) ) ); $processor->get_tag(); // "DIV"
Change History (13)
This ticket was mentioned in PR #7754 on WordPress/wordpress-develop by @jonsurrell.
3 months ago
#1
- Keywords has-patch has-unit-tests added
@jonsurrell commented on PR #7754:
3 months ago
#3
As far as I can tell this was an oversight. I can't think of a reason _to_ match this case-sensitively.
I suspect this was not noticed because the shorthand exists for the next tag with a tag name in the HTML Processor: next_tag( 'div' )
.
Under the hood that uses next_tag( array( 'breadcrumbs' => array( 'div' ) ) )
:
And _that_ is handled correctly by the matches_breadcrumbs
function:
(I had included a change in this patch to change the shorthand implementation to use tag_name
instead of breadcrumbs
, but removed that as out-of-scope for a bugfix.)
This ticket was mentioned in Slack in #core by desrosj. View the logs.
3 months ago
#5
@
3 months ago
- Milestone changed from 6.7.1 to 6.7.2
Thanks @jonsurrell! Does this cause any unintended side effects as is? It's not clear if this fixes unexpected behavior or is meant for consistency/clarity.
That said, because of a few bug reports opened since 6.7 was released, the Core team is evaluating the need for a short 6.7.1 cycle (possibly next week).
To help prepare for this scenario in case it's decided to move forward, I'm going to punt this to the 6.7.2 milestone. This issue was not introduced in 6.7, so it now falls outside of the focus for 6.7.1 as currently defined.
3 months ago
#8
Committed to core in https://core.trac.wordpress.org/changeset/59422
#9
@
3 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for consideration for 6.7.2.
Trac ticket: https://core.trac.wordpress.org/ticket/62427