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 ba14edf57e..3e84bec1f0 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php @@ -21,6 +21,14 @@ */ final class Image_Prioritizer_Img_Tag_Visitor extends Image_Prioritizer_Tag_Visitor { + /** + * List of PICTURE XPaths to skip processing of child IMG tags. + * + * @since n.e.x.t + * @var string[] + */ + private $picture_ancestor_xpaths_to_skip = array(); + /** * Visits a tag. * @@ -35,7 +43,11 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { $tag = $processor->get_tag(); if ( 'PICTURE' === $tag ) { - return $this->process_picture( $processor, $context ); + $picture_xpath = $processor->get_xpath(); + if ( false === $this->process_picture( $processor, $context ) ) { + $this->picture_ancestor_xpaths_to_skip[] = $picture_xpath; + } + return false; // Because the IMG child is what gets tracked in URL Metrics. } elseif ( 'IMG' === $tag ) { return $this->process_img( $processor, $context ); } @@ -43,6 +55,38 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { return false; } + /** + * Determines whether the current IMG is valid for tracking in URL Metrics. + * + * An IMG must have a src attribute which is not a data: URL. And if it has a srcset attribute, it also must not be + * a data: URL. + * + * @since n.e.x.t + * + * @param OD_HTML_Tag_Processor $processor Tag Processor. + * @return bool Whether valid for tracking in URL Metrics. + */ + private function is_img_with_valid_src_and_srcset( OD_HTML_Tag_Processor $processor ): bool { + $src = $this->get_attribute_value( $processor, 'src' ); + $has_src = ( is_string( $src ) && '' !== $src ); + if ( ! $has_src ) { + return false; + } + + $srcset = $this->get_attribute_value( $processor, 'srcset' ); + $has_srcset = ( is_string( $srcset ) && '' !== $srcset ); + + // Abort data: URLs (which may very be JS-based lazy-loading). + if ( $this->is_data_url( $src ) ) { + return false; + } + if ( $has_srcset && $this->is_data_url( $srcset ) ) { + return false; + } + + return true; + } + /** * Process an IMG element. * @@ -53,13 +97,21 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { * @return bool Whether the tag should be tracked in URL Metrics. */ private function process_img( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_Context $context ): bool { - $src = $this->get_valid_src( $processor ); - if ( null === $src ) { + if ( ! $this->is_img_with_valid_src_and_srcset( $processor ) ) { return false; } $xpath = $processor->get_xpath(); + // If the PICTURE's processing was aborted, then abort processing its child IMG as well. + if ( 'PICTURE' === $this->get_parent_tag_name( $context ) ) { + foreach ( $this->picture_ancestor_xpaths_to_skip as $picture_xpath ) { + if ( str_starts_with( $xpath, $picture_xpath ) ) { + return false; + } + } + } + $current_fetchpriority = $this->get_attribute_value( $processor, 'fetchpriority' ); $is_lazy_loaded = 'lazy' === $this->get_attribute_value( $processor, 'loading' ); $updated_fetchpriority = null; @@ -187,7 +239,7 @@ private function process_img( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_C * * @param OD_HTML_Tag_Processor $processor HTML tag processor. * @param OD_Tag_Visitor_Context $context Tag visitor context. - * @return bool Whether the tag should be tracked in URL Metrics. + * @return bool Whether the PICTURE was processed. */ private function process_picture( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_Context $context ): bool { /** @@ -218,8 +270,13 @@ private function process_picture( OD_HTML_Tag_Processor $processor, OD_Tag_Visit } // Abort processing if a SOURCE lacks the required srcset attribute. - $srcset = $this->get_valid_src( $processor, 'srcset' ); - if ( null === $srcset ) { + $srcset = $this->get_attribute_value( $processor, 'srcset' ); + if ( ! is_string( $srcset ) ) { + return false; + } + + // Abort if the srcset is a data: URL since there is nothing to optimize. + if ( $this->is_data_url( $srcset ) ) { return false; } @@ -242,8 +299,8 @@ private function process_picture( OD_HTML_Tag_Processor $processor, OD_Tag_Visit // Process the IMG element within the PICTURE. if ( 'IMG' === $tag && ! $processor->is_tag_closer() ) { - $src = $this->get_valid_src( $processor ); - if ( null === $src ) { + // Abort if process_img() won't later be processing this IMG. + if ( ! $this->is_img_with_valid_src_and_srcset( $processor ) ) { return false; } @@ -274,31 +331,7 @@ private function process_picture( OD_HTML_Tag_Processor $processor, OD_Tag_Visit ) ); - return false; - } - - /** - * Gets valid src attribute value for preloading. - * - * Returns null if the src attribute is not a string (i.e. src was used as a boolean attribute was used), if it - * it has an empty string value after trimming, or if it is a data: URL. - * - * @since n.e.x.t - * - * @param OD_HTML_Tag_Processor $processor Processor. - * @param 'src'|'srcset' $attribute_name Attribute name. - * @return non-empty-string|null URL which is not a data: URL. - */ - private function get_valid_src( OD_HTML_Tag_Processor $processor, string $attribute_name = 'src' ): ?string { - $src = $processor->get_attribute( $attribute_name ); - if ( ! is_string( $src ) ) { - return null; - } - $src = trim( $src ); - if ( '' === $src || $this->is_data_url( $src ) ) { - return null; - } - return $src; + return true; } /** diff --git a/plugins/image-prioritizer/class-image-prioritizer-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-tag-visitor.php index ba2850f6a8..a464d29334 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-tag-visitor.php @@ -14,7 +14,7 @@ /** * Tag visitor that optimizes image tags. * - * @phpstan-type NormalizedAttributeNames 'fetchpriority'|'loading'|'crossorigin'|'preload'|'referrerpolicy'|'type' + * @phpstan-type NormalizedAttributeNames 'src'|'srcset'|'fetchpriority'|'loading'|'crossorigin'|'preload'|'referrerpolicy'|'type' * * @since 0.1.0 * @access private diff --git a/plugins/image-prioritizer/tests/test-cases/img-non-native-lazy-loading.php b/plugins/image-prioritizer/tests/test-cases/img-non-native-lazy-loading.php new file mode 100644 index 0000000000..6cd029b771 --- /dev/null +++ b/plugins/image-prioritizer/tests/test-cases/img-non-native-lazy-loading.php @@ -0,0 +1,99 @@ + static function (): void {}, + + /* + * Example 1 comes from Avada's Fusion_Images lazy images which replaces the srcset attribute with data-srcset but + * which leaves the src attribute as-is. + * + * Example 2 comes from Speed Optimizer v7.7.2 by Site Ground which uses lazysizes v5.3.1. + * See . + */ + 'buffer' => ' + + + + ... + + + + + + + + + + Foo + + + + Foo + + + + Foo + + + + Bar + + + + ', + 'expected' => ' + + + + ... + + + + + + + + + + Foo + + + + Foo + + + + Foo + + + + Bar + + + + + ', +); diff --git a/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-having-media-attribute.php b/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-having-media-attribute.php index 8f2af94d69..72ee5e8292 100644 --- a/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-having-media-attribute.php +++ b/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-having-media-attribute.php @@ -1,24 +1,6 @@ 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; - } - ); - - $test_case->populate_url_metrics( - array( - array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::PICTURE]/*[2][self::IMG]', - 'isLCP' => true, - ), - ) - ); - }, + 'set_up' => static function ( Test_Image_Prioritizer_Helper $test_case ): void {}, 'buffer' => ' @@ -42,8 +24,9 @@ static function () use ( $breakpoint_max_widths ) { - Foo + Foo + ', diff --git a/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-missing-type-attribute.php b/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-missing-type-attribute.php index 24c4021b01..9cba6d5dc0 100644 --- a/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-missing-type-attribute.php +++ b/plugins/image-prioritizer/tests/test-cases/picture-element-with-source-missing-type-attribute.php @@ -1,24 +1,6 @@ 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; - } - ); - - $test_case->populate_url_metrics( - array( - array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::PICTURE]/*[2][self::IMG]', - 'isLCP' => true, - ), - ) - ); - }, + 'set_up' => static function (): void {}, 'buffer' => ' @@ -42,8 +24,9 @@ static function () use ( $breakpoint_max_widths ) { - Foo + Foo + ', diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 74548eef12..4d7bec7783 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -227,6 +227,11 @@ function od_optimize_template_output_buffer( string $buffer ): string { $needs_detection = ! $group_collection->is_every_group_complete(); do { + // Never process anything inside NOSCRIPT since it will never show up in the DOM when scripting is enabled, and thus it can never be detected nor measured. + if ( in_array( 'NOSCRIPT', $processor->get_breadcrumbs(), true ) ) { + continue; + } + $tracked_in_url_metrics = false; $processor->set_bookmark( $current_tag_bookmark ); // TODO: Should we break if this returns false? diff --git a/plugins/optimization-detective/tests/test-cases/noscript.php b/plugins/optimization-detective/tests/test-cases/noscript.php new file mode 100644 index 0000000000..9756210dbe --- /dev/null +++ b/plugins/optimization-detective/tests/test-cases/noscript.php @@ -0,0 +1,31 @@ + static function (): void {}, + 'buffer' => ' + + + + ... + + + + + + ', + 'expected' => ' + + + + ... + + + + + + + ', +);