Merge pull request #1821 from WordPress/add/track-tag-method

Introduce `OD_Tag_Visitor_Context::track_tag()` method as alternative for returning `true` in tag visitor callback

Author: westonruter

plugins/optimization-detective/class-od-tag-visitor-context.php

@@ -16,7 +16,6 @@
  * Context for tag visitors invoked for each tag while walking over a document.
  *
  * @since 0.4.0
- * @access private
  *
  * @property-read OD_URL_Metric_Group_Collection $url_metrics_group_collection Deprecated property accessed via magic getter. Use the url_metric_group_collection property instead.
  */
@@ -25,6 +24,7 @@ final class OD_Tag_Visitor_Context {
 	/**
 	 * HTML tag processor.
 	 *
+	 * @since 0.4.0
 	 * @var OD_HTML_Tag_Processor
 	 * @readonly
 	 */
@@ -33,6 +33,7 @@ final class OD_Tag_Visitor_Context {
 	/**
 	 * URL Metric group collection.
 	 *
+	 * @since 0.4.0
 	 * @var OD_URL_Metric_Group_Collection
 	 * @readonly
 	 */
@@ -41,22 +42,46 @@ final class OD_Tag_Visitor_Context {
 	/**
 	 * Link collection.
 	 *
+	 * @since 0.4.0
 	 * @var OD_Link_Collection
 	 * @readonly
 	 */
 	public $link_collection;
 
+	/**
+	 * Visited tag state.
+	 *
+	 * @since n.e.x.t
+	 * @var OD_Visited_Tag_State
+	 */
+	private $visited_tag_state;
+
 	/**
 	 * Constructor.
 	 *
+	 * @since 0.4.0
+	 *
 	 * @param OD_HTML_Tag_Processor          $processor                   HTML tag processor.
 	 * @param OD_URL_Metric_Group_Collection $url_metric_group_collection URL Metric group collection.
 	 * @param OD_Link_Collection             $link_collection             Link collection.
+	 * @param OD_Visited_Tag_State           $visited_tag_state           Visited tag state.
 	 */
-	public function __construct( OD_HTML_Tag_Processor $processor, OD_URL_Metric_Group_Collection $url_metric_group_collection, OD_Link_Collection $link_collection ) {
+	public function __construct( OD_HTML_Tag_Processor $processor, OD_URL_Metric_Group_Collection $url_metric_group_collection, OD_Link_Collection $link_collection, OD_Visited_Tag_State $visited_tag_state ) {
 		$this->processor                   = $processor;
 		$this->url_metric_group_collection = $url_metric_group_collection;
 		$this->link_collection             = $link_collection;
+		$this->visited_tag_state           = $visited_tag_state;
+	}
+
+	/**
+	 * Marks the tag for being tracked in URL Metrics.
+	 *
+	 * Calling this method from a tag visitor has the same effect as a tag visitor returning `true`.
+	 *
+	 * @since n.e.x.t
+	 */
+	public function track_tag(): void {
+		$this->visited_tag_state->track_tag();
 	}
 
 	/**

plugins/optimization-detective/class-od-tag-visitor-registry.php

@@ -15,7 +15,7 @@
 /**
  * Registry for tag visitors invoked for each tag while walking over a document.
  *
- * @phpstan-type TagVisitorCallback callable( OD_Tag_Visitor_Context ): bool
+ * @phpstan-type TagVisitorCallback callable( OD_Tag_Visitor_Context ): ( bool | void )
  *
  * @implements IteratorAggregate<string, TagVisitorCallback>
  *

plugins/optimization-detective/class-od-visited-tag-state.php

@@ -0,0 +1,67 @@
+<?php
+/**
+ * Optimization Detective: OD_Visited_Tag_State class
+ *
+ * @package optimization-detective
+ * @since n.e.x.t
+ */
+
+// @codeCoverageIgnoreStart
+if ( ! defined( 'ABSPATH' ) ) {
+	exit; // Exit if accessed directly.
+}
+// @codeCoverageIgnoreEnd
+
+/**
+ * State for a tag visitation when visited by tag visitors while walking over a document.
+ *
+ * @since n.e.x.t
+ * @access private
+ */
+final class OD_Visited_Tag_State {
+
+	/**
+	 * Whether the tag should be tracked among the elements in URL Metrics.
+	 *
+	 * @since n.e.x.t
+	 * @var bool
+	 */
+	private $should_track_tag;
+
+	/**
+	 * Constructor.
+	 */
+	public function __construct() {
+		$this->reset();
+	}
+
+	/**
+	 * Marks the tag for being tracked in URL Metrics.
+	 *
+	 * @since n.e.x.t
+	 */
+	public function track_tag(): void {
+		$this->should_track_tag = true;
+	}
+
+	/**
+	 * Whether the tag should be tracked among the elements in URL Metrics.
+	 *
+	 * @since n.e.x.t
+	 * @return bool Whether tracked.
+	 */
+	public function is_tag_tracked(): bool {
+		return $this->should_track_tag;
+	}
+
+	/**
+	 * Resets state.
+	 *
+	 * This should be called after tag visitors have been invoked on a tag.
+	 *
+	 * @since n.e.x.t
+	 */
+	public function reset(): void {
+		$this->should_track_tag = false;
+	}
+}

plugins/optimization-detective/load.php

@@ -123,6 +123,7 @@ class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collec
 		// Optimization logic.
 		require_once __DIR__ . '/class-od-link-collection.php';
 		require_once __DIR__ . '/class-od-tag-visitor-registry.php';
+		require_once __DIR__ . '/class-od-visited-tag-state.php';
 		require_once __DIR__ . '/class-od-tag-visitor-context.php';
 		require_once __DIR__ . '/optimization.php';
 

plugins/optimization-detective/optimization.php

@@ -220,7 +220,8 @@ function od_optimize_template_output_buffer( string $buffer ): string {
 		od_get_url_metric_freshness_ttl()
 	);
 	$link_collection      = new OD_Link_Collection();
-	$tag_visitor_context  = new OD_Tag_Visitor_Context( $processor, $group_collection, $link_collection );
+	$visited_tag_state    = new OD_Visited_Tag_State();
+	$tag_visitor_context  = new OD_Tag_Visitor_Context( $processor, $group_collection, $link_collection, $visited_tag_state );
 	$current_tag_bookmark = 'optimization_detective_current_tag';
 	$visitors             = iterator_to_array( $tag_visitor_registry );
 
@@ -237,8 +238,11 @@ function od_optimize_template_output_buffer( string $buffer ): string {
 		$processor->set_bookmark( $current_tag_bookmark ); // TODO: Should we break if this returns false?
 
 		foreach ( $visitors as $visitor ) {
-			$cursor_move_count      = $processor->get_cursor_move_count();
-			$tracked_in_url_metrics = $visitor( $tag_visitor_context ) || $tracked_in_url_metrics;
+			$cursor_move_count    = $processor->get_cursor_move_count();
+			$visitor_return_value = $visitor( $tag_visitor_context );
+			if ( true === $visitor_return_value ) {
+				$tracked_in_url_metrics = true;
+			}
 
 			// If the visitor traversed HTML tags, we need to go back to this tag so that in the next iteration any
 			// relevant tag visitors may apply, in addition to properly setting the data-od-xpath on this tag below.
@@ -248,11 +252,17 @@ function od_optimize_template_output_buffer( string $buffer ): string {
 		}
 		$processor->release_bookmark( $current_tag_bookmark );
 
+		if ( $visited_tag_state->is_tag_tracked() ) {
+			$tracked_in_url_metrics = true;
+		}
+
 		if ( $tracked_in_url_metrics && $needs_detection ) {
 			// TODO: Replace get_stored_xpath with get_xpath once the transitional period is over.
 			$xpath = $processor->get_stored_xpath();
 			$processor->set_meta_attribute( 'xpath', $xpath );
 		}
+
+		$visited_tag_state->reset();
 	} while ( $processor->next_open_tag() );
 
 	// Send any preload links in a Link response header and in a LINK tag injected at the end of the HEAD.

plugins/optimization-detective/tests/test-cases/tag-track-opt-in/buffer.html

@@ -0,0 +1,14 @@
+<html lang="en">
+	<head>
+		<meta charset="utf-8">
+		<title>...</title>
+	</head>
+	<body>
+		<div id="page">
+			<div id="not-tracked-by-any"></div>
+			<div id="tracked-by-return-true"></div>
+			<div id="tracked-by-track-tag-method"></div>
+			<div id="tracked-by-return-value-and-track-tag-method"></div>
+		</div>
+	</body>
+</html>

plugins/optimization-detective/tests/test-cases/tag-track-opt-in/expected.html

@@ -0,0 +1,50 @@
+<?php
+return static function (): void {
+	add_action(
+		'od_register_tag_visitors',
+		static function ( OD_Tag_Visitor_Registry $registry ): void {
+			$registry->register(
+				'not-tracking-anything-return-false',
+				static function (): bool {
+					return false;
+				}
+			);
+
+			$registry->register(
+				'not-tracking-anything-return-void',
+				static function ( OD_Tag_Visitor_Context $context ): void {}
+			);
+
+			$registry->register(
+				'track-by-return-true',
+				static function ( OD_Tag_Visitor_Context $context ): bool {
+					return in_array(
+						$context->processor->get_attribute( 'id' ),
+						array(
+							'tracked-by-return-true',
+							'tracked-by-return-value-and-track-tag-method',
+						),
+						true
+					);
+				}
+			);
+
+			$registry->register(
+				'track-by-track-tag-method',
+				static function ( OD_Tag_Visitor_Context $context ): void {
+					$should_track = in_array(
+						$context->processor->get_attribute( 'id' ),
+						array(
+							'tracked-by-track-tag-method',
+							'tracked-by-return-value-and-track-tag-method',
+						),
+						true
+					);
+					if ( $should_track ) {
+						$context->track_tag();
+					}
+				}
+			);
+		}
+	);
+};

plugins/optimization-detective/tests/test-cases/tag-track-opt-in/set-up.php

@@ -337,6 +337,11 @@ public function data_provider_test_od_optimize_template_output_buffer(): array {
 	 * @covers ::od_is_response_html_content_type
 	 * @covers OD_Tag_Visitor_Context::__construct
 	 * @covers OD_Tag_Visitor_Context::__get
+	 * @covers OD_Tag_Visitor_Context::track_tag
+	 * @covers OD_Visited_Tag_State::__construct
+	 * @covers OD_Visited_Tag_State::track_tag
+	 * @covers OD_Visited_Tag_State::is_tag_tracked
+	 * @covers OD_Visited_Tag_State::reset
 	 *
 	 * @dataProvider data_provider_test_od_optimize_template_output_buffer
 	 *
@@ -379,9 +384,11 @@ function ( OD_Tag_Visitor_Context $context ): bool {
 			function ( OD_Tag_Visitor_Registry $tag_visitor_registry ): void {
 				$tag_visitor_registry->register(
 					'video',
-					function ( OD_Tag_Visitor_Context $context ): bool {
+					function ( OD_Tag_Visitor_Context $context ): void {
 						$this->assertFalse( $context->processor->is_tag_closer() );
-						return $context->processor->get_tag() === 'VIDEO';
+						if ( $context->processor->get_tag() === 'VIDEO' ) {
+							$context->track_tag();
+						}
 					}
 				);
 			}