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

Add preload links LCP picture elements #1707

Merged
merged 31 commits into from
Dec 5, 2024
Merged

Add preload links LCP picture elements #1707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
43525c0
Fix preloading of LCP images for <picture> elements
b1ink0 Nov 27, 2024
c20943c
Add media attribute support for <source> elements in image prioritiza…
b1ink0 Nov 27, 2024
83c19a2
Break logic into multiple methods for handling img and picture tag pr…
b1ink0 Nov 28, 2024
2b812fb
Remove handling of picture with no source scenario
b1ink0 Nov 29, 2024
df1d90e
Ensure source elements have a type attribute and no media attribute, …
b1ink0 Nov 29, 2024
b8a17c0
Refactor conditional logic so the cases are consistent
b1ink0 Dec 2, 2024
194075c
Refactor get_parent_tag_name to use breadcrumbs from context and rena…
b1ink0 Dec 2, 2024
ff8df96
Move pre_img_process logic directly into process_img method
b1ink0 Dec 2, 2024
bc178ba
Refactor to use single function for adding preload links
b1ink0 Dec 2, 2024
ed2e2da
Merge branch 'trunk' into fix/preload-lcp-images-for-picture-elements
b1ink0 Dec 2, 2024
664c228
Refactor how preload links are added
westonruter Dec 2, 2024
d76759e
Align phpdoc in get_breadcrumbs() with WP_HTML_Processor
westonruter Dec 2, 2024
a767385
Add test for type attribute on link
westonruter Dec 2, 2024
34272cd
Fix sourcing of crossorigin and referrerpolicy attributes
westonruter Dec 2, 2024
b468896
Only capture the first source
westonruter Dec 2, 2024
93a0f1b
Use upper-case tag names rather than markup in phpdoc
westonruter Dec 2, 2024
bf3932f
Add test for get_breadcrumbs() and fix bug in implicit P closing
westonruter Dec 2, 2024
71c198c
Merge branch 'trunk' into fix/preload-lcp-images-for-picture-elements
b1ink0 Dec 3, 2024
79d5fc3
Add test case for picture element with LCP image and fully populated …
b1ink0 Dec 3, 2024
9f593a2
Add test case for picture element with missing LCP metrics for tablet…
b1ink0 Dec 3, 2024
f3c2b6b
Add test case for picture element with missing type attribute in source
b1ink0 Dec 3, 2024
ac9f499
Add tests for picture element with media attribute in source
b1ink0 Dec 3, 2024
71adb58
Add test case for picture element with crossorigin and referrerpolicy…
b1ink0 Dec 3, 2024
d7d24b4
Merge branch 'trunk' into fix/preload-lcp-images-for-picture-elements
b1ink0 Dec 3, 2024
b4660de
Improve normalization of crossorigin attribute value
westonruter Dec 4, 2024
e7cb4a7
Harden processing of PICTURE tag
westonruter Dec 4, 2024
ebbf0d3
Supply missing array key
westonruter Dec 4, 2024
7d2e654
Factor out common logic to obtain valid src attribute
westonruter Dec 4, 2024
a92b899
Update test case to test for multiple sources
westonruter Dec 4, 2024
8f6550a
Remove largely duplicate test case by merging
westonruter Dec 4, 2024
ece0618
Reuse get_valid_src to obtain valid srcset
westonruter Dec 4, 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
242 changes: 214 additions & 28 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 @@ -14,6 +14,8 @@
/**
* Tag visitor that optimizes IMG tags.
*
* @phpstan-import-type LinkAttributes from OD_Link_Collection
*
* @since 0.1.0
* @access private
*/
Expand All @@ -22,19 +24,37 @@ final class Image_Prioritizer_Img_Tag_Visitor extends Image_Prioritizer_Tag_Visi
/**
* Visits a tag.
*
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @since 0.1.0
* @since n.e.x.t Separate the processing of IMG and PICTURE elements.
*
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @return bool Whether the tag should be tracked in URL Metrics.
*/
public function __invoke( OD_Tag_Visitor_Context $context ): bool {
$processor = $context->processor;
if ( 'IMG' !== $processor->get_tag() ) {
return false;
$tag = $processor->get_tag();

if ( 'PICTURE' === $tag ) {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of relying on this __invoke function being called on the PICTURE and then also on the child SOURCE tags (and IMG) tag, I suggest instead that when 'PICTURE' === $tag that this here call a new method like process_picture() and that this method would:

  1. Call $processor->set_bookmark( 'img-prioritizer-picture' )
  2. Add a while( $processor->next_tag() loop. Note that our subclass's next_tag() method always visits tag closers until $processor->get_tag() === 'picture' && $processor->is_tag_closer().
  3. Gather up the attributes data from the SOURCE tags and the final IMG tag.
  4. Finally reset the processor back to where you started: $processor->set_bookmark('img-prioritizer-picture').
  5. Process the data collected from the child tags, and call $context->link_collection->add_link(), leaving the other location as-is for just for processing bare IMG tags.

This approach of a sub-loop was also taken by Embed Optimizer in embed_optimizer_update_markup().

return $this->process_picture( $processor, $context );
} elseif ( 'IMG' === $tag ) {
return $this->process_img( $processor, $context );
}

// Skip empty src attributes and data: URLs.
$src = trim( (string) $processor->get_attribute( 'src' ) );
if ( '' === $src || $this->is_data_url( $src ) ) {
return false;
}

/**
* Process an IMG element.
*
* @since n.e.x.t
*
* @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.
*/
private function process_img( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_Context $context ): bool {
$src = $this->get_valid_src( $processor );
if ( null === $src ) {
return false;
}

Expand Down Expand Up @@ -142,41 +162,207 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
}
}

// If this element is the LCP (for a breakpoint group), add a preload link for it.
foreach ( $context->url_metric_group_collection->get_groups_by_lcp_element( $xpath ) as $group ) {
$link_attributes = array_merge(
$parent_tag = $this->get_parent_tag_name( $context );
if ( 'PICTURE' !== $parent_tag ) {
$this->add_image_preload_link_for_lcp_element_groups(
$context,
$xpath,
array(
'rel' => 'preload',
'fetchpriority' => 'high',
'as' => 'image',
),
array_filter(
array(
'href' => (string) $processor->get_attribute( 'src' ),
'imagesrcset' => (string) $processor->get_attribute( 'srcset' ),
'imagesizes' => (string) $processor->get_attribute( 'sizes' ),
),
static function ( string $value ): bool {
return '' !== $value;
}
'href' => $processor->get_attribute( 'src' ),
'imagesrcset' => $processor->get_attribute( 'srcset' ),
'imagesizes' => $processor->get_attribute( 'sizes' ),
'crossorigin' => $this->get_attribute_value( $processor, 'crossorigin' ),
'referrerpolicy' => $this->get_attribute_value( $processor, 'referrerpolicy' ),
)
);
}

return true;
}

/**
* Process a PICTURE element.
*
* @since n.e.x.t
*
* @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.
*/
private function process_picture( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_Context $context ): bool {
/**
* First SOURCE tag's attributes.
*
* @var array{ srcset: non-empty-string, sizes: string|null, type: non-empty-string }|null $first_source
*/
$first_source = null;
$img_xpath = null;

$referrerpolicy = null;
$crossorigin = null;

// Loop through child tags until we reach the closing PICTURE tag.
while ( $processor->next_tag() ) {
$tag = $processor->get_tag();

// If we reached the closing PICTURE tag, break.
if ( 'PICTURE' === $tag && $processor->is_tag_closer() ) {
break;
}

// Process the SOURCE elements.
if ( 'SOURCE' === $tag && ! $processor->is_tag_closer() ) {
// Abort processing if the PICTURE involves art direction since then adding a preload link is infeasible.
if ( null !== $processor->get_attribute( 'media' ) ) {
return false;
}

// Abort processing if a SOURCE lacks the required srcset attribute.
$srcset = $this->get_valid_src( $processor, 'srcset' );
if ( null === $srcset ) {
return false;
}

// Abort processing if there is no valid image type.
$type = $this->get_attribute_value( $processor, 'type' );
if ( ! is_string( $type ) || ! str_starts_with( $type, 'image/' ) ) {
return false;
}

// Collect the first valid SOURCE as the preload link.
if ( null === $first_source ) {
$sizes = $processor->get_attribute( 'sizes' );
$first_source = array(
'srcset' => $srcset,
'sizes' => is_string( $sizes ) ? $sizes : null,
'type' => $type,
);
}
}

// Process the IMG element within the PICTURE.
if ( 'IMG' === $tag && ! $processor->is_tag_closer() ) {
$src = $this->get_valid_src( $processor );
if ( null === $src ) {
return false;
}

$crossorigin = $this->get_attribute_value( $processor, 'crossorigin' );
if ( null !== $crossorigin ) {
$link_attributes['crossorigin'] = 'use-credentials' === $crossorigin ? 'use-credentials' : 'anonymous';
// These attributes are only defined on the IMG itself.
$referrerpolicy = $this->get_attribute_value( $processor, 'referrerpolicy' );
$crossorigin = $this->get_attribute_value( $processor, 'crossorigin' );

// Capture the XPath for the IMG since the browser captures it as the LCP element, so we need this to
// look up whether it is the LCP element in the URL Metric groups.
$img_xpath = $processor->get_xpath();
}
}

$link_attributes['media'] = 'screen';
// Abort if we never encountered a SOURCE or IMG tag.
if ( null === $img_xpath || null === $first_source ) {
return false;
}

$this->add_image_preload_link_for_lcp_element_groups(
$context,
$img_xpath,
array(
'imagesrcset' => $first_source['srcset'],
'imagesizes' => $first_source['sizes'],
'type' => $first_source['type'],
'crossorigin' => $crossorigin,
'referrerpolicy' => $referrerpolicy,
)
);

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

/**
* Adds a LINK with the supplied attributes for each viewport group when the provided XPath is the LCP element.
*
* @since n.e.x.t
*
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @param string $xpath XPath of the element.
* @param array<string, string|true|null> $attributes Attributes to add to the link.
*/
private function add_image_preload_link_for_lcp_element_groups( OD_Tag_Visitor_Context $context, string $xpath, array $attributes ): void {
$attributes = array_filter(
$attributes,
static function ( $attribute_value ) {
return is_string( $attribute_value ) && '' !== $attribute_value;
}
);

/**
* Link attributes.
*
* This type is needed because PHPStan isn't apparently aware of the new keys added after the array_merge().
* Note that there is no type checking being done on the attributes above other than ensuring they are
* non-empty-strings.
*
* @var LinkAttributes $attributes
*/
$attributes = array_merge(
array(
'rel' => 'preload',
'fetchpriority' => 'high',
'as' => 'image',
),
$attributes,
array(
'media' => 'screen',
)
);

foreach ( $context->url_metric_group_collection->get_groups_by_lcp_element( $xpath ) as $group ) {
$context->link_collection->add_link(
$link_attributes,
$attributes,
$group->get_minimum_viewport_width(),
$group->get_maximum_viewport_width()
);
}
}

return true;
/**
* Gets the parent tag name.
*
* @since n.e.x.t
*
westonruter marked this conversation as resolved.
Show resolved Hide resolved
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @return string|null The parent tag name or null if not found.
*/
private function get_parent_tag_name( OD_Tag_Visitor_Context $context ): ?string {
$breadcrumbs = $context->processor->get_breadcrumbs();
$length = count( $breadcrumbs );
if ( $length < 2 ) {
return null;
}
return $breadcrumbs[ $length - 2 ];
}

/**
Expand Down
10 changes: 9 additions & 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'
* @phpstan-type NormalizedAttributeNames 'fetchpriority'|'loading'|'crossorigin'|'preload'|'referrerpolicy'|'type'
*
* @since 0.1.0
* @access private
Expand Down Expand Up @@ -44,6 +44,7 @@ protected function is_data_url( string $url ): bool {
*
* @since 0.2.0
* @todo Move this into the OD_HTML_Tag_Processor/OD_HTML_Processor class eventually.
* @todo It would be nice if PHPStan could know that if you pass 'crossorigin' as $attribute_name that you will get back null|'anonymous'|'use-credentials'.
*
* @phpstan-param NormalizedAttributeNames $attribute_name
*
Expand All @@ -53,9 +54,16 @@ protected function is_data_url( string $url ): bool {
*/
protected function get_attribute_value( OD_HTML_Tag_Processor $processor, string $attribute_name ) {
$value = $processor->get_attribute( $attribute_name );
if ( null === $value ) {
return null;
}

if ( is_string( $value ) ) {
$value = strtolower( trim( $value, " \t\f\r\n" ) );
}
if ( 'crossorigin' === $attribute_name && 'use-credentials' !== $value ) {
$value = 'anonymous';
}
return $value;
}
}
27 changes: 17 additions & 10 deletions plugins/image-prioritizer/tests/test-cases/different-lcp-elements-for-all-breakpoints.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php
return array(
'set_up' => static function ( Test_Image_Prioritizer_Helper $test_case ): void {
$breakpoint_max_widths = array( 480, 600, 782 );
$breakpoint_max_widths = array( 480, 600, 782, 1000 );

add_filter(
'od_breakpoint_max_widths',
Expand All @@ -28,6 +28,10 @@ static function () use ( $breakpoint_max_widths ) {
'isLCP' => false,
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[4][self::IMG]',
),
array(
'isLCP' => false,
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[5][self::IMG]',
),
);
$elements[ $i ]['isLCP'] = true;
OD_URL_Metrics_Post_Type::store_url_metric(
Expand All @@ -49,9 +53,10 @@ static function () use ( $breakpoint_max_widths ) {
</head>
<body>
<img src="https://example.com/mobile-logo.png" alt="Mobile Logo" width="600" height="600" crossorigin>
<img src="https://example.com/phablet-logo.png" alt="Phablet Logo" width="600" height="600" crossorigin="">
<img src="https://example.com/tablet-logo.png" alt="Tablet Logo" width="600" height="600" crossorigin="anonymous">
<img src="https://example.net/desktop-logo.png" alt="Desktop Logo" width="600" height="600" crossorigin="use-credentials">
<img src="https://example.com/phablet-logo.png" alt="Phablet Logo" width="600" height="600" crossorigin="" referrerpolicy="no-referrer">
<img src="https://example.com/tablet-logo.png" alt="Tablet Logo" width="600" height="600" crossorigin="anonymous" referrerpolicy="no-referrer-when-downgrade">
<img src="https://example.net/desktop-logo.png" alt="Desktop Logo" width="600" height="600" crossorigin="use-credentials" referrerpolicy="origin-when-cross-origin">
<img src="https://example.net/ultra-desktop-logo.png" alt="Desktop Logo" width="600" height="600" crossorigin=" something-custom " referrerpolicy="same-origin">
</body>
</html>
',
Expand All @@ -61,15 +66,17 @@ static function () use ( $breakpoint_max_widths ) {
<meta charset="utf-8">
<title>...</title>
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/mobile-logo.png" crossorigin="anonymous" media="screen and (max-width: 480px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/phablet-logo.png" crossorigin="anonymous" media="screen and (min-width: 481px) and (max-width: 600px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/tablet-logo.png" crossorigin="anonymous" media="screen and (min-width: 601px) and (max-width: 782px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.net/desktop-logo.png" crossorigin="use-credentials" media="screen and (min-width: 783px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/phablet-logo.png" crossorigin="anonymous" referrerpolicy="no-referrer" media="screen and (min-width: 481px) and (max-width: 600px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/tablet-logo.png" crossorigin="anonymous" referrerpolicy="no-referrer-when-downgrade" media="screen and (min-width: 601px) and (max-width: 782px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.net/desktop-logo.png" crossorigin="use-credentials" referrerpolicy="origin-when-cross-origin" media="screen and (min-width: 783px) and (max-width: 1000px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.net/ultra-desktop-logo.png" crossorigin="anonymous" referrerpolicy="same-origin" media="screen and (min-width: 1001px)">
</head>
<body>
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]" src="https://example.com/mobile-logo.png" alt="Mobile Logo" width="600" height="600" crossorigin>
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::IMG]" src="https://example.com/phablet-logo.png" alt="Phablet Logo" width="600" height="600" crossorigin="">
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::IMG]" src="https://example.com/tablet-logo.png" alt="Tablet Logo" width="600" height="600" crossorigin="anonymous">
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[4][self::IMG]" src="https://example.net/desktop-logo.png" alt="Desktop Logo" width="600" height="600" crossorigin="use-credentials">
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::IMG]" src="https://example.com/phablet-logo.png" alt="Phablet Logo" width="600" height="600" crossorigin="" referrerpolicy="no-referrer">
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::IMG]" src="https://example.com/tablet-logo.png" alt="Tablet Logo" width="600" height="600" crossorigin="anonymous" referrerpolicy="no-referrer-when-downgrade">
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[4][self::IMG]" src="https://example.net/desktop-logo.png" alt="Desktop Logo" width="600" height="600" crossorigin="use-credentials" referrerpolicy="origin-when-cross-origin">
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[5][self::IMG]" src="https://example.net/ultra-desktop-logo.png" alt="Desktop Logo" width="600" height="600" crossorigin=" something-custom " referrerpolicy="same-origin">
<script type="module">/* import detect ... */</script>
</body>
</html>
Expand Down
Loading
Loading