Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logic for seeking during optimization loop to prevent emitting seek() notices #1376

Merged
merged 8 commits into from
Jul 19, 2024
2 changes: 1 addition & 1 deletion plugins/auto-sizes/tests/test-optimization-detective.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public function data_provider_test_od_optimize_template_output_buffer(): array {
'intersectionRatio' => 1,
),
'buffer' => '<img src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" loading="lazy" srcset="https://example.com/foo-480w.jpg 480w, https://example.com/foo-800w.jpg 800w" sizes="auto, (max-width: 600px) 480px, 800px">',
'expected' => '<img data-od-replaced-sizes="auto, (max-width: 600px) 480px, 800px" data-od-removed-loading="lazy" src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" srcset="https://example.com/foo-480w.jpg 480w, https://example.com/foo-800w.jpg 800w" sizes="(max-width: 600px) 480px, 800px">',
'expected' => '<img data-od-removed-loading="lazy" data-od-replaced-sizes="auto, (max-width: 600px) 480px, 800px" src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" srcset="https://example.com/foo-480w.jpg 480w, https://example.com/foo-800w.jpg 800w" sizes="(max-width: 600px) 480px, 800px">',
),
);
}
Expand Down
18 changes: 16 additions & 2 deletions plugins/optimization-detective/class-od-html-tag-processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,17 @@ public function seek( $bookmark_name ): bool {
return $result;
}

/**
* Gets the seek count.
*
* @since n.e.x.t
*
* @return int Seek count.
*/
public function get_seek_count(): int {
return $this->seek_count;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

}

/**
* Sets a bookmark in the HTML document.
*
Expand Down Expand Up @@ -539,8 +550,11 @@ public function get_updated_html(): string {
}
if ( ! $this->has_bookmark( $bookmark ) ) {
$this->warn(
/* translators: %s is the bookmark name */
__( 'Unable to append markup to %s since the bookmark no longer exists.', 'optimization-detective' )
sprintf(
/* translators: %s is the bookmark name */
__( 'Unable to append markup to %s since the bookmark no longer exists.', 'optimization-detective' ),
$bookmark
)
);
} else {
$start = $this->bookmarks[ $bookmark ]->start;
Expand Down
15 changes: 9 additions & 6 deletions plugins/optimization-detective/optimization.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,16 @@ function od_optimize_template_output_buffer( string $buffer ): string {
while ( $processor->next_open_tag() ) {
$did_visit = false;
$processor->set_bookmark( $current_tag_bookmark );
foreach ( $visitors as $visitor ) {
$did_visit = $visitor( $tag_visitor_context ) || $did_visit;

// Since the visitor may have traversed HTML tags, we need to make sure we 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.
$processor->seek( $current_tag_bookmark );
foreach ( $visitors as $visitor ) {
$seek_count = $processor->get_seek_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() ) {
$processor->seek( $current_tag_bookmark );
}
}
$processor->release_bookmark( $current_tag_bookmark );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ public function test_html_tag_processor_wrapper_methods(): void {
/**
* Test bookmarking and seeking.
*
* @covers ::get_seek_count
* @covers ::set_bookmark
* @covers ::seek
* @covers ::release_bookmark
Expand All @@ -519,6 +520,7 @@ public function test_bookmarking_and_seeking(): void {
);

$actual_figure_contents = array();
$this->assertSame( 0, $processor->get_seek_count() );

$bookmarks = array();
while ( $processor->next_open_tag() ) {
Expand Down Expand Up @@ -580,6 +582,7 @@ 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 );

Expand Down
74 changes: 73 additions & 1 deletion plugins/optimization-detective/tests/test-optimization.php
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ public function data_provider_test_od_optimize_template_output_buffer(): array {
'video' => array(
'set_up' => function (): void {
$slug = od_get_url_metrics_slug( od_get_normalized_query_vars() );
$sample_size = od_get_url_metrics_breakpoint_sample_size();
foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $viewport_width ) {
OD_URL_Metrics_Post_Type::store_url_metric(
$slug,
Expand Down Expand Up @@ -309,6 +308,79 @@ public function data_provider_test_od_optimize_template_output_buffer(): array {
',
),

'many_images' => array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😎

'set_up' => function (): void {
$slug = od_get_url_metrics_slug( od_get_normalized_query_vars() );
foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $viewport_width ) {

$elements = array();
for ( $i = 1; $i < WP_HTML_Tag_Processor::MAX_SEEK_OPS; $i++ ) {
$elements[] = array(
'xpath' => sprintf( '/*[1][self::HTML]/*[2][self::BODY]/*[%d][self::IMG]', $i ),
'isLCP' => false,
);
}

OD_URL_Metrics_Post_Type::store_url_metric(
$slug,
$this->get_validated_url_metric(
$viewport_width,
$elements
)
);
}
},
'buffer' => '
<html lang="en">
<head>
<meta charset="utf-8">
<title>...</title>
</head>
<body>
' .
join(
"\n",
call_user_func(
static function () {
$tags = array();
for ( $i = 1; $i < WP_HTML_Tag_Processor::MAX_SEEK_OPS + 1; $i++ ) {
$tags[] = sprintf( '<img src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" loading="lazy">' );
}
return $tags;
}
)
) .
'
</body>
</html>
',
'expected' => '
<html lang="en">
<head>
<meta charset="utf-8">
<title>...</title>
</head>
<body>
' .
join(
"\n",
call_user_func(
static function () {
$tags = array();
for ( $i = 1; $i < WP_HTML_Tag_Processor::MAX_SEEK_OPS + 1; $i++ ) {
$tags[] = sprintf( '<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[%d][self::IMG]" src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" loading="lazy">', $i );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spiffy

}
return $tags;
}
)
) .
'
<script type="module">/* import detect ... */</script>
</body>
</html>
',
),

'rss-response' => array(
'set_up' => static function (): void {
ini_set( 'default_mimetype', 'application/rss+xml' ); // phpcs:ignore WordPress.PHP.IniSet.Risky
Expand Down
Loading