From 0ad8338805ac80d6a25608e69719e728c0471cbe Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 15 Aug 2024 17:21:50 -0700 Subject: [PATCH 1/9] Reuse logic with Optimization Detective and account for removing erroneous auto --- plugins/auto-sizes/hooks.php | 46 +++++++++++++------ plugins/auto-sizes/optimization-detective.php | 30 +----------- plugins/auto-sizes/tests/test-auto-sizes.php | 4 ++ .../tests/test-optimization-detective.php | 10 ++++ 4 files changed, 48 insertions(+), 42 deletions(-) diff --git a/plugins/auto-sizes/hooks.php b/plugins/auto-sizes/hooks.php index 66d4d1efd7..fde8b72db6 100644 --- a/plugins/auto-sizes/hooks.php +++ b/plugins/auto-sizes/hooks.php @@ -64,28 +64,46 @@ function auto_sizes_update_content_img_tag( $html ): string { return $html; } - // Bail early if the image is not lazy-loaded. - $value = $processor->get_attribute( 'loading' ); - if ( ! is_string( $value ) || 'lazy' !== strtolower( trim( $value, " \t\f\r\n" ) ) ) { - return $html; + auto_sizes_apply_tag_parser_updates( $processor ); + return $processor->get_updated_html(); +} +add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' ); + +/** + * Applies changes to current IMG tag to add or remove 'auto' from the sizes attribute when needed. + * + * @since n.e.x.t + * @access private + * + * @param WP_HTML_Tag_Processor $processor HTML processor currently at an IMG tag. + * @return bool Whether changes were made. + */ +function auto_sizes_apply_tag_parser_updates( WP_HTML_Tag_Processor $processor ): bool { + if ( $processor->get_tag() !== 'IMG' ) { + return false; } $sizes = $processor->get_attribute( 'sizes' ); - - // Bail early if the image is not responsive. if ( ! is_string( $sizes ) ) { - return $html; + return false; } - // Don't add 'auto' to the sizes attribute if it already exists. - if ( auto_sizes_attribute_includes_valid_auto( $sizes ) ) { - return $html; - } + $value = $processor->get_attribute( 'loading' ); + $is_lazy = ( is_string( $value ) && 'lazy' === strtolower( trim( $value, " \t\f\r\n" ) ) ); - $processor->set_attribute( 'sizes', "auto, $sizes" ); - return $processor->get_updated_html(); + $has_auto = auto_sizes_attribute_includes_valid_auto( $sizes ); + + if ( $is_lazy && ! $has_auto ) { + $processor->set_attribute( 'sizes', "auto, $sizes" ); + } elseif ( ! $is_lazy && $has_auto ) { + // Remove auto from the beginning of the list. + $processor->set_attribute( + 'sizes', + (string) preg_replace( '/^[ \t\f\r\n]*auto[ \t\f\r\n]*(,[ \t\f\r\n]*)?/i', '', $sizes ) + ); + } + return true; } -add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' ); /** * Checks whether the given 'sizes' attribute includes the 'auto' keyword as the first item in the list. diff --git a/plugins/auto-sizes/optimization-detective.php b/plugins/auto-sizes/optimization-detective.php index 3b10027dac..c29f15680d 100644 --- a/plugins/auto-sizes/optimization-detective.php +++ b/plugins/auto-sizes/optimization-detective.php @@ -19,35 +19,9 @@ * @return false Whether the tag should be tracked in URL metrics. */ function auto_sizes_visit_tag( OD_Tag_Visitor_Context $context ): bool { - if ( 'IMG' !== $context->processor->get_tag() ) { - return false; + if ( 'IMG' === $context->processor->get_tag() ) { + auto_sizes_apply_tag_parser_updates( $context->processor ); } - - $sizes = $context->processor->get_attribute( 'sizes' ); - if ( ! is_string( $sizes ) ) { - return false; - } - - $sizes = preg_split( '/\s*,\s*/', $sizes ); - if ( ! is_array( $sizes ) ) { - return false; - } - - $is_lazy_loaded = ( 'lazy' === $context->processor->get_attribute( 'loading' ) ); - $has_auto_sizes = in_array( 'auto', $sizes, true ); - - $changed = false; - if ( $is_lazy_loaded && ! $has_auto_sizes ) { - array_unshift( $sizes, 'auto' ); - $changed = true; - } elseif ( ! $is_lazy_loaded && $has_auto_sizes ) { - $sizes = array_diff( $sizes, array( 'auto' ) ); - $changed = true; - } - if ( $changed ) { - $context->processor->set_attribute( 'sizes', join( ', ', $sizes ) ); - } - return false; // Since this tag visitor does not require this tag to be included in the URL Metrics. } diff --git a/plugins/auto-sizes/tests/test-auto-sizes.php b/plugins/auto-sizes/tests/test-auto-sizes.php index 13898a4ae9..10d4c20b2d 100644 --- a/plugins/auto-sizes/tests/test-auto-sizes.php +++ b/plugins/auto-sizes/tests/test-auto-sizes.php @@ -262,6 +262,10 @@ public function data_provider_to_test_auto_sizes_update_content_img_tag(): array 'input' => '', 'expected' => '', ), + 'not_expected_when_loading_eager_and_initially_auto_sizes' => array( + 'input' => '', + 'expected' => '', + ), ); } diff --git a/plugins/auto-sizes/tests/test-optimization-detective.php b/plugins/auto-sizes/tests/test-optimization-detective.php index 0dd936c812..6d19c80d31 100644 --- a/plugins/auto-sizes/tests/test-optimization-detective.php +++ b/plugins/auto-sizes/tests/test-optimization-detective.php @@ -93,6 +93,16 @@ public function data_provider_test_od_optimize_template_output_buffer(): array { 'buffer' => 'Foo', 'expected' => 'Foo', ), + + 'wrongly_auto_sized_responsive_img_with_only_auto' => array( + 'element_metrics' => array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => false, + 'intersectionRatio' => 1, + ), + 'buffer' => 'Foo', + 'expected' => 'Foo', + ), ); } From 65bc8f0112b1a86c70e42b9de95ebcaf14ff31bb Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 15 Aug 2024 17:28:35 -0700 Subject: [PATCH 2/9] Account for strtok() consuming initial commas --- plugins/auto-sizes/hooks.php | 4 ++-- plugins/auto-sizes/tests/test-auto-sizes.php | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/auto-sizes/hooks.php b/plugins/auto-sizes/hooks.php index fde8b72db6..c35731d163 100644 --- a/plugins/auto-sizes/hooks.php +++ b/plugins/auto-sizes/hooks.php @@ -116,8 +116,8 @@ function auto_sizes_apply_tag_parser_updates( WP_HTML_Tag_Processor $processor ) * @return bool True if the 'auto' keyword is present, false otherwise. */ function auto_sizes_attribute_includes_valid_auto( string $sizes_attr ): bool { - $token = strtok( strtolower( $sizes_attr ), ',' ); - return false !== $token && 'auto' === trim( $token, " \t\f\r\n" ); + list( $first_size ) = explode( ',', $sizes_attr, 2 ); + return 'auto' === strtolower( trim( $first_size, " \t\f\r\n" ) ); } /** diff --git a/plugins/auto-sizes/tests/test-auto-sizes.php b/plugins/auto-sizes/tests/test-auto-sizes.php index 10d4c20b2d..6de1d01f88 100644 --- a/plugins/auto-sizes/tests/test-auto-sizes.php +++ b/plugins/auto-sizes/tests/test-auto-sizes.php @@ -266,6 +266,10 @@ public function data_provider_to_test_auto_sizes_update_content_img_tag(): array 'input' => '', 'expected' => '', ), + 'expected_when_auto_size_preceded_by_extra_commas' => array( + 'input' => '', + 'expected' => '', + ), ); } From 5dc1eb5c9589c8952de482e0068af047a784ab58 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Sep 2024 16:29:25 -0700 Subject: [PATCH 3/9] Fix handling crossorigin attribute with no value --- ...lass-image-prioritizer-img-tag-visitor.php | 2 +- ...erent-lcp-elements-for-all-breakpoints.php | 77 +++++++++++++++++++ .../class-od-link-collection.php | 2 +- 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 287e955f31..f3c6b8ee3b 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -111,7 +111,7 @@ static function ( string $value ): bool { ); $crossorigin = $processor->get_attribute( 'crossorigin' ); - if ( is_string( $crossorigin ) ) { + if ( null !== $crossorigin ) { $link_attributes['crossorigin'] = 'use-credentials' === $crossorigin ? 'use-credentials' : 'anonymous'; } diff --git a/plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php b/plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php new file mode 100644 index 0000000000..aab449d398 --- /dev/null +++ b/plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php @@ -0,0 +1,77 @@ + static function ( Test_Image_Prioritizer_Helper $test_case ): void { + $breakpoint_max_widths = array( 480, 600, 782 ); + + add_filter( + 'od_breakpoint_max_widths', + static function () use ( $breakpoint_max_widths ) { + return $breakpoint_max_widths; + } + ); + + foreach ( array_merge( $breakpoint_max_widths, array( 1600 ) ) as $i => $viewport_width ) { + $elements = array( + array( + 'isLCP' => false, + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + ), + array( + 'isLCP' => false, + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[2][self::IMG]', + ), + array( + 'isLCP' => false, + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[3][self::IMG]', + ), + array( + 'isLCP' => false, + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[4][self::IMG]', + ), + ); + $elements[ $i ]['isLCP'] = true; + OD_URL_Metrics_Post_Type::store_url_metric( + od_get_url_metrics_slug( od_get_normalized_query_vars() ), + $test_case->get_sample_url_metric( + array( + 'viewport_width' => $viewport_width, + 'elements' => $elements, + ) + ) + ); + } + }, + 'buffer' => ' + + + + ... + + + Mobile Logo + Phablet Logo + Tablet Logo + Desktop Logo + + + ', + 'expected' => ' + + + + ... + + + + + + + Mobile Logo + Phablet Logo + Tablet Logo + Desktop Logo + + + + ', +); diff --git a/plugins/optimization-detective/class-od-link-collection.php b/plugins/optimization-detective/class-od-link-collection.php index 28084c9bd5..b1fbfacb6c 100644 --- a/plugins/optimization-detective/class-od-link-collection.php +++ b/plugins/optimization-detective/class-od-link-collection.php @@ -25,7 +25,7 @@ * href?: non-empty-string, * imagesrcset?: non-empty-string, * imagesizes?: non-empty-string, - * crossorigin?: ''|'anonymous'|'use-credentials', + * crossorigin?: 'anonymous'|'use-credentials', * fetchpriority?: 'high'|'low'|'auto', * as?: 'audio'|'document'|'embed'|'fetch'|'font'|'image'|'object'|'script'|'style'|'track'|'video'|'worker', * media?: non-empty-string, From ae5d3ce06489dc4481319b21d9e4b95c5daafd5d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Sep 2024 17:10:45 -0700 Subject: [PATCH 4/9] Account for enumerated attribute value normalization --- ...lass-image-prioritizer-img-tag-visitor.php | 22 +++++++++++++++---- ...wport-with-fully-populated-sample-data.php | 4 ++-- ...erent-lcp-elements-for-all-breakpoints.php | 10 ++++----- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index f3c6b8ee3b..1276ba0a2b 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -40,6 +40,20 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { $xpath = $processor->get_xpath(); + /** + * Gets attribute value. + * + * @param string $attribute_name Attribute name. + * @return string|true|null Normalized attribute value. + */ + $get_attribute_value = static function ( string $attribute_name ) use ( $processor ) { + $value = $processor->get_attribute( $attribute_name ); + if ( is_string( $value ) ) { + $value = strtolower( trim( $value, " \t\f\r\n" ) ); + } + return $value; + }; + /* * When the same LCP element is common/shared among all viewport groups, make sure that the element has * fetchpriority=high, even though it won't really be needed because a preload link with fetchpriority=high @@ -47,13 +61,13 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { */ $common_lcp_element = $context->url_metrics_group_collection->get_common_lcp_element(); if ( ! is_null( $common_lcp_element ) && $xpath === $common_lcp_element['xpath'] ) { - if ( 'high' === $processor->get_attribute( 'fetchpriority' ) ) { + if ( 'high' === $get_attribute_value( 'fetchpriority' ) ) { $processor->set_meta_attribute( 'fetchpriority-already-added', true ); } else { $processor->set_attribute( 'fetchpriority', 'high' ); } } elseif ( - is_string( $processor->get_attribute( 'fetchpriority' ) ) + is_string( $get_attribute_value( 'fetchpriority' ) ) && // Temporary condition in case someone updates Image Prioritizer without also updating Optimization Detective. method_exists( $context->url_metrics_group_collection, 'is_any_group_populated' ) @@ -81,7 +95,7 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { } else { // Otherwise, make sure visible elements omit the loading attribute, and hidden elements include loading=lazy. $is_visible = $element_max_intersection_ratio > 0.0; - $loading = (string) $processor->get_attribute( 'loading' ); + $loading = $get_attribute_value( 'loading' ); if ( $is_visible && 'lazy' === $loading ) { $processor->remove_attribute( 'loading' ); } elseif ( ! $is_visible && 'lazy' !== $loading ) { @@ -110,7 +124,7 @@ static function ( string $value ): bool { ) ); - $crossorigin = $processor->get_attribute( 'crossorigin' ); + $crossorigin = $get_attribute_value( 'crossorigin' ); if ( null !== $crossorigin ) { $link_attributes['crossorigin'] = 'use-credentials' === $crossorigin ? 'use-credentials' : 'anonymous'; } diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php index 356ddfb0d6..31f6a9eba6 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php @@ -55,7 +55,7 @@

