From 069dc53fabf187f51b920007644374be6af2d8eb Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 16 Feb 2025 11:35:01 -0800 Subject: [PATCH 1/8] Try: Allow queries to be passed to next_tag() --- .../class-od-html-tag-processor.php | 21 ++++++++----------- .../optimization-detective/optimization.php | 9 ++++++-- .../tests/test-cases/admin-bar/set-up.php | 3 ++- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php index de9cffd163..4e0f490520 100644 --- a/plugins/optimization-detective/class-od-html-tag-processor.php +++ b/plugins/optimization-detective/class-od-html-tag-processor.php @@ -246,38 +246,35 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor { /** * Finds the next tag. * - * Unlike the base class, this subclass disallows querying. This is to ensure the breadcrumbs can be tracked. - * It will _always_ visit tag closers. + * Unlike the base class, this subclass visits tag closers by default. * * @inheritDoc * @since 0.4.0 + * @since n.e.x.t Passing a $query is now allowed. * - * @param array{tag_name?: string|null, match_offset?: int|null, class_name?: string|null, tag_closers?: string|null}|null $query Query, but only null is accepted for this subclass. + * @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 ) { - throw new InvalidArgumentException( esc_html__( 'Processor subclass does not support queries.', 'optimization-detective' ) ); + if ( null === $query ) { + // For back-compat with tag visitor which previously relied next_tag() always visiting tag closers. + $query = array( array( 'tag_closers' => 'visit' ) ); } - - // Elements in the Admin Bar are not relevant for optimization, so this loop ensures that no tags in the Admin Bar are visited. - do { - $matched = parent::next_tag( array( 'tag_closers' => 'visit' ) ); - } while ( $matched && $this->is_admin_bar() ); - return $matched; + return parent::next_tag( $query ); } /** * 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() ) { + while ( $this->next_tag( array( 'tag_closers' => 'visit' ) ) ) { if ( ! $this->is_tag_closer() ) { return true; } diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index a793a67594..6b6b215e02 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -355,7 +355,12 @@ function od_optimize_template_output_buffer( string $buffer ): string { do { // Never process anything inside NOSCRIPT since it will never show up in the DOM when scripting is enabled, and thus it can never be detected nor measured. - if ( in_array( 'NOSCRIPT', $processor->get_breadcrumbs(), true ) ) { + // Similarly, elements in the Admin Bar are not relevant for optimization, so this loop ensures that no tags in the Admin Bar are visited. + if ( + in_array( 'NOSCRIPT', $processor->get_breadcrumbs(), true ) + || + str_starts_with( $processor->get_xpath(), "/HTML/BODY/DIV[@id='wpadminbar']" ) + ) { continue; } @@ -388,7 +393,7 @@ function od_optimize_template_output_buffer( string $buffer ): string { } $visited_tag_state->reset(); - } while ( $processor->next_open_tag() ); + } while ( $processor->next_tag( array( 'tag_closers' => 'skip' ) ) ); // Send any preload links in a Link response header and in a LINK tag injected at the end of the HEAD. if ( count( $link_collection ) > 0 ) { diff --git a/plugins/optimization-detective/tests/test-cases/admin-bar/set-up.php b/plugins/optimization-detective/tests/test-cases/admin-bar/set-up.php index b0c84bf3b9..a8889e6ee3 100644 --- a/plugins/optimization-detective/tests/test-cases/admin-bar/set-up.php +++ b/plugins/optimization-detective/tests/test-cases/admin-bar/set-up.php @@ -6,7 +6,8 @@ static function ( OD_Tag_Visitor_Registry $registry ) use ( $test_case ): void { $registry->register( 'everything', static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): void { - $test_case->assertNotEquals( 'wpadminbar', $context->processor->get_attribute( 'id' ) ); + $test_case->assertStringNotContainsString( 'wpadminbar', $context->processor->get_xpath() ); + $test_case->assertNotEquals( 'wpadminbar', $context->processor->get_attribute( 'id' ), $context->processor->get_xpath() ); $test_case->assertNull( $context->url_metrics_id, 'Expected url_metrics_id to be null since no od_url_metrics_post exists for this URL.' ); } ); From 5aae390dbc042e6e273cc09769d8b648f7ac553c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 17 Feb 2025 12:19:33 -0800 Subject: [PATCH 2/8] Begin transition for next_tag() to align with base class behavior --- plugins/embed-optimizer/hooks.php | 11 +- ...lass-image-prioritizer-img-tag-visitor.php | 2 +- plugins/image-prioritizer/helper.php | 9 +- .../class-od-html-tag-processor.php | 19 ++- plugins/optimization-detective/load.php | 4 +- .../optimization-detective/optimization.php | 4 +- .../test-class-od-html-tag-processor.php | 118 +++++++++++++----- 7 files changed, 111 insertions(+), 56 deletions(-) diff --git a/plugins/embed-optimizer/hooks.php b/plugins/embed-optimizer/hooks.php index 2ee56e818a..20d009d5f4 100644 --- a/plugins/embed-optimizer/hooks.php +++ b/plugins/embed-optimizer/hooks.php @@ -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 { @@ -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; diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 58ab0516be..41f87857f7 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -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. diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 998a27a2f4..d0cf283260 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -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 { diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php index 4e0f490520..1832b504ac 100644 --- a/plugins/optimization-detective/class-od-html-tag-processor.php +++ b/plugins/optimization-detective/class-od-html-tag-processor.php @@ -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 ); } @@ -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' ) ); } /** diff --git a/plugins/optimization-detective/load.php b/plugins/optimization-detective/load.php index e8cfc0ee81..8c9faa1c17 100644 --- a/plugins/optimization-detective/load.php +++ b/plugins/optimization-detective/load.php @@ -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 @@ -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; diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 6b6b215e02..0b5bfca9c3 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -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() ) ) { @@ -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; } diff --git a/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php b/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php index 0c6953c654..f7b2d873ce 100644 --- a/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php +++ b/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php @@ -341,18 +341,20 @@ public function data_provider_sample_documents(): array { ', - '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' ), ), ), @@ -392,12 +394,14 @@ public function data_provider_sample_documents(): array { ', - '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' ), @@ -405,12 +409,12 @@ public function data_provider_sample_documents(): array { '/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' ), ), ), ); @@ -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(); @@ -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( '' ); - $p->next_tag( array( 'tag_name' => 'HTML' ) ); + $html = ' + + + + + + +
+

Hello world

+
+ +
+
+ +
+
+ Foo! +
+
+ + + '; + + $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() ); } /** @@ -484,7 +534,7 @@ public function test_next_tag_with_query(): void { public function test_expects_closer(): void { $p = new OD_HTML_Tag_Processor( '
' ); $this->assertFalse( $p->expects_closer() ); - while ( $p->next_tag() ) { + while ( $p->next_tag( array( 'tag_closers' => 'visit' ) ) ) { if ( 'BODY' === $p->get_tag() ) { break; } @@ -492,7 +542,7 @@ public function test_expects_closer(): void { $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' ) ); @@ -594,7 +644,7 @@ public function test_get_updated_html_when_out_of_bookmarks(): void { '; $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++ ) { @@ -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' ) ) ); } } @@ -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 and . - $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 and . - $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 and . - $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() ); From 6e9bb16543521e5fc0f5cf1dedbc33ac27d798ee Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 17 Feb 2025 12:24:06 -0800 Subject: [PATCH 3/8] Use is_admin_bar since faaster than computing XPath for every tag --- plugins/optimization-detective/class-od-html-tag-processor.php | 2 +- plugins/optimization-detective/optimization.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php index 1832b504ac..ab4040173f 100644 --- a/plugins/optimization-detective/class-od-html-tag-processor.php +++ b/plugins/optimization-detective/class-od-html-tag-processor.php @@ -715,7 +715,7 @@ public function get_stored_xpath(): string { * * @return bool Whether at or inside the admin bar. */ - private function is_admin_bar(): bool { + public function is_admin_bar(): bool { return ( isset( $this->open_stack_tags[2], $this->open_stack_attributes[2]['id'] ) && diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 0b5bfca9c3..292a02fc71 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -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_stored_xpath(), "/HTML/BODY/DIV[@id='wpadminbar']" ) + $processor->is_admin_bar() ) { continue; } From b45e4bb859737b03a6d0ff7cbb55762fdd87a89d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 22 Feb 2025 09:33:15 +0800 Subject: [PATCH 4/8] Opt to vary query arg for next_tag() by OD version Co-authored-by: felixarntz --- plugins/embed-optimizer/hooks.php | 17 +++++++++-------- .../class-image-prioritizer-img-tag-visitor.php | 7 ++++++- plugins/image-prioritizer/helper.php | 9 ++------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/plugins/embed-optimizer/hooks.php b/plugins/embed-optimizer/hooks.php index 20d009d5f4..37f47f347a 100644 --- a/plugins/embed-optimizer/hooks.php +++ b/plugins/embed-optimizer/hooks.php @@ -44,13 +44,8 @@ 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 { - 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 - ) { + $required_od_version = '0.9.0'; + if ( version_compare( (string) strtok( $optimization_detective_version, '-' ), $required_od_version, '<' ) ) { add_action( 'admin_notices', static function (): void { @@ -192,6 +187,12 @@ function embed_optimizer_update_markup( WP_HTML_Tag_Processor $html_processor, b $trigger_error = static function ( string $message ) use ( $function_name ): void { wp_trigger_error( $function_name, esc_html( $message ) ); }; + + // As of 1.0.0-beta3, next_tag() allows $query and is beginning to migrate to skip tag closers by default. + // In versions prior to this, the method always visited closers and passing a $query actually threw an exception. + $tag_query = version_compare( OPTIMIZATION_DETECTIVE_VERSION, '1.0.0-beta3', '>=' ) + ? array( 'tag_closers' => 'visit' ) + : null; try { /* * Determine how to lazy load the embed. @@ -258,7 +259,7 @@ function embed_optimizer_update_markup( WP_HTML_Tag_Processor $html_processor, b } } } - } while ( $html_processor->next_tag( array( 'tag_closers' => 'visit' ) ) ); + } while ( $html_processor->next_tag( $tag_query ) ); // 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; diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 41f87857f7..7cd47b90df 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -226,7 +226,12 @@ 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( array( 'tag_closers' => 'visit' ) ) ) { + // As of 1.0.0-beta3, next_tag() allows $query and is beginning to migrate to skip tag closers by default. + // In versions prior to this, the method always visited closers and passing a $query actually threw an exception. + $tag_query = version_compare( OPTIMIZATION_DETECTIVE_VERSION, '1.0.0-beta3', '>=' ) + ? array( 'tag_closers' => 'visit' ) + : null; + while ( $processor->next_tag( $tag_query ) ) { $tag = $processor->get_tag(); // If we reached the closing PICTURE tag, break. diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index d0cf283260..998a27a2f4 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -21,13 +21,8 @@ * @param string $optimization_detective_version Current version of the optimization detective plugin. */ function image_prioritizer_init( string $optimization_detective_version ): void { - 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 - ) { + $required_od_version = '0.9.0'; + if ( ! version_compare( (string) strtok( $optimization_detective_version, '-' ), $required_od_version, '>=' ) ) { add_action( 'admin_notices', static function (): void { From 4cfe451653e99aa1001f401a833c5d67c65f6ba1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 22 Feb 2025 10:24:45 +0800 Subject: [PATCH 5/8] Test that next_tag() without query defaults to visiting tag closers (for now) --- .../test-class-od-html-tag-processor.php | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php b/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php index f7b2d873ce..7c21fc639c 100644 --- a/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php +++ b/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php @@ -526,6 +526,57 @@ public function test_next_tag_with_query(): void { $this->assertSame( 5, $p->get_current_depth() ); } + /** + * Test next_tag() without passing query. + * + * @todo This test will need to be updated once next_tag() defaults to not visiting closers by default. + * + * @covers ::next_tag + */ + public function test_next_tag_without_query(): void { + $html = ' + + + + + + + + '; + + $p = new OD_HTML_Tag_Processor( $html ); + $this->assertTrue( $p->next_tag() ); + $this->assertSame( 'HTML', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_tag() ); + $this->assertSame( 'HEAD', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_tag() ); + $this->assertSame( 'TITLE', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + // Note: This is not a closing TITLE tag as would be expected since it is special. + $this->assertTrue( $p->next_tag() ); + $this->assertSame( 'HEAD', $p->get_tag() ); + $this->assertTrue( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_tag() ); + $this->assertSame( 'BODY', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_tag() ); + $this->assertSame( 'BODY', $p->get_tag() ); + $this->assertTrue( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_tag() ); + $this->assertSame( 'HTML', $p->get_tag() ); + $this->assertTrue( $p->is_tag_closer() ); + + $this->assertFalse( $p->next_tag() ); + } + /** * Test expects_closer(). * From 2c6271bbb1f0198a007482e856448ee0f3cf51f0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 22 Feb 2025 11:02:03 +0800 Subject: [PATCH 6/8] Improve code coverage --- plugins/embed-optimizer/hooks.php | 3 +-- .../class-image-prioritizer-img-tag-visitor.php | 3 +-- plugins/optimization-detective/tests/test-optimization.php | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/embed-optimizer/hooks.php b/plugins/embed-optimizer/hooks.php index 37f47f347a..c01b99d46f 100644 --- a/plugins/embed-optimizer/hooks.php +++ b/plugins/embed-optimizer/hooks.php @@ -191,8 +191,7 @@ function embed_optimizer_update_markup( WP_HTML_Tag_Processor $html_processor, b // As of 1.0.0-beta3, next_tag() allows $query and is beginning to migrate to skip tag closers by default. // In versions prior to this, the method always visited closers and passing a $query actually threw an exception. $tag_query = version_compare( OPTIMIZATION_DETECTIVE_VERSION, '1.0.0-beta3', '>=' ) - ? array( 'tag_closers' => 'visit' ) - : null; + ? array( 'tag_closers' => 'visit' ) : null; try { /* * Determine how to lazy load the embed. diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 7cd47b90df..6418c15dc2 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -229,8 +229,7 @@ private function process_picture( OD_HTML_Tag_Processor $processor, OD_Tag_Visit // As of 1.0.0-beta3, next_tag() allows $query and is beginning to migrate to skip tag closers by default. // In versions prior to this, the method always visited closers and passing a $query actually threw an exception. $tag_query = version_compare( OPTIMIZATION_DETECTIVE_VERSION, '1.0.0-beta3', '>=' ) - ? array( 'tag_closers' => 'visit' ) - : null; + ? array( 'tag_closers' => 'visit' ) : null; while ( $processor->next_tag( $tag_query ) ) { $tag = $processor->get_tag(); diff --git a/plugins/optimization-detective/tests/test-optimization.php b/plugins/optimization-detective/tests/test-optimization.php index 2c54fc333e..3347317866 100644 --- a/plugins/optimization-detective/tests/test-optimization.php +++ b/plugins/optimization-detective/tests/test-optimization.php @@ -376,6 +376,7 @@ public function data_provider_test_od_optimize_template_output_buffer(): array { * @covers OD_Visited_Tag_State::track_tag * @covers OD_Visited_Tag_State::is_tag_tracked * @covers OD_Visited_Tag_State::reset + * @covers OD_HTML_Tag_Processor::is_admin_bar * * @dataProvider data_provider_test_od_optimize_template_output_buffer * From 7a7411ad00a91d3b0cd7adda462ee8ea7ff69397 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 22 Feb 2025 13:17:28 +0800 Subject: [PATCH 7/8] Soft-deprecate next_open_tag() --- .../class-od-html-tag-processor.php | 9 +++- .../test-class-od-html-tag-processor.php | 44 +++++++++++++++++-- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php index ab4040173f..e203ba97fe 100644 --- a/plugins/optimization-detective/class-od-html-tag-processor.php +++ b/plugins/optimization-detective/class-od-html-tag-processor.php @@ -246,11 +246,13 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor { /** * Finds the next tag. * - * Unlike the base class, this subclass visits tag closers by default. + * Unlike the base class, this subclass currently visits tag closers by default. + * However, for the 1.0.0 release this method will behave the same as the method in + * the base class, where it skips tag closers by default. * * @inheritDoc * @since 0.4.0 - * @since n.e.x.t Passing a $query is now allowed. In a future release, this will default to skipping tag closers. + * @since n.e.x.t Passing a $query is now allowed. In the 1.0.0 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. @@ -269,7 +271,10 @@ public function next_tag( $query = null ): bool { /** * Finds the next open tag. * + * This method will soon be equivalent to calling {@see self::next_tag()} without passing any `$query`. + * * @since 0.4.0 + * @deprecated n.e.x.t Use {@see self::next_tag()} instead. * * @return bool Whether a tag was matched. */ diff --git a/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php b/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php index 7c21fc639c..a86f038e52 100644 --- a/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php +++ b/plugins/optimization-detective/tests/test-class-od-html-tag-processor.php @@ -577,6 +577,42 @@ public function test_next_tag_without_query(): void { $this->assertFalse( $p->next_tag() ); } + /** + * Test next_open_tag(). + * + * @covers ::next_open_tag + */ + public function test_next_open_tag(): void { + $html = ' + + + + + + + + '; + + $p = new OD_HTML_Tag_Processor( $html ); + $this->assertTrue( $p->next_open_tag() ); // @phpstan-ignore method.deprecated + $this->assertSame( 'HTML', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_open_tag() ); // @phpstan-ignore method.deprecated + $this->assertSame( 'HEAD', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_open_tag() ); // @phpstan-ignore method.deprecated + $this->assertSame( 'TITLE', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + $this->assertTrue( $p->next_open_tag() ); // @phpstan-ignore method.deprecated + $this->assertSame( 'BODY', $p->get_tag() ); + $this->assertFalse( $p->is_tag_closer() ); + + $this->assertFalse( $p->next_open_tag() ); // @phpstan-ignore method.deprecated + } + /** * Test expects_closer(). * @@ -632,7 +668,7 @@ public function test_append_head_and_body_html(): void { $saw_head = false; $saw_body = false; $did_seek = false; - while ( $processor->next_open_tag() ) { + while ( $processor->next_tag( array( 'tag_closers' => 'skip' ) ) ) { $this->assertStringNotContainsString( $head_injected, $processor->get_updated_html(), 'Only expecting end-of-head injection once document was finalized.' ); $this->assertStringNotContainsString( $body_injected, $processor->get_updated_html(), 'Only expecting end-of-body injection once document was finalized.' ); $tag = $processor->get_tag(); @@ -708,7 +744,7 @@ public function test_get_updated_html_when_out_of_bookmarks(): void { $saw_head = false; $saw_body = false; - while ( $processor->next_open_tag() ) { + while ( $processor->next_tag( array( 'tag_closers' => 'skip' ) ) ) { $tag = $processor->get_tag(); if ( 'HEAD' === $tag ) { $saw_head = true; @@ -732,7 +768,7 @@ public function test_get_updated_html_when_out_of_bookmarks(): void { */ public function test_html_tag_processor_wrapper_methods(): void { $processor = new OD_HTML_Tag_Processor( '' ); - while ( $processor->next_open_tag() ) { + while ( $processor->next_tag( array( 'tag_closers' => 'skip' ) ) ) { $open_tag = $processor->get_tag(); if ( 'HTML' === $open_tag ) { $processor->set_attribute( 'lang', 'es' ); @@ -787,7 +823,7 @@ public function test_bookmarking_and_seeking(): void { $this->assertSame( 0, $last_cursor_move_count ); $bookmarks = array(); - while ( $processor->next_open_tag() ) { + while ( $processor->next_tag( array( 'tag_closers' => 'skip' ) ) ) { $this_cursor_move_count = $processor->get_cursor_move_count(); $this->assertGreaterThan( $last_cursor_move_count, $this_cursor_move_count ); $last_cursor_move_count = $this_cursor_move_count; From 2f97645a39a8c74bedaa0f7b4d23c2f590338850 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 25 Feb 2025 14:44:35 -0800 Subject: [PATCH 8/8] Mark is_admin_bar as private --- .../optimization-detective/class-od-html-tag-processor.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php index e203ba97fe..644bcfce86 100644 --- a/plugins/optimization-detective/class-od-html-tag-processor.php +++ b/plugins/optimization-detective/class-od-html-tag-processor.php @@ -716,7 +716,12 @@ public function get_stored_xpath(): string { /** * Returns whether the processor is currently at or inside the admin bar. * + * This is only intended to be used internally by Optimization Detective as part of the "optimization loop". Tag + * visitors should not rely on this method as it may be deprecated in the future, especially with a migration to + * WP_HTML_Processor after {@link https://core.trac.wordpress.org/ticket/63020} is implemented. + * * @since 1.0.0 + * @access private * * @return bool Whether at or inside the admin bar. */