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.

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

HTML API: Only stop on full matches for requested tag name. #7189

Closed
wants to merge 4 commits into from
Closed

HTML API: Only stop on full matches for requested tag name. #7189

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2b3ae1
HTML API: Only stop on full matches for requested tag name.
dmsnell Aug 13, 2024
f93df18
Add test verifying behavior.
dmsnell Aug 13, 2024
a518f86
Add negative test.
dmsnell Aug 13, 2024
77f1244
Merge branch 'trunk' into html-api/fix-match-the-right-tag-name
dmsnell Aug 13, 2024
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
8 changes: 7 additions & 1 deletion 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 @@ -4009,7 +4009,13 @@ private function matches(): bool {
}

// Does the tag name match the requested tag name in a case-insensitive manner?
if ( isset( $this->sought_tag_name ) && 0 !== substr_compare( $this->html, $this->sought_tag_name, $this->tag_name_starts_at, $this->tag_name_length, true ) ) {
if (
isset( $this->sought_tag_name ) &&
(
strlen( $this->sought_tag_name ) !== $this->tag_name_length ||
0 !== substr_compare( $this->html, $this->sought_tag_name, $this->tag_name_starts_at, $this->tag_name_length, true )
)
) {
return false;
}

Expand Down
18 changes: 18 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 @@ -601,6 +601,24 @@ public function test_next_tag_should_return_false_for_a_non_existing_tag() {
$this->assertFalse( $processor->next_tag( 'p' ), 'Querying a non-existing tag did not return false' );
}

/**
* @ticket 61545
*/
public function test_next_tag_should_not_match_on_substrings_of_a_requested_tag() {
$processor = new WP_HTML_Tag_Processor( '<p><pic><picture>' );

$this->assertTrue(
$processor->next_tag( 'PICTURE' ),
'Failed to find a tag when requested: check test setup.'
);

$this->assertSame(
'PICTURE',
$processor->get_tag(),
'Should have skipped past substring tag matches, directly finding the PICTURE element.'
);
}
Copy link
Member

Choose a reason for hiding this comment

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

In the case we had, this would have failed:

$processor = new WP_HTML_Tag_Processor( '<p><img></p>' );
$this->assertFalse(
	$processor->next_tag( 'PICTURE' ),
	'Unexpectedly found a tag when requested: check test setup.'
);

Maybe good to have a negative case added to this as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

@westonruter I think these are the same case. in both cases they stop at the P - am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

nevermind - I didn't understand your comment at first.

Copy link
Member Author

Choose a reason for hiding this comment

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

added the test, but I used a different failure message


/**
* @ticket 59209
*
Expand Down
Loading