Skip to content

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

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

Draft
wants to merge 4 commits into
base: trunk
Choose a base branch
from
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
4 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
2aa7235
Merge branch 'trunk' of https://github.com/WordPress/performance into…
westonruter Feb 7, 2025
524796e
Only look for directories when loading snapshot test cases
westonruter Feb 7, 2025
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 @@ -22,6 +22,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 @@ -36,14 +44,50 @@ 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 );
}

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 @@ -54,13 +98,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 ) ) {
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 @@ -188,7 +240,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 {
/**
Expand Down Expand Up @@ -219,8 +271,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.
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 @@ -243,8 +300,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;
}

Expand Down Expand Up @@ -275,31 +332,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 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 @@ -15,7 +15,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
33 changes: 32 additions & 1 deletion plugins/image-prioritizer/tests/test-cases/img-non-native-lazy-loading/buffer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,38 @@
</head>
<body>
<div id="page">
<!-- Example with an adjoining NOSCRIPT > IMG tag which should be excluded from URL Metrics. -->
<!-- Example from Avada's Fusion_Images lazy images which replaces the srcset attribute with data-srcset but which leaves the src attribute as-is. -->
<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"
>

<!-- Same example 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 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>.
Adjoining NOSCRIPT > IMG tag which should be excluded from URL Metrics.
-->
<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
33 changes: 32 additions & 1 deletion plugins/image-prioritizer/tests/test-cases/img-non-native-lazy-loading/expected.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 1 addition & 19 deletions plugins/image-prioritizer/tests/test-cases/img-non-native-lazy-loading/set-up.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,2 @@
<?php
return static function ( Test_Image_Prioritizer_Helper $test_case ): void {
$slug = od_get_url_metrics_slug( od_get_normalized_query_vars() );

// 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' => '/HTML/BODY/DIV[@id=\'page\']/*[1][self::IMG]',
'isLCP' => true,
),
),
)
)
);
};
return static function ( Test_Image_Prioritizer_Helper $test_case ): void {};
5 changes: 3 additions & 2 deletions ...ritizer/tests/test-cases/picture-element-with-source-having-media-attribute/expected.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 1 addition & 19 deletions ...rioritizer/tests/test-cases/picture-element-with-source-having-media-attribute/set-up.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,2 @@
<?php
return 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' => '/HTML/BODY/DIV[@id=\'page\']/*[1][self::PICTURE]/*[2][self::IMG]',
'isLCP' => true,
),
)
);
};
return static function ( Test_Image_Prioritizer_Helper $test_case ): void {};
5 changes: 3 additions & 2 deletions ...ritizer/tests/test-cases/picture-element-with-source-missing-type-attribute/expected.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 1 addition & 19 deletions ...rioritizer/tests/test-cases/picture-element-with-source-missing-type-attribute/set-up.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,2 @@
<?php
return 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' => '/HTML/BODY/DIV[@id=\'page\']/*[1][self::PICTURE]/*[2][self::IMG]',
'isLCP' => true,
),
)
);
};
return static function ( Test_Image_Prioritizer_Helper $test_case ): void {};
3 changes: 2 additions & 1 deletion plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ static function ( JsonSerializable $item ) {
*/
public function load_snapshot_test_cases( string $directory ): array {
$test_cases = array();
foreach ( (array) glob( $directory . '/*' ) as $test_case ) {
foreach ( (array) glob( $directory . '/*', GLOB_ONLYDIR ) as $test_case ) {

/**
* Test case directory.
*
Expand Down
Loading