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.

Sign up for GitHub

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 handling IMG tags with JS-based lazy-loading #1785

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
+137 −94
Draft

Fix handling IMG tags with JS-based lazy-loading #1785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b4cbc0
Add failing test case where data: in srcset should not be preloaded
westonruter Jan 9, 2025
e046d70
Fix handling images with JS-based lazy-loading
westonruter Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 66 additions & 33 deletions plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -35,14 +43,50 @@
$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 );
}

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.
*
Expand All @@ -53,13 +97,21 @@
* @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 ) ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

As noted below, this may be too conservative. We could still process this IMG. It's just we would skip any parts related to preloading the src or srcset.

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;
Expand Down Expand Up @@ -187,7 +239,7 @@
*
* @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 {
/**
Expand Down Expand Up @@ -218,8 +270,13 @@
}

// 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;

Check warning on line 275 in plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php

View check run for this annotation

Codecov / codecov/patch

plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php#L275 

Added line #L275 was not covered by tests
}

// Abort if the srcset is a data: URL since there is nothing to optimize.
Copy link
Member Author

Choose a reason for hiding this comment

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

Well, there could be something to optimize. We can't preload the URL, but we could still add fetchpriority=high to the IMG.

if ( $this->is_data_url( $srcset ) ) {
return false;
}

Expand All @@ -242,8 +299,8 @@

// 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;
}

Expand Down Expand Up @@ -274,31 +331,7 @@
)
);

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 0.3.0
*
* @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;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion plugins/image-prioritizer/class-image-prioritizer-tag-visitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 64 additions & 20 deletions plugins/image-prioritizer/tests/test-cases/img-non-native-lazy-loading.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,14 @@
<?php
return array(
'set_up' => static function ( Test_Image_Prioritizer_Helper $test_case ): void {
$slug = od_get_url_metrics_slug( od_get_normalized_query_vars() );
'set_up' => static function (): void {},

// Populate one URL Metric so that none of the IMG elements are unknown.
OD_URL_Metrics_Post_Type::store_url_metric(
$slug,
$test_case->get_sample_url_metric(
array(
'viewport_width' => 1000,
'elements' => array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]',
'isLCP' => true,
),
),
)
)
);
},
/*
* 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 <https://plugins.trac.wordpress.org/browser/sg-cachepress/tags/7.7.2/core/Lazy_Load/Lazy_Load_Images.php>.
*/
'buffer' => '
<html lang="en">
<head>
Expand All @@ -27,7 +17,34 @@
<script>/* custom lazy-loading */</script>
</head>
<body>
<!-- Example with an adjoining NOSCRIPT > IMG tag which should be excluded from URL Metrics. -->
<!-- Example 1 -->
<img
src="https://example.com/foo.webp"
data-orig-src="https://example.com/foo.webp"
width="1000"
height="800"
class="lazyload wp-image-1"
srcset="data:image/svg+xml,%3Csvg%20xmlns%3D%27http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%27%20width%3D%271000%27%20height%3D%27800%27%20viewBox%3D%270%200%201000%20800%27%3E%3Crect%20width%3D%271000%27%20height%3D%27800%27%20fill-opacity%3D%220%22%2F%3E%3C%2Fsvg%3E"
data-srcset="https://example.com/foo-200x91.webp 200w, https://example.com/foo-300x136.webp 300w, https://example.com/foo-400x181.webp 400w, https://example.com/foo-600x272.webp 600w, https://example.com/foo-768x348.webp 768w, https://example.com/foo-800x362.webp 800w, https://example.com/foo-1024x463.webp 1024w, https://example.com/foo-1200x543.webp 1200w, https://example.com/foo-1536x695.webp 1536w, https://example.com/foo.webp 1920w"
data-sizes="auto"
data-orig-sizes="(max-width: 767px) 100vw, 1920px"
>

<!-- Example 1 extended to PICTURE, where none of these should be tracked in URL Metrics -->
<picture>
<source type="image/avif" srcset="https://example.com/foo-300x225.avif 300w, https://example.com/foo-1024x768.avif 1024w, https://example.com/foo-768x576.avif 768w, https://example.com/foo-1536x1152.avif 1536w, https://example.com/foo-2048x1536.avif 2048w" sizes="(max-width: 600px) 480px, 800px">
<img class="lazyload" width="1200" height="800" src="https://example.com/foo.avif" alt="Foo" srcset="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" data-srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
</picture>
<picture>
<source type="image/avif" srcset="https://example.com/foo-300x225.avif 300w, https://example.com/foo-1024x768.avif 1024w, https://example.com/foo-768x576.avif 768w, https://example.com/foo-1536x1152.avif 1536w, https://example.com/foo-2048x1536.avif 2048w" sizes="(max-width: 600px) 480px, 800px">
<img class="lazyload" width="1200" height="800" src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" alt="Foo">
</picture>
<picture>
<source type="image/avif" srcset="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" data-srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
<img class="lazyload" width="1200" height="800" src="https://example.com/foo.avif" alt="Foo">
</picture>

<!-- Example 2 -->
<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" data-src="https://example.com/bar.jpg" data-srcset="https://example.com/bar-large.jpg 1000w, https://example.com/bar-large.jpg 1000w" sizes="(max-width: 556px) 100vw, 556px" alt="Bar" class="attachment-large size-large wp-image-2 has-transparency lazyload" width="500" height="300">
<noscript>
<img src="https://example.com/bar.jpg" srcset="https://example.com/bar-large.jpg 1000w, https://example.com/bar-large.jpg 1000w" sizes="(max-width: 556px) 100vw, 556px" alt="Bar" class="attachment-large size-large wp-image-2 has-transparency lazyload" width="500" height="300">
Expand All @@ -43,7 +60,34 @@
<script>/* custom lazy-loading */</script>
</head>
<body>
<!-- Example with an adjoining NOSCRIPT > IMG tag which should be excluded from URL Metrics. -->
<!-- Example 1 -->
<img
src="https://example.com/foo.webp"
data-orig-src="https://example.com/foo.webp"
width="1000"
height="800"
class="lazyload wp-image-1"
srcset="data:image/svg+xml,%3Csvg%20xmlns%3D%27http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%27%20width%3D%271000%27%20height%3D%27800%27%20viewBox%3D%270%200%201000%20800%27%3E%3Crect%20width%3D%271000%27%20height%3D%27800%27%20fill-opacity%3D%220%22%2F%3E%3C%2Fsvg%3E"
data-srcset="https://example.com/foo-200x91.webp 200w, https://example.com/foo-300x136.webp 300w, https://example.com/foo-400x181.webp 400w, https://example.com/foo-600x272.webp 600w, https://example.com/foo-768x348.webp 768w, https://example.com/foo-800x362.webp 800w, https://example.com/foo-1024x463.webp 1024w, https://example.com/foo-1200x543.webp 1200w, https://example.com/foo-1536x695.webp 1536w, https://example.com/foo.webp 1920w"
data-sizes="auto"
data-orig-sizes="(max-width: 767px) 100vw, 1920px"
>

<!-- Example 1 extended to PICTURE, where none of these should be tracked in URL Metrics -->
<picture>
<source type="image/avif" srcset="https://example.com/foo-300x225.avif 300w, https://example.com/foo-1024x768.avif 1024w, https://example.com/foo-768x576.avif 768w, https://example.com/foo-1536x1152.avif 1536w, https://example.com/foo-2048x1536.avif 2048w" sizes="(max-width: 600px) 480px, 800px">
<img class="lazyload" width="1200" height="800" src="https://example.com/foo.avif" alt="Foo" srcset="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" data-srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
</picture>
<picture>
<source type="image/avif" srcset="https://example.com/foo-300x225.avif 300w, https://example.com/foo-1024x768.avif 1024w, https://example.com/foo-768x576.avif 768w, https://example.com/foo-1536x1152.avif 1536w, https://example.com/foo-2048x1536.avif 2048w" sizes="(max-width: 600px) 480px, 800px">
<img class="lazyload" width="1200" height="800" src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" alt="Foo">
</picture>
<picture>
<source type="image/avif" srcset="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" data-srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
<img class="lazyload" width="1200" height="800" src="https://example.com/foo.avif" alt="Foo">
</picture>

<!-- Example 2 -->
<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" data-src="https://example.com/bar.jpg" data-srcset="https://example.com/bar-large.jpg 1000w, https://example.com/bar-large.jpg 1000w" sizes="(max-width: 556px) 100vw, 556px" alt="Bar" class="attachment-large size-large wp-image-2 has-transparency lazyload" width="500" height="300">
<noscript>
<img src="https://example.com/bar.jpg" srcset="https://example.com/bar-large.jpg 1000w, https://example.com/bar-large.jpg 1000w" sizes="(max-width: 556px) 100vw, 556px" alt="Bar" class="attachment-large size-large wp-image-2 has-transparency lazyload" width="500" height="300">
Expand Down
23 changes: 3 additions & 20 deletions ...image-prioritizer/tests/test-cases/picture-element-with-source-having-media-attribute.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,6 @@
<?php
return array(
'set_up' => 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' => '
<html lang="en">
<head>
Expand All @@ -42,8 +24,9 @@ static function () use ( $breakpoint_max_widths ) {
<body>
<picture>
<source type="image/avif" media="(max-width: 600px)" srcset="https://example.com/foo-300x225.avif 300w, https://example.com/foo-1024x768.avif 1024w, https://example.com/foo-768x576.avif 768w, https://example.com/foo-1536x1152.avif 1536w, https://example.com/foo-2048x1536.avif 2048w" sizes="(max-width: 600px) 480px, 800px">
<img data-od-fetchpriority-already-added fetchpriority="high" decoding="async" width="1200" height="800" src="https://example.com/foo.jpg" alt="Foo" srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
<img fetchpriority="high" decoding="async" width="1200" height="800" src="https://example.com/foo.jpg" alt="Foo" srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
</picture>
<script type="module">/* import detect ... */</script>
</body>
</html>
',
Expand Down
23 changes: 3 additions & 20 deletions ...image-prioritizer/tests/test-cases/picture-element-with-source-missing-type-attribute.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,6 @@
<?php
return array(
'set_up' => 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' => '
<html lang="en">
<head>
Expand All @@ -42,8 +24,9 @@ static function () use ( $breakpoint_max_widths ) {
<body>
<picture>
<source srcset="https://example.com/foo-300x225.avif 300w, https://example.com/foo-1024x768.avif 1024w, https://example.com/foo-768x576.avif 768w, https://example.com/foo-1536x1152.avif 1536w, https://example.com/foo-2048x1536.avif 2048w" sizes="(max-width: 600px) 480px, 800px">
<img data-od-fetchpriority-already-added fetchpriority="high" decoding="async" width="1200" height="800" src="https://example.com/foo.jpg" alt="Foo" srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
<img fetchpriority="high" decoding="async" width="1200" height="800" src="https://example.com/foo.jpg" alt="Foo" srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="(max-width: 600px) 480px, 800px">
</picture>
<script type="module">/* import detect ... */</script>
</body>
</html>
',
Expand Down
Loading