Now the following image is definitely outside the initial viewport.

Baz Qux - Quux + Quux ', @@ -73,7 +73,7 @@

Now the following image is definitely outside the initial viewport.

Baz Qux - Quux + Quux ', diff --git a/plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php b/plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php index aab449d398..c51a47dcac 100644 --- a/plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php +++ b/plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php @@ -48,8 +48,8 @@ static function () use ( $breakpoint_max_widths ) { ... - Mobile Logo - Phablet Logo + Mobile Logo + Phablet Logo Tablet Logo Desktop Logo @@ -60,14 +60,14 @@ static function () use ( $breakpoint_max_widths ) { ... - + - Mobile Logo - Phablet Logo + Mobile Logo + Phablet Logo Tablet Logo Desktop Logo From f7a8700bb6690b880a4028c17fc3790dbb642cf4 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Sep 2024 17:24:10 -0700 Subject: [PATCH 5/9] Move sizes=auto logic to Image Prioritizer plugin --- plugins/auto-sizes/auto-sizes.php | 2 - plugins/auto-sizes/hooks.php | 23 +-- plugins/auto-sizes/optimization-detective.php | 40 ------ .../tests/test-optimization-detective.php | 133 ------------------ ...lass-image-prioritizer-img-tag-visitor.php | 33 +++++ .../image-prioritizer/tests/test-helper.php | 96 +++++++++++++ 6 files changed, 134 insertions(+), 193 deletions(-) delete mode 100644 plugins/auto-sizes/optimization-detective.php delete mode 100644 plugins/auto-sizes/tests/test-optimization-detective.php diff --git a/plugins/auto-sizes/auto-sizes.php b/plugins/auto-sizes/auto-sizes.php index 6abd2300c1..d44f8755a9 100644 --- a/plugins/auto-sizes/auto-sizes.php +++ b/plugins/auto-sizes/auto-sizes.php @@ -28,5 +28,3 @@ define( 'IMAGE_AUTO_SIZES_VERSION', '1.2.0' ); require_once __DIR__ . '/hooks.php'; - -require_once __DIR__ . '/optimization-detective.php'; diff --git a/plugins/auto-sizes/hooks.php b/plugins/auto-sizes/hooks.php index c35731d163..56f15fcdba 100644 --- a/plugins/auto-sizes/hooks.php +++ b/plugins/auto-sizes/hooks.php @@ -64,28 +64,13 @@ function auto_sizes_update_content_img_tag( $html ): string { return $html; } - auto_sizes_apply_tag_parser_updates( $processor ); - return $processor->get_updated_html(); -} -add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' ); - -/** - * Applies changes to current IMG tag to add or remove 'auto' from the sizes attribute when needed. - * - * @since n.e.x.t - * @access private - * - * @param WP_HTML_Tag_Processor $processor HTML processor currently at an IMG tag. - * @return bool Whether changes were made. - */ -function auto_sizes_apply_tag_parser_updates( WP_HTML_Tag_Processor $processor ): bool { if ( $processor->get_tag() !== 'IMG' ) { - return false; + return $html; } $sizes = $processor->get_attribute( 'sizes' ); if ( ! is_string( $sizes ) ) { - return false; + return $html; } $value = $processor->get_attribute( 'loading' ); @@ -102,8 +87,10 @@ function auto_sizes_apply_tag_parser_updates( WP_HTML_Tag_Processor $processor ) (string) preg_replace( '/^[ \t\f\r\n]*auto[ \t\f\r\n]*(,[ \t\f\r\n]*)?/i', '', $sizes ) ); } - return true; + + return $processor->get_updated_html(); } +add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' ); /** * Checks whether the given 'sizes' attribute includes the 'auto' keyword as the first item in the list. diff --git a/plugins/auto-sizes/optimization-detective.php b/plugins/auto-sizes/optimization-detective.php deleted file mode 100644 index c29f15680d..0000000000 --- a/plugins/auto-sizes/optimization-detective.php +++ /dev/null @@ -1,40 +0,0 @@ -processor->get_tag() ) { - auto_sizes_apply_tag_parser_updates( $context->processor ); - } - return false; // Since this tag visitor does not require this tag to be included in the URL Metrics. -} - -/** - * Registers the tag visitor for image tags. - * - * @since 1.1.0 - * - * @param OD_Tag_Visitor_Registry $registry Tag visitor registry. - */ -function auto_sizes_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void { - $registry->register( 'auto-sizes', 'auto_sizes_visit_tag' ); -} - -// Important: The Image Prioritizer's IMG tag visitor is registered at priority 10, so priority 100 ensures that the loading attribute has been correctly set by the time the Auto Sizes visitor runs. -add_action( 'od_register_tag_visitors', 'auto_sizes_register_tag_visitors', 100 ); diff --git a/plugins/auto-sizes/tests/test-optimization-detective.php b/plugins/auto-sizes/tests/test-optimization-detective.php deleted file mode 100644 index 6d19c80d31..0000000000 --- a/plugins/auto-sizes/tests/test-optimization-detective.php +++ /dev/null @@ -1,133 +0,0 @@ -markTestSkipped( 'Optimization Detective is not active.' ); - } - } - - /** - * Tests auto_sizes_register_tag_visitors(). - * - * @covers ::auto_sizes_register_tag_visitors - */ - public function test_auto_sizes_register_tag_visitors(): void { - if ( ! class_exists( OD_Tag_Visitor_Registry::class ) ) { - $this->markTestSkipped( 'Optimization Detective is not active.' ); - } - $registry = new OD_Tag_Visitor_Registry(); - auto_sizes_register_tag_visitors( $registry ); - $this->assertTrue( $registry->is_registered( 'auto-sizes' ) ); - $this->assertEquals( 'auto_sizes_visit_tag', $registry->get_registered( 'auto-sizes' ) ); - } - - /** - * Data provider. - * - * @return array Data. - */ - public function data_provider_test_od_optimize_template_output_buffer(): array { - return array( - // Note: The Image Prioritizer plugin removes the loading attribute, and so then Auto Sizes does not then add sizes=auto. - 'wrongly_lazy_responsive_img' => array( - 'element_metrics' => array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', - 'isLCP' => false, - 'intersectionRatio' => 1, - ), - 'buffer' => 'Foo', - 'expected' => 'Foo', - ), - - 'non_responsive_image' => array( - 'element_metrics' => array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', - 'isLCP' => false, - 'intersectionRatio' => 0, - ), - 'buffer' => 'Quux', - 'expected' => 'Quux', - ), - - 'auto_sizes_added' => array( - 'element_metrics' => array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', - 'isLCP' => false, - 'intersectionRatio' => 0, - ), - 'buffer' => 'Foo', - 'expected' => 'Foo', - ), - - 'auto_sizes_already_added' => array( - 'element_metrics' => array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', - 'isLCP' => false, - 'intersectionRatio' => 0, - ), - 'buffer' => 'Foo', - 'expected' => 'Foo', - ), - - // If Auto Sizes added the sizes=auto attribute but Image Prioritizer ended up removing it due to the image not being lazy-loaded, remove sizes=auto again. - 'wrongly_auto_sized_responsive_img' => array( - 'element_metrics' => array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', - 'isLCP' => false, - 'intersectionRatio' => 1, - ), - 'buffer' => 'Foo', - 'expected' => 'Foo', - ), - - 'wrongly_auto_sized_responsive_img_with_only_auto' => array( - 'element_metrics' => array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', - 'isLCP' => false, - 'intersectionRatio' => 1, - ), - 'buffer' => 'Foo', - 'expected' => 'Foo', - ), - ); - } - - /** - * Test auto_sizes_visit_tag(). - * - * @covers ::auto_sizes_visit_tag - * - * @dataProvider data_provider_test_od_optimize_template_output_buffer - * @phpstan-param array{ xpath: string, isLCP: bool, intersectionRatio: int } $element_metrics - */ - public function test_od_optimize_template_output_buffer( array $element_metrics, string $buffer, string $expected ): void { - $this->populate_url_metrics( array( $element_metrics ) ); - - $html_start_doc = '...'; - $html_end_doc = ''; - - $buffer = od_optimize_template_output_buffer( $html_start_doc . $buffer . $html_end_doc ); - $buffer = preg_replace( '#.+?]*>#s', '', $buffer ); - $buffer = preg_replace( '#.*$#s', '', $buffer ); - - $this->assertEquals( - $this->remove_initial_tabs( $expected ), - $this->remove_initial_tabs( $buffer ), - "Buffer snapshot:\n$buffer" - ); - } -} diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 1276ba0a2b..224ad18fda 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -104,6 +104,23 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { } // TODO: If an image is visible in one breakpoint but not another, add loading=lazy AND add a regular-priority preload link with media queries (unless LCP in which case it should already have a fetchpriority=high link) so that the image won't be eagerly-loaded for viewports on which it is not shown. + // Ensure that sizes=auto is set properly. + $sizes = $processor->get_attribute( 'sizes' ); + if ( is_string( $sizes ) ) { + $is_lazy = 'lazy' === $get_attribute_value( 'loading' ); + $has_auto = $this->sizes_attribute_includes_valid_auto( $sizes ); + + if ( $is_lazy && ! $has_auto ) { + $processor->set_attribute( 'sizes', "auto, $sizes" ); + } elseif ( ! $is_lazy && $has_auto ) { + // Remove auto from the beginning of the list. + $processor->set_attribute( + 'sizes', + (string) preg_replace( '/^[ \t\f\r\n]*auto[ \t\f\r\n]*(,[ \t\f\r\n]*)?/i', '', $sizes ) + ); + } + } + // If this element is the LCP (for a breakpoint group), add a preload link for it. foreach ( $context->url_metrics_group_collection->get_groups_by_lcp_element( $xpath ) as $group ) { $link_attributes = array_merge( @@ -140,4 +157,20 @@ static function ( string $value ): bool { return true; } + + /** + * Checks whether the given 'sizes' attribute includes the 'auto' keyword as the first item in the list. + * + * Per the HTML spec, if present it must be the first entry. + * + * @since n.e.x.t + * @see auto_sizes_attribute_includes_valid_auto() + * + * @param string $sizes_attr The 'sizes' attribute value. + * @return bool True if the 'auto' keyword is present, false otherwise. + */ + private function sizes_attribute_includes_valid_auto( string $sizes_attr ): bool { + list( $first_size ) = explode( ',', $sizes_attr, 2 ); + return 'auto' === strtolower( trim( $first_size, " \t\f\r\n" ) ); + } } diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index 716e057bc6..708c062a53 100644 --- a/plugins/image-prioritizer/tests/test-helper.php +++ b/plugins/image-prioritizer/tests/test-helper.php @@ -63,4 +63,100 @@ public function test_image_prioritizer_register_tag_visitors( Closure $set_up, s "Buffer snapshot:\n$buffer" ); } + + /** + * Data provider. + * + * @return array Data. + */ + public function data_provider_test_auto_sizes(): array { + return array( + // Note: The Image Prioritizer plugin removes the loading attribute, and so then Auto Sizes does not then add sizes=auto. + 'wrongly_lazy_responsive_img' => array( + 'element_metrics' => array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => false, + 'intersectionRatio' => 1, + ), + 'buffer' => 'Foo', + 'expected' => 'Foo', + ), + + 'non_responsive_image' => array( + 'element_metrics' => array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => false, + 'intersectionRatio' => 0, + ), + 'buffer' => 'Quux', + 'expected' => 'Quux', + ), + + 'auto_sizes_added' => array( + 'element_metrics' => array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => false, + 'intersectionRatio' => 0, + ), + 'buffer' => 'Foo', + 'expected' => 'Foo', + ), + + 'auto_sizes_already_added' => array( + 'element_metrics' => array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => false, + 'intersectionRatio' => 0, + ), + 'buffer' => 'Foo', + 'expected' => 'Foo', + ), + + // If Auto Sizes added the sizes=auto attribute but Image Prioritizer ended up removing it due to the image not being lazy-loaded, remove sizes=auto again. + 'wrongly_auto_sized_responsive_img' => array( + 'element_metrics' => array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => false, + 'intersectionRatio' => 1, + ), + 'buffer' => 'Foo', + 'expected' => 'Foo', + ), + + 'wrongly_auto_sized_responsive_img_with_only_auto' => array( + 'element_metrics' => array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => false, + 'intersectionRatio' => 1, + ), + 'buffer' => 'Foo', + 'expected' => 'Foo', + ), + ); + } + + /** + * Test auto sizes. + * + * @covers Image_Prioritizer_Img_Tag_Visitor::__invoke + * + * @dataProvider data_provider_test_auto_sizes + * @phpstan-param array{ xpath: string, isLCP: bool, intersectionRatio: int } $element_metrics + */ + public function test_auto_sizes( array $element_metrics, string $buffer, string $expected ): void { + $this->populate_url_metrics( array( $element_metrics ) ); + + $html_start_doc = '...'; + $html_end_doc = ''; + + $buffer = od_optimize_template_output_buffer( $html_start_doc . $buffer . $html_end_doc ); + $buffer = preg_replace( '#.+?]*>#s', '', $buffer ); + $buffer = preg_replace( '#.*$#s', '', $buffer ); + + $this->assertEquals( + $this->remove_initial_tabs( $expected ), + $this->remove_initial_tabs( $buffer ), + "Buffer snapshot:\n$buffer" + ); + } } From e01fafe043eefa4fe13e72c4cdd22595b96f1260 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 6 Sep 2024 11:28:22 -0700 Subject: [PATCH 6/9] Revert removal of sizes=auto if not loading=lazy in non-OD code --- plugins/auto-sizes/hooks.php | 23 ++++++++------------ plugins/auto-sizes/tests/test-auto-sizes.php | 4 ---- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/plugins/auto-sizes/hooks.php b/plugins/auto-sizes/hooks.php index 56f15fcdba..4d2479b745 100644 --- a/plugins/auto-sizes/hooks.php +++ b/plugins/auto-sizes/hooks.php @@ -64,30 +64,25 @@ function auto_sizes_update_content_img_tag( $html ): string { return $html; } - if ( $processor->get_tag() !== 'IMG' ) { + // Bail early if the image is not lazy-loaded. + $value = $processor->get_attribute( 'loading' ); + if ( ! is_string( $value ) || 'lazy' !== strtolower( trim( $value, " \t\f\r\n" ) ) ) { return $html; } $sizes = $processor->get_attribute( 'sizes' ); + + // Bail early if the image is not responsive. if ( ! is_string( $sizes ) ) { return $html; } - $value = $processor->get_attribute( 'loading' ); - $is_lazy = ( is_string( $value ) && 'lazy' === strtolower( trim( $value, " \t\f\r\n" ) ) ); - - $has_auto = auto_sizes_attribute_includes_valid_auto( $sizes ); - - if ( $is_lazy && ! $has_auto ) { - $processor->set_attribute( 'sizes', "auto, $sizes" ); - } elseif ( ! $is_lazy && $has_auto ) { - // Remove auto from the beginning of the list. - $processor->set_attribute( - 'sizes', - (string) preg_replace( '/^[ \t\f\r\n]*auto[ \t\f\r\n]*(,[ \t\f\r\n]*)?/i', '', $sizes ) - ); + // Don't add 'auto' to the sizes attribute if it already exists. + if ( auto_sizes_attribute_includes_valid_auto( $sizes ) ) { + return $html; } + $processor->set_attribute( 'sizes', "auto, $sizes" ); return $processor->get_updated_html(); } add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' ); diff --git a/plugins/auto-sizes/tests/test-auto-sizes.php b/plugins/auto-sizes/tests/test-auto-sizes.php index 6de1d01f88..8a25f2b059 100644 --- a/plugins/auto-sizes/tests/test-auto-sizes.php +++ b/plugins/auto-sizes/tests/test-auto-sizes.php @@ -262,10 +262,6 @@ public function data_provider_to_test_auto_sizes_update_content_img_tag(): array 'input' => '', 'expected' => '', ), - 'not_expected_when_loading_eager_and_initially_auto_sizes' => array( - 'input' => '', - 'expected' => '', - ), 'expected_when_auto_size_preceded_by_extra_commas' => array( 'input' => '', 'expected' => '', From 9d86f62ffa623a90755e374f311f76acbd9ba91b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 6 Sep 2024 11:33:28 -0700 Subject: [PATCH 7/9] Remove needless use of get_attribute_value --- .../class-image-prioritizer-img-tag-visitor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 224ad18fda..77962f004b 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -67,7 +67,7 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { $processor->set_attribute( 'fetchpriority', 'high' ); } } elseif ( - is_string( $get_attribute_value( 'fetchpriority' ) ) + is_string( $processor->get_attribute( 'fetchpriority' ) ) && // Temporary condition in case someone updates Image Prioritizer without also updating Optimization Detective. method_exists( $context->url_metrics_group_collection, 'is_any_group_populated' ) From 775ab186dad9e6854d15cfc4d6f0aed295abf18c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 6 Sep 2024 13:10:19 -0700 Subject: [PATCH 8/9] Reuse other sizes_attribute_includes_valid_auto implementations when available Co-authored-by: felixarntz --- .../class-image-prioritizer-img-tag-visitor.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 77962f004b..14ba5c8ff2 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -170,7 +170,12 @@ static function ( string $value ): bool { * @return bool True if the 'auto' keyword is present, false otherwise. */ private function sizes_attribute_includes_valid_auto( string $sizes_attr ): bool { - list( $first_size ) = explode( ',', $sizes_attr, 2 ); - return 'auto' === strtolower( trim( $first_size, " \t\f\r\n" ) ); + if ( function_exists( 'wp_sizes_attribute_includes_valid_auto' ) ) { + return wp_sizes_attribute_includes_valid_auto( $sizes_attr ); + } elseif ( function_exists( 'auto_sizes_attribute_includes_valid_auto' ) ) { + return auto_sizes_attribute_includes_valid_auto( $sizes_attr ); + } else { + return 'auto' === $sizes_attr || str_starts_with( $sizes_attr, 'auto,' ); + } } } From b0a9c3fc8e8db8129c7724c7ff6cf690009fe861 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 9 Sep 2024 10:43:53 -0700 Subject: [PATCH 9/9] Remove see phpdoc tag Co-authored-by: Mukesh Panchal --- .../class-image-prioritizer-img-tag-visitor.php | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php index 14ba5c8ff2..43151939b3 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -164,7 +164,6 @@ static function ( string $value ): bool { * Per the HTML spec, if present it must be the first entry. * * @since n.e.x.t - * @see auto_sizes_attribute_includes_valid_auto() * * @param string $sizes_attr The 'sizes' attribute value. * @return bool True if the 'auto' keyword is present, false otherwise.