diff --git a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed/set-up.php b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed/set-up.php index 59e3337af5..c209530f45 100644 --- a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed/set-up.php +++ b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed/set-up.php @@ -59,7 +59,7 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool { 'href' => $poster, ) ); - $processor->set_attribute( 'preload', 'auto' ); + $processor->set_attribute( 'preload', 'auto' ); // TODO: For some reason the order here is different with data-od-xpath. } else { $processor->set_attribute( 'preload', 'none' ); } diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php index de9cffd163..ae4ae01f63 100644 --- a/plugins/optimization-detective/class-od-html-tag-processor.php +++ b/plugins/optimization-detective/class-od-html-tag-processor.php @@ -236,13 +236,49 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor { /** * Count for the number of times that the cursor was moved. * + * The use of this has been replaced with {@see self::$cursor_at_bookmark}. + * * @since 0.6.0 * @var non-negative-int * @see self::next_token() - * @see self::seek() */ private $cursor_move_count = 0; + /** + * The bookmark that the cursor is currently known to be at. + * + * This is used as a backport for the WP 6.8 fix in {@link https://core.trac.wordpress.org/ticket/62085} which + * no-ops seek() calls in which the cursor is already at the provided bookmark. + * + * @since n.e.x.t + * @var string|null + * @see self::next_token() + * @see self::seek() + * @see self::set_bookmark() + */ + private $cursor_at_bookmark = null; + + /** + * Whether current WP version has the no-op check in seek() for when the cursor is already at the provided bookmark. + * + * @since n.e.x.t + * @see self::seek() + * @var bool + */ + private $has_seek_noop; + + /** + * Constructor. + * + * @since n.e.x.t + * + * @param string $html HTML to process. + */ + public function __construct( string $html ) { + parent::__construct( $html ); + $this->has_seek_noop = version_compare( (string) strtok( get_bloginfo( 'version' ), '-' ), '6.8', '>=' ); + } + /** * Finds the next tag. * @@ -320,6 +356,7 @@ public function expects_closer( ?string $tag_name = null ): bool { public function next_token(): bool { $this->current_stored_xpath = null; // Clear cache. $this->current_xpath = null; // Clear cache. + $this->cursor_at_bookmark = null; ++$this->cursor_move_count; if ( ! parent::next_token() ) { $this->open_stack_tags = array(); @@ -423,12 +460,13 @@ public function next_token(): bool { * Gets the number of times the cursor has moved. * * @since 0.6.0 - * @see self::next_token() - * @see self::seek() + * @deprecated n.e.x.t The use of this has been replaced with {@see self::$cursor_at_bookmark}. + * @codeCoverageIgnore * * @return non-negative-int Count of times the cursor has moved. */ public function get_cursor_move_count(): int { + _deprecated_function( __METHOD__, 'optimization-detective n.e.x.t' ); return $this->cursor_move_count; } @@ -509,8 +547,14 @@ public function get_current_depth(): int { * @return bool Whether the internal cursor was successfully moved to the bookmark's location. */ public function seek( $bookmark_name ): bool { + // This is only needed prior to WP 6.8 per . + if ( ! $this->has_seek_noop && $bookmark_name === $this->cursor_at_bookmark ) { + return true; + } + $result = parent::seek( $bookmark_name ); if ( $result ) { + $this->cursor_at_bookmark = $bookmark_name; $this->open_stack_tags = $this->bookmarked_open_stacks[ $bookmark_name ]['tags']; $this->open_stack_attributes = $this->bookmarked_open_stacks[ $bookmark_name ]['attributes']; $this->open_stack_indices = $this->bookmarked_open_stacks[ $bookmark_name ]['indices']; @@ -530,6 +574,8 @@ public function seek( $bookmark_name ): bool { public function set_bookmark( $name ): bool { $result = parent::set_bookmark( $name ); if ( $result ) { + $this->cursor_at_bookmark = $name; // Only needed prior to WP 6.8 per . + $this->bookmarked_open_stacks[ $name ] = array( 'tags' => $this->open_stack_tags, 'attributes' => $this->open_stack_attributes, @@ -558,6 +604,10 @@ public function release_bookmark( $name ): bool { return false; } unset( $this->bookmarked_open_stacks[ $name ] ); + if ( $this->cursor_at_bookmark === $name ) { + // Only needed prior to WP 6.8 per . + $this->cursor_at_bookmark = null; + } return parent::release_bookmark( $name ); } diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index a793a67594..cceb8f6328 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -363,7 +363,6 @@ 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(); $visitor_return_value = $visitor( $tag_visitor_context ); if ( true === $visitor_return_value ) { $tracked_in_url_metrics = true; @@ -371,9 +370,7 @@ function od_optimize_template_output_buffer( string $buffer ): string { // 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 ( $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? - } + $processor->seek( $current_tag_bookmark ); // TODO: Should this break out of the optimization loop if it returns false? } $processor->release_bookmark( $current_tag_bookmark ); 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..db29077ac1 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 @@ -419,6 +419,7 @@ public function data_provider_sample_documents(): array { /** * Test next_tag(), next_token(), get_xpath(), expects_closer(). * + * @covers ::__construct * @covers ::next_open_tag * @covers ::next_tag * @covers ::next_token @@ -682,14 +683,9 @@ public function test_bookmarking_and_seeking(): void { ); $actual_figure_contents = array(); - $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() && @@ -773,56 +769,6 @@ public function test_bookmarking_and_seeking(): void { // TODO: Try adding too many bookmarks. } - /** - * Test get_cursor_move_count(). - * - * @covers ::get_cursor_move_count - */ - public function test_get_cursor_move_count(): void { - $processor = new OD_HTML_Tag_Processor( - trim( - ' - - - - - ' - ) - ); - $this->assertSame( 0, $processor->get_cursor_move_count() ); - $this->assertTrue( $processor->next_tag() ); - $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->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->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->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->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->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->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. - } - /** * Export an array as a PHP literal to use as a snapshot. *