Merge pull request #1478 from WordPress/improve/cursor-move-count

Introduce get_cursor_move_count() to use instead of get_seek_count() and get_next_token_count()
WordPress · Aug 21, 2024 · e414842 · e414842
2 parents 81842e5 + 5b683b5
commit e414842
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 38 deletions.
diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php
@@ -179,13 +179,14 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
 	private $buffered_text_replacements = array();
 
 	/**
-	 * Count for the number of times next_token() was called
+	 * Count for the number of times that the cursor was moved.
 	 *
-	 * @since 0.4.1
+	 * @since n.e.x.t
 	 * @var int
 	 * @see self::next_token()
+	 * @see self::seek()
 	 */
-	private $next_token_count = 0;
+	private $cursor_move_count = 0;
 
 	/**
 	 * Finds the next tag.
@@ -258,7 +259,7 @@ public function expects_closer( ?string $tag_name = null ): bool {
 	 */
 	public function next_token(): bool {
 		$this->current_xpath = null; // Clear cache.
-		++$this->next_token_count;
+		++$this->cursor_move_count;
 		if ( ! parent::next_token() ) {
 			$this->open_stack_tags    = array();
 			$this->open_stack_indices = array();
@@ -339,15 +340,16 @@ public function next_token(): bool {
 	}
 
 	/**
-	 * Gets the number of times next_token() was called.
+	 * Gets the number of times the cursor has moved.
 	 *
-	 * @since 0.4.1
+	 * @since n.e.x.t
 	 * @see self::next_token()
+	 * @see self::seek()
 	 *
-	 * @return int Count of next_token() calls.
+	 * @return int Count of times the cursor has moved.
 	 */
-	public function get_next_token_count(): int {
-		return $this->next_token_count;
+	public function get_cursor_move_count(): int {
+		return $this->cursor_move_count;
 	}
 
 	/**
@@ -376,7 +378,9 @@ public function set_attribute( $name, $value ): bool { // phpcs:ignore SlevomatC
 	/**
 	 * Sets a meta attribute.
 	 *
-	 * All meta attributes are prefixed with 'data-od-'.
+	 * All meta attributes are prefixed with data-od-.
+	 *
+	 * @since 0.4.0
 	 *
 	 * @param string      $name  Meta attribute name.
 	 * @param string|true $value Value.
@@ -433,18 +437,6 @@ public function seek( $bookmark_name ): bool {
 		return $result;
 	}
 
-	/**
-	 * Gets the number of times seek() was called.
-	 *
-	 * @since 0.4.1
-	 * @see self::seek()
-	 *
-	 * @return int Count of seek() calls.
-	 */
-	public function get_seek_count(): int {
-		return $this->seek_count;
-	}
-
 	/**
 	 * Sets a bookmark in the HTML document.
 	 *
@@ -567,6 +559,8 @@ public function append_body_html( string $html ): void {
 	/**
 	 * Returns the string representation of the HTML Tag Processor.
 	 *
+	 * @since 0.4.0
+	 *
 	 * @return string The processed HTML.
 	 */
 	public function get_updated_html(): string {

diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php
@@ -219,13 +219,12 @@ 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 ) {
-			$seek_count       = $processor->get_seek_count();
-			$next_token_count = $processor->get_next_token_count();
-			$did_visit        = $visitor( $tag_visitor_context ) || $did_visit;
+			$cursor_move_count = $processor->get_cursor_move_count();
+			$did_visit         = $visitor( $tag_visitor_context ) || $did_visit;
 
 			// 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.
-			if ( $seek_count !== $processor->get_seek_count() || $next_token_count !== $processor->get_next_token_count() ) {
+			if ( $cursor_move_count !== $processor->get_cursor_move_count() ) {
 				$processor->seek( $current_tag_bookmark ); // TODO: Should this break out of the optimization loop if it returns false?
 			}
 		}

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
@@ -492,7 +492,8 @@ public function test_html_tag_processor_wrapper_methods(): void {
 	 */
 	public function test_bookmarking_and_seeking(): void {
 		$processor = new OD_HTML_Tag_Processor(
-			'
+			trim(
+				'
 				<html>
 					<head></head>
 					<body>
@@ -507,14 +508,19 @@ public function test_bookmarking_and_seeking(): void {
 						<img src="https://example.com/foo.jpg">
 					</body>
 				</html>
-			'
+				'
+			)
 		);
 
 		$actual_figure_contents = array();
-		$this->assertSame( 0, $processor->get_seek_count() );
+		$last_cursor_move_count = $processor->get_cursor_move_count();
+		$this->assertSame( 0, $last_cursor_move_count );
 
 		$bookmarks = array();
 		while ( $processor->next_open_tag() ) {
+			$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;
 			if (
 				'FIGURE' === $processor->get_tag()
 				&&
@@ -573,7 +579,6 @@ public function test_bookmarking_and_seeking(): void {
 				'depth' => $processor->get_current_depth(),
 			);
 		}
-		$this->assertSame( count( $bookmarks ), $processor->get_seek_count() );
 
 		$this->assertSame( $expected_figure_contents, $sought_actual_contents );
 
@@ -597,11 +602,11 @@ public function test_bookmarking_and_seeking(): void {
 	}
 
 	/**
-	 * Test get_seek_count.
+	 * Test get_cursor_move_count().
 	 *
-	 * @covers ::get_seek_count
+	 * @covers ::get_cursor_move_count
 	 */
-	public function test_get_next_token_count(): void {
+	public function test_get_cursor_move_count(): void {
 		$processor = new OD_HTML_Tag_Processor(
 			trim(
 				'
@@ -612,20 +617,38 @@ public function test_get_next_token_count(): void {
 				'
 			)
 		);
-		$this->assertSame( 0, $processor->get_next_token_count() );
+		$this->assertSame( 0, $processor->get_cursor_move_count() );
 		$this->assertTrue( $processor->next_tag() );
 		$this->assertSame( 'HTML', $processor->get_tag() );
-		$this->assertSame( 1, $processor->get_next_token_count() );
+		$this->assertTrue( $processor->set_bookmark( 'document_root' ) );
+		$this->assertSame( 1, $processor->get_cursor_move_count() );
 		$this->assertTrue( $processor->next_tag() );
 		$this->assertSame( 'HEAD', $processor->get_tag() );
-		$this->assertSame( 3, $processor->get_next_token_count() ); // Note that next_token() call #2 was for the whitespace between <html> and <head>.
+		$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->assertSame( 'HEAD', $processor->get_tag() );
 		$this->assertTrue( $processor->is_tag_closer() );
-		$this->assertSame( 4, $processor->get_next_token_count() );
+		$this->assertSame( 4, $processor->get_cursor_move_count() );
+		$this->assertTrue( $processor->next_tag() );
+		$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->assertSame( 'BODY', $processor->get_tag() );
-		$this->assertSame( 6, $processor->get_next_token_count() ); // Note that next_token() call #5 was for the whitespace between </head> and <body>.
+		$this->assertTrue( $processor->is_tag_closer() );
+		$this->assertSame( 7, $processor->get_cursor_move_count() );
+		$this->assertTrue( $processor->next_tag() );
+		$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->assertSame( 10, $processor->get_cursor_move_count() );
+		$this->assertFalse( $processor->next_tag() );
+		$this->assertSame( 11, $processor->get_cursor_move_count() );
+		$this->assertTrue( $processor->seek( 'document_root' ) );
+		$this->assertSame( 12, $processor->get_cursor_move_count() );
+		$this->setExpectedIncorrectUsage( 'WP_HTML_Tag_Processor::seek' );
+		$this->assertFalse( $processor->seek( 'does_not_exist' ) );
+		$this->assertSame( 12, $processor->get_cursor_move_count() ); // The bookmark does not exist so no change.
 	}
 
 	/**