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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ 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 @@ -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 ) ) {
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 @@ 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 @@ -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.
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 @@ 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 @@ -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;
}

/**
Expand Down
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php
return array(
'set_up' => 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 <https://plugins.trac.wordpress.org/browser/sg-cachepress/tags/7.7.2/core/Lazy_Load/Lazy_Load_Images.php>.
*/
'buffer' => '
<html lang="en">
<head>
<meta charset="utf-8">
<title>...</title>
<script>/* custom lazy-loading */</script>
</head>
<body>
<!-- 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">
</noscript>
</body>
</html>
',
'expected' => '
<html lang="en">
<head>
<meta charset="utf-8">
<title>...</title>
<script>/* custom lazy-loading */</script>
</head>
<body>
<!-- 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">
</noscript>
<script type="module">/* import detect ... */</script>
</body>
</html>
',
);
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
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
5 changes: 5 additions & 0 deletions plugins/optimization-detective/optimization.php
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down
31 changes: 31 additions & 0 deletions plugins/optimization-detective/tests/test-cases/noscript.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php
return array(
'set_up' => static function (): void {},
'buffer' => '
<html lang="en">
<head>
<meta charset="utf-8">
<title>...</title>
</head>
<body>
<noscript>
<img src="https://example.com/pixel.gif" alt="" width="1" height="1">
</noscript>
</body>
</html>
',
'expected' => '
<html lang="en">
<head>
<meta charset="utf-8">
<title>...</title>
</head>
<body>
<noscript>
<img src="https://example.com/pixel.gif" alt="" width="1" height="1">
</noscript>
<script type="module">/* import detect ... */</script>
</body>
</html>
',
);
Loading