Merge pull request #1872 from WordPress/try/allow-next-tag-query

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

Author: westonruter

plugins/embed-optimizer/hooks.php

@@ -187,6 +187,11 @@ 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.
@@ -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( $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;

plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php

@@ -226,7 +226,11 @@ 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() ) {
+		// 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.

plugins/optimization-detective/class-od-html-tag-processor.php

@@ -246,43 +246,40 @@ 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 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 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, 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 ) {
+			$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' )
+			);
 		}
-
-		// 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.
 	 *
+	 * 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.
 	 */
 	public function next_open_tag(): bool {
-		while ( $this->next_tag() ) {
-			if ( ! $this->is_tag_closer() ) {
-				return true;
-			}
-		}
-		return false;
+		return $this->next_tag( array( 'tag_closers' => 'skip' ) );
 	}
 
 	/**
@@ -719,11 +716,16 @@ 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.
 	 */
-	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'] )
 			&&

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;

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()
 	) ) {
@@ -284,7 +284,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 )
+			||
+			$processor->is_admin_bar()
+		) {
 			continue;
 		}
 
@@ -317,7 +322,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 ) {

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.' );
 				}
 			);