Skip to content

Update OD_HTML_Tag_Processor::next_tag() to allow $query arg and prepare to skip visiting tag closers by default #1872

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

Merged
merged 8 commits into from
Feb 26, 2025
Merged

Update OD_HTML_Tag_Processor::next_tag() to allow $query arg and prepare to skip visiting tag closers by default #1872

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
069dc53
Try: Allow queries to be passed to next_tag()
westonruter Feb 16, 2025
5aae390
Begin transition for next_tag() to align with base class behavior
westonruter Feb 17, 2025
6e9bb16
Use is_admin_bar since faaster than computing XPath for every tag
westonruter Feb 17, 2025
b45e4bb
Opt to vary query arg for next_tag() by OD version
westonruter Feb 22, 2025
4cfe451
Test that next_tag() without query defaults to visiting tag closers (…
westonruter Feb 22, 2025
2c6271b
Improve code coverage
westonruter Feb 22, 2025
7a7411a
Soft-deprecate next_open_tag()
westonruter Feb 22, 2025
2f97645
Mark is_admin_bar as private
westonruter Feb 25, 2025
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
Prev Previous commit
Next Next commit
Begin transition for next_tag() to align with base class behavior
  • Loading branch information
@westonruter
westonruter committed Feb 17, 2025
commit 5aae390dbc042e6e273cc09769d8b648f7ac553c
11 changes: 8 additions & 3 deletions plugins/embed-optimizer/hooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,13 @@ function embed_optimizer_add_non_optimization_detective_hooks(): void {
* @param string $optimization_detective_version Current version of the optimization detective plugin.
*/
function embed_optimizer_init_optimization_detective( string $optimization_detective_version ): void {
$required_od_version = '0.9.0';
if ( version_compare( (string) strtok( $optimization_detective_version, '-' ), $required_od_version, '<' ) ) {
if (
version_compare( (string) strtok( $optimization_detective_version, '-' ), '1.0.0', '<' )
||
'1.0.0-beta1' === $optimization_detective_version
||
'1.0.0-beta2' === $optimization_detective_version
) {
add_action(
'admin_notices',
static function (): void {
Expand Down Expand Up @@ -253,7 +258,7 @@ function embed_optimizer_update_markup( WP_HTML_Tag_Processor $html_processor, b
}
}
}
} while ( $html_processor->next_tag() );
} while ( $html_processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
// If there was only one non-inline script, make it lazy.
if ( 1 === $script_count && ! $has_inline_script && $html_processor->has_bookmark( $bookmark_names['script'] ) ) {
$needs_lazy_script = true;
Expand Down
2 changes: 1 addition & 1 deletion plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private function process_picture( OD_HTML_Tag_Processor $processor, OD_Tag_Visit
$crossorigin = null;

// Loop through child tags until we reach the closing PICTURE tag.
while ( $processor->next_tag() ) {
while ( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) ) {
$tag = $processor->get_tag();

// If we reached the closing PICTURE tag, break.
Expand Down
9 changes: 7 additions & 2 deletions plugins/image-prioritizer/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@
* @param string $optimization_detective_version Current version of the optimization detective plugin.
*/
function image_prioritizer_init( string $optimization_detective_version ): void {
$required_od_version = '0.9.0';
if ( ! version_compare( (string) strtok( $optimization_detective_version, '-' ), $required_od_version, '>=' ) ) {
if (
version_compare( (string) strtok( $optimization_detective_version, '-' ), '1.0.0', '<' )
||
'1.0.0-beta1' === $optimization_detective_version
||
'1.0.0-beta2' === $optimization_detective_version
) {
add_action(
'admin_notices',
static function (): void {
Expand Down
19 changes: 7 additions & 12 deletions plugins/optimization-detective/class-od-html-tag-processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,17 +250,18 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
*
* @inheritDoc
* @since 0.4.0
* @since n.e.x.t Passing a $query is now allowed.
* @since n.e.x.t Passing a $query is now allowed. In a future release, this will default to skipping tag closers.
*
* @param array{tag_name?: string|null, match_offset?: int|null, class_name?: string|null, tag_closers?: string|null}|null $query Query.
* @return bool Whether a tag was matched.
*
* @throws InvalidArgumentException If attempting to pass a query.
*/
public function next_tag( $query = null ): bool {
if ( null === $query ) {
// For back-compat with tag visitor which previously relied next_tag() always visiting tag closers.
$query = array( array( 'tag_closers' => 'visit' ) );
$query = array( 'tag_closers' => 'visit' );
$this->warn(
__METHOD__,
esc_html__( 'Previously this method always visited tag closers and did not allow a query to be supplied. Now, however, a query can be supplied. To align this method with the behavior of the base class, a future version of this method will default to skipping tag closers.', 'optimization-detective' )
);
}
return parent::next_tag( $query );
}
Expand All @@ -269,17 +270,11 @@ public function next_tag( $query = null ): bool {
* Finds the next open tag.
*
* @since 0.4.0
* @deprecated
*
* @return bool Whether a tag was matched.
*/
public function next_open_tag(): bool {
while ( $this->next_tag( array( 'tag_closers' => 'visit' ) ) ) {
if ( ! $this->is_tag_closer() ) {
return true;
}
}
return false;
return $this->next_tag( array( 'tag_closers' => 'skip' ) );
}

/**
Expand Down
4 changes: 2 additions & 2 deletions plugins/optimization-detective/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Description: Provides a framework for leveraging real user metrics to detect optimizations for improving page performance.
* Requires at least: 6.6
* Requires PHP: 7.2
* Version: 1.0.0-beta2
* Version: 1.0.0-beta3
* Author: WordPress Performance Team
* Author URI: https://make.wordpress.org/performance/
* License: GPLv2 or later
Expand Down Expand Up @@ -71,7 +71,7 @@ static function ( string $global_var_name, string $version, Closure $load ): voi
}
)(
'optimization_detective_pending_plugin',
'1.0.0-beta2',
'1.0.0-beta3',
static function ( string $version ): void {
if ( defined( 'OPTIMIZATION_DETECTIVE_VERSION' ) ) {
return;
Expand Down
4 changes: 2 additions & 2 deletions plugins/optimization-detective/optimization.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ function od_optimize_template_output_buffer( string $buffer ): string {
// If the initial tag is not an open HTML tag, then abort since the buffer is not a complete HTML document.
$processor = new OD_HTML_Tag_Processor( $buffer );
if ( ! (
$processor->next_tag() &&
$processor->next_tag( array( 'tag_closers' => 'visit' ) ) &&
! $processor->is_tag_closer() &&
'HTML' === $processor->get_tag()
) ) {
Expand Down Expand Up @@ -359,7 +359,7 @@ function od_optimize_template_output_buffer( string $buffer ): string {
if (
in_array( 'NOSCRIPT', $processor->get_breadcrumbs(), true )
||
str_starts_with( $processor->get_xpath(), "/HTML/BODY/DIV[@id='wpadminbar']" )
str_starts_with( $processor->get_stored_xpath(), "/HTML/BODY/DIV[@id='wpadminbar']" )
) {
continue;
}
Expand Down
118 changes: 84 additions & 34 deletions plugins/optimization-detective/tests/test-class-od-html-tag-processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,18 +341,20 @@ public function data_provider_sample_documents(): array {
</body>
</html>
',
'open_tags' => array( 'HTML', 'HEAD', 'BODY', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG' ),
'open_tags' => array( 'HTML', 'HEAD', 'BODY', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG' ),
'xpath_breadcrumbs' => array(
'/HTML' => array( 'HTML' ),
'/HTML/HEAD' => array( 'HTML', 'HEAD' ),
'/HTML/BODY' => array( 'HTML', 'BODY' ),
'/HTML/BODY/DIV[@id=\'header\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML' => array( 'HTML' ),
'/HTML/HEAD' => array( 'HTML', 'HEAD' ),
'/HTML/BODY' => array( 'HTML', 'BODY' ),
'/HTML/BODY/DIV[@id=\'wpadminbar\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'wpadminbar\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@id=\'header\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'header\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@id=\'primary\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'primary\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'primary\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@id=\'secondary\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'secondary\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'secondary\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@id=\'colophon\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'colophon\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'colophon\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
),
),
Expand Down Expand Up @@ -392,25 +394,27 @@ public function data_provider_sample_documents(): array {
</body>
</html>
',
'open_tags' => array( 'HTML', 'HEAD', 'BODY', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG' ),
'open_tags' => array( 'HTML', 'HEAD', 'BODY', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG', 'DIV', 'IMG' ),
'xpath_breadcrumbs' => array(
'/HTML' => array( 'HTML' ),
'/HTML/HEAD' => array( 'HTML', 'HEAD' ),
'/HTML/BODY' => array( 'HTML', 'BODY' ),
'/HTML/BODY/DIV[@role=\'banner\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML' => array( 'HTML' ),
'/HTML/HEAD' => array( 'HTML', 'HEAD' ),
'/HTML/BODY' => array( 'HTML', 'BODY' ),
'/HTML/BODY/DIV[@id=\'wpadminbar\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@id=\'wpadminbar\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@role=\'banner\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@role=\'banner\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@class=\'content-area main\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@class=\'content-area main\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@class=\'widget-area\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@class=\'widget-area\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@class=\'site-footer\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@class=\'site-footer\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@class=\'\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@class=\'\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@class=\'\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV[@role=\'\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@role=\'\']' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV[@role=\'\']/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
'/HTML/BODY/DIV' => array( 'HTML', 'BODY', 'DIV' ),
'/HTML/BODY/DIV/*[1][self::IMG]' => array( 'HTML', 'BODY', 'DIV', 'IMG' ),
),
),
);
Expand Down Expand Up @@ -442,7 +446,7 @@ public function test_next_tag_and_get_xpath( string $document, array $open_tags,
$this->assertSame( '', $p->get_stored_xpath(), 'Expected empty XPath since iteration has not started.' );
$actual_open_tags = array();
$actual_xpath_breadcrumbs_mapping = array();
while ( $p->next_open_tag() ) {
while ( $p->next_tag( array( 'tag_closers' => 'skip' ) ) ) {
$actual_open_tags[] = $p->get_tag();

$xpath = $p->get_stored_xpath();
Expand All @@ -466,14 +470,60 @@ public function test_next_tag_and_get_xpath( string $document, array $open_tags,
}

/**
* Test next_tag() passing query which is invalid.
* Test next_tag() passing query.
*
* @covers ::next_tag
* @covers ::get_xpath
* @covers ::get_current_depth
*/
public function test_next_tag_with_query(): void {
$this->expectException( InvalidArgumentException::class );
$p = new OD_HTML_Tag_Processor( '<html></html>' );
$p->next_tag( array( 'tag_name' => 'HTML' ) );
$html = '
<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body>
<main>
<p>Hello world</p>
<figure>
<img src="https://example.com/img1.jpg">
</figure>
<figure>
<img src="https://example.com/img2.jpg">
</figure>
<div class="foo">
Foo!
</div>
</main>
</body>
</html>
';

$p = new OD_HTML_Tag_Processor( $html );
$this->assertTrue( $p->next_tag( array( 'tag_name' => 'HTML' ) ) );
$this->assertTrue( $p->set_bookmark( 'document_root' ) );
$this->assertSame( 1, $p->get_current_depth() );

$this->assertTrue( $p->next_tag( array( 'tag_name' => 'IMG' ) ) );
$this->assertEquals( '/HTML/BODY/MAIN/*[2][self::FIGURE]/*[1][self::IMG]', $p->get_xpath() );
$this->assertSame( 5, $p->get_current_depth() );

$this->assertTrue( $p->next_tag( array( 'class_name' => 'foo' ) ) );
$this->assertEquals( '/HTML/BODY/MAIN/*[4][self::DIV]', $p->get_xpath() );
$this->assertSame( 4, $p->get_current_depth() );

$this->assertTrue( $p->seek( 'document_root' ) );
$this->assertTrue(
$p->next_tag(
array(
'tag_name' => 'IMG',
'match_offset' => 2,
)
)
);
$this->assertEquals( '/HTML/BODY/MAIN/*[3][self::FIGURE]/*[1][self::IMG]', $p->get_xpath() );
$this->assertSame( 5, $p->get_current_depth() );
}

/**
Expand All @@ -484,15 +534,15 @@ public function test_next_tag_with_query(): void {
public function test_expects_closer(): void {
$p = new OD_HTML_Tag_Processor( '<html><body><hr></body></html>' );
$this->assertFalse( $p->expects_closer() );
while ( $p->next_tag() ) {
while ( $p->next_tag( array( 'tag_closers' => 'visit' ) ) ) {
if ( 'BODY' === $p->get_tag() ) {
break;
}
}
$this->assertSame( 'BODY', $p->get_tag() );
$this->assertFalse( $p->expects_closer( 'IMG' ) );
$this->assertTrue( $p->expects_closer() );
$p->next_tag();
$p->next_tag( array( 'tag_closers' => 'visit' ) );
$this->assertSame( 'HR', $p->get_tag() );
$this->assertFalse( $p->expects_closer() );
$this->assertTrue( $p->expects_closer( 'DIV' ) );
Expand Down Expand Up @@ -594,7 +644,7 @@ public function test_get_updated_html_when_out_of_bookmarks(): void {
</html>
';
$processor = new OD_HTML_Tag_Processor( $html );
$this->assertTrue( $processor->next_tag() );
$this->assertTrue( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertEquals( 'HTML', $processor->get_tag() );
$max_bookmarks = max( WP_HTML_Processor::MAX_BOOKMARKS, WP_HTML_Tag_Processor::MAX_BOOKMARKS );
for ( $i = 0; $i < $max_bookmarks + 1; $i++ ) {
Expand Down Expand Up @@ -710,7 +760,7 @@ public function test_bookmarking_and_seeking(): void {
if ( $processor->get_current_depth() < $embed_block_depth ) {
break;
}
} while ( $processor->next_tag() );
} while ( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
}
}

Expand Down Expand Up @@ -790,31 +840,31 @@ public function test_get_cursor_move_count(): void {
)
);
$this->assertSame( 0, $processor->get_cursor_move_count() );
$this->assertTrue( $processor->next_tag() );
$this->assertTrue( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 'HTML', $processor->get_tag() );
$this->assertTrue( $processor->set_bookmark( 'document_root' ) );
$this->assertSame( 1, $processor->get_cursor_move_count() );
$this->assertTrue( $processor->next_tag() );
$this->assertTrue( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 'HEAD', $processor->get_tag() );
$this->assertSame( 3, $processor->get_cursor_move_count() ); // Note that next_token() call #2 was for the whitespace between <html> and <head>.
$this->assertTrue( $processor->next_tag() );
$this->assertTrue( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 'HEAD', $processor->get_tag() );
$this->assertTrue( $processor->is_tag_closer() );
$this->assertSame( 4, $processor->get_cursor_move_count() );
$this->assertTrue( $processor->next_tag() );
$this->assertTrue( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 'BODY', $processor->get_tag() );
$this->assertSame( 6, $processor->get_cursor_move_count() ); // Note that next_token() call #5 was for the whitespace between </head> and <body>.
$this->assertTrue( $processor->next_tag() );
$this->assertTrue( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 'BODY', $processor->get_tag() );
$this->assertTrue( $processor->is_tag_closer() );
$this->assertSame( 7, $processor->get_cursor_move_count() );
$this->assertTrue( $processor->next_tag() );
$this->assertTrue( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 'HTML', $processor->get_tag() );
$this->assertTrue( $processor->is_tag_closer() );
$this->assertSame( 9, $processor->get_cursor_move_count() ); // Note that next_token() call #8 was for the whitespace between </body> and <html>.
$this->assertFalse( $processor->next_tag() );
$this->assertFalse( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 10, $processor->get_cursor_move_count() );
$this->assertFalse( $processor->next_tag() );
$this->assertFalse( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) );
$this->assertSame( 11, $processor->get_cursor_move_count() );
$this->assertTrue( $processor->seek( 'document_root' ) );
$this->assertSame( 12, $processor->get_cursor_move_count() );
Expand Down