Add preload links LCP picture elements #1707

b1ink0 · 2024-11-27T17:52:33Z

This PR fixes the issue of preloading fallback image instead of preloading the images in the source element when wrapped in picture element.

Example Picture element

<picture class="wp-picture-7" style="display: contents;">
   <source 
        type="image/avif" 
        srcset="
          http://localhost:8888/wp-content/uploads/2024/11/demo-img-1024x683-jpg.avif 1024w, 
          http://localhost:8888/wp-content/uploads/2024/11/demo-img-300x200-jpg.avif 300w, 
          http://localhost:8888/wp-content/uploads/2024/11/demo-img-768x512-jpg.avif 768w, 
          http://localhost:8888/wp-content/uploads/2024/11/demo-img-1536x1024-jpg.avif 1536w, 
          http://localhost:8888/wp-content/uploads/2024/11/demo-img-2048x1365-jpg.avif 2048w
        " 
        sizes="(max-width: 645px) 100vw, 645px"
   >
   <img 
        data-od-removed-fetchpriority="high" 
        data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]/*[2][self::MAIN]/*[1][self::DIV]/*[3][self::DIV]/*[2][self::FIGURE]/*[1][self::PICTURE]/*[2][self::IMG]" 
        data-dominant-color="6a6a6a" 
        data-has-transparency="false" 
        style="--dominant-color: #6a6a6a;" 
        decoding="async" 
        width="1024" 
        height="683" 
        src="http://localhost:8888/wp-content/uploads/2024/11/demo-img-1024x683.jpg" 
        alt="" 
        class="wp-image-7 not-transparent" 
        srcset="
            http://localhost:8888/wp-content/uploads/2024/11/demo-img-1024x683.jpg 1024w,
            http://localhost:8888/wp-content/uploads/2024/11/demo-img-300x200.jpg 300w,
            http://localhost:8888/wp-content/uploads/2024/11/demo-img-768x512.jpg 768w,
            http://localhost:8888/wp-content/uploads/2024/11/demo-img-1536x1024.jpg 1536w,
            http://localhost:8888/wp-content/uploads/2024/11/demo-img-2048x1365.jpg 2048w
        " 
       sizes="(max-width: 645px) 100vw, 645px"
    >
</picture>

Preload Link Before

<link 
    data-od-added-tag="" 
    rel="preload" 
    fetchpriority="high" 
    as="image" 
    href="http://localhost:8888/wp-content/uploads/2024/11/demo-img-1024x683.jpg" 
    imagesrcset="
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-1024x683.jpg 1024w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-300x200.jpg 300w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-768x512.jpg 768w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-1536x1024.jpg 1536w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-2048x1365.jpg 2048w
    " 
    imagesizes="(max-width: 645px) 100vw, 645px" media="screen and (max-width: 480px)"
>

Preload Link After

<link 
    data-od-added-tag="" 
    rel="preload" 
    fetchpriority="high" 
    as="image" 
    imagesrcset="
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-1024x683-jpg.avif 1024w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-300x200-jpg.avif 300w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-768x512-jpg.avif 768w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-1536x1024-jpg.avif 1536w,
        http://localhost:8888/wp-content/uploads/2024/11/demo-img-2048x1365-jpg.avif 2048w
    " 
    imagesizes="(max-width: 645px) 100vw, 645px" type="image/avif" media="screen and (max-width: 480px)"
>

This PR also introduces a public OD_HTML_Tag_Processor::get_breadcrumbs() method which mirrors the functionality of WP_HTML_Processor::get_breadcrumbs(). As part of that, a bug was discovered in how OD_HTML_Tag_Processor handles implicitly-closed P tags which resulted in duplicate XPaths being generated.

Additionally, when an IMG had a referrerpolicy attribute this is copied to the preload LINK the same as the crossorigin attribute is copied.

…tion

github-actions · 2024-11-27T17:52:46Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: b1ink0 <b1ink0@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

westonruter

Thanks for working on this!

westonruter · 2024-11-27T17:56:29Z

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

+	/**
+	 * Flag to indicate whether we're within a `<picture>` element.
+	 *
+	 * @var bool
+	 */
+	private $is_within_picture = false;
+
+	/**
+	 * Collected `<source>` elements within the current `<picture>`.
+	 *
+	 * @var array<int, array{srcset?: string|true|null,sizes?: string|true|null,type?: string|true|null,media?: string|true|null,crossorigin?: string|true|null}>
+	 */
+	private $collected_sources = array();
+


Do these need to be member variables? They're only ever used in one method, so wouldn't they make more sense as just regular variables?

Oh, well I see it's because you're relying on this being invoked multiple times, so you're wanting to persist this state across multiple tag visits.

westonruter · 2024-11-27T17:57:59Z

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

+				// Opening <picture> tag.
+				$this->is_within_picture = true;
+				$this->collected_sources = array();
+			} else {


Actually, this condition should never occur because in the optimization loop it only ever visits open tags:

performance/plugins/optimization-detective/optimization.php

Line 242 in 69eca9a

} while ( $processor->next_open_tag() );

westonruter · 2024-11-27T18:06:12Z

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

+			$this->collected_sources[] = array(
+				'srcset'      => $processor->get_attribute( 'srcset' ),
+				'sizes'       => $processor->get_attribute( 'sizes' ),
+				'type'        => $processor->get_attribute( 'type' ),
+				'media'       => $processor->get_attribute( 'media' ),
+				'crossorigin' => $this->get_attribute_value( $processor, 'crossorigin' ),
+			);


I think this needs some additional logic. We need to prevent a scenario where there is a PICTURE with the first SOURCE being an AVIF image and then the second being a WebP, with the third being a JPEG fallback. We must only ever add a preload link for the first SOURCE because otherwise if a browser supports AVIF and WebP then they could end up preloading both but only the first would be used.

See comments:

Image Prioritizer fails to preload LCP images for picture elements #1312 (comment)

Image Prioritizer fails to preload LCP images for picture elements #1312 (comment)

Hi can you please complete this comment it seems to be cutoff.

I have identified different cases which we need to handle.

Picture element with multiple source tags without media attribute.

Select first source

Picture element with multiple source tags with media attribute.

Add preload link for every source

Picture element with no source tags.

Add preload link using img tags srcset

Picture element with multiple source but some have media attribute and some don't have media attribute.

How should we handle this case?

Did I miss some cases?

Oops. Sorry for the poor comment. Not sure how that happened.

Picture element with multiple source tags without media attribute.

And naturally with type attributes.

2. Picture element with multiple source tags with media attribute.

And without type attributes.

3. Picture element with no source tags.

This would be an error scenario. A PICTURE tag without a SOURCE is not valid. So just do nothing in this case.

4. Picture element with multiple source but some have media attribute and some don't have media attribute.

I think this would be an unusual case. I suggest we skip this scenario.

In fact, I suggest we may want to skip handling PICTURE tags that have SOURCE tags with media attributes entirely. This greatly complicate things, since we'd need to make sure that only one of the SOURCE tags ever get preloaded, but this would require computing some negated media query or something. As I recall, when a PICTURE decides which SOURCE to use, it will iterate over the SOURCE tags until it arrives at the first one that has a matching media and has a supported type. But this first-match logic is difficult to replicate for preload links.

Since our Modern Image Formats plugin only outputs SOURCE tags with type attributes, this would be the most important to support.

We can leave support for media attributes for a future enhancement, especially as we find there is a need for it.

Gemini concurs that preloading SOURCE with media attributes is not feasible, after initially getting it wrong: https://g.co/gemini/share/57ac56545028

So yeah, for PICTURE, grab the first SOURCE child and only preload it if none of the SOURCE tags have a media attribute. The only way to properly prioritize such an image is to ensure that fetchpriority=high is on the IMG tag, which we should make sure is happening in addition to adding preload links.

I have now implemented this in df1d90e

So yeah, for PICTURE, grab the first SOURCE child and only preload it if none of the SOURCE tags have a media attribute.

But I have a doubt about the part

The only way to properly prioritize such an image is to ensure that fetchpriority=high is on the IMG tag, which we should make sure is happening in addition to adding preload links.

Currently preload link has fetchpriority=high set but the image of that picture element has data-od-removed-fetchpriority="high" should we add fetchpriority=high to the image and remove the added data-od-removed-fetchpriority aatribute?

OK, I just tested this out and it is actually working currently as expected. Chrome is reporting that the LCP element is the PICTURE > IMG tag, not the PICTURE wrapper tag:

So when all viewport groups are populated with URL Metrics, it is resulting in the following markup in trunk:

<picture class="wp-picture-394" style="display: contents" ><source type="image/webp" srcset=" http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-1024x683-jpg.webp 1024w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-300x200-jpg.webp 300w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-768x512-jpg.webp 768w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-1536x1024-jpg.webp 1536w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-jpg.webp 1620w " sizes="(max-width: 1024px) 100vw, 1024px" /> <img data-od-fetchpriority-already-added="" data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[3][self::DIV]/*[1][self::DIV]/*[1][self::MAIN]/*[1][self::ARTICLE]/*[2][self::DIV]/*[1][self::FIGURE]/*[1][self::PICTURE]/*[2][self::IMG]" data-dominant-color="8a9464" data-has-transparency="false" style="--dominant-color: #8a9464" fetchpriority="high" decoding="async" width="1024" height="683" src="http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-1024x683.jpg" alt="" class="wp-image-394 not-transparent" srcset=" http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-1024x683.jpg 1024w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-300x200.jpg 300w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-768x512.jpg 768w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-1536x1024.jpg 1536w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi-1568x1045.jpg 1568w, http://localhost:8888/wp-content/uploads/2024/11/1620px-Australasian_swamphen_Porphyrio_melanotus_Tiritiri_Matangi.jpg 1620w " sizes="(max-width: 1024px) 100vw, 1024px" /></picture>

I believe you saw data-od-removed-fetchpriority="high" being added because you didn't have URL Metrics collected for some of the viewport groups, and currently the OD_URL_Metric_Group_Collection::get_common_lcp_element() method is defined so that all URL Metrics groups must be populated in order for the method to return an element. I think this should be changed to align better with how we determine whether to do lazy-loading: if both the narrowest viewport group (i.e. mobile) and the widest viewport group (i.e. desktop) are populated with URL Metrics and these groups both return an LCP Element via $group->get_lcp_element() which has the same XPath, then we should consider that it is likely for all of the viewport groups to have the same LCP element (i.e. phablet and tablet).

I've filed this in #1710.

westonruter · 2024-11-27T18:18:15Z

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

+		$tag       = $processor->get_tag();
+
+		// Handle opening and closing `<picture>` tags.
+		if ( 'PICTURE' === $tag ) {


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:

Call $processor->set_bookmark( 'img-prioritizer-picture' )

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().

Gather up the attributes data from the SOURCE tags and the final IMG tag.

Finally reset the processor back to where you started: $processor->set_bookmark('img-prioritizer-picture').

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().

westonruter · 2024-11-27T18:21:30Z

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

+			if ( $this->is_within_picture && count( $this->collected_sources ) > 0 ) {
+				foreach ( $this->collected_sources as $source ) {
+					$link_attributes = array_merge(
+						array(
+							'rel'           => 'preload',
+							'fetchpriority' => 'high',
+							'as'            => 'image',
+						),
+						array_filter(
+							array(
+								'href'        => isset( $source['srcset'] ) && is_string( $source['srcset'] )
+													? explode( ' ', $source['srcset'] )[0]
+													: '',
+								'imagesrcset' => isset( $source['srcset'] ) && is_string( $source['srcset'] ) ? $source['srcset'] : '',
+								'imagesizes'  => isset( $source['sizes'] ) && is_string( $source['sizes'] ) ? $source['sizes'] : '',
+								'type'        => isset( $source['type'] ) && is_string( $source['type'] ) ? $source['type'] : '',
+								'media'       => isset( $source['media'] ) && is_string( $source['media'] ) ? 'screen and ' . $source['media'] : 'screen',
+							),
+							static function ( string $value ): bool {
+								return '' !== $value;
+							}
+						)
+					);
+
+					if ( isset( $source['crossorigin'] ) ) {
+						$link_attributes['crossorigin'] = 'use-credentials' === $source['crossorigin'] ? 'use-credentials' : 'anonymous';
+					}
+
+					$context->link_collection->add_link(
+						$link_attributes,
+						$group->get_minimum_viewport_width(),
+						$group->get_maximum_viewport_width()
+					);
+				}
+			} else {


As noted above, I think this logic should rather go into a process_picture() method along with the other new logic above.

westonruter · 2024-11-27T18:23:09Z

We'll also need to add unit test cases specifically to test the various PICTURE scenarios.

…eload

westonruter · 2024-11-28T18:17:57Z

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

+
+		if ( 'PICTURE' === $tag ) {
+			$this->process_picture( $processor, $context );
+			return false;


This would need to return true because we need to capture this tag in URL Metrics, correct? Or maybe we don't if the browser reports that the IMG inside of the PICTURE is the LCP element. We should double check which element is reported as LCP.

Yes its need to be true now the process_picture return will handle should URL Metrics should be tracked or not.

…Only add preload link for first source

westonruter · 2024-11-30T00:41:58Z

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

+		}
+
+		if ( 'IMG' !== $tag ) {
 			return false;
 		}

+		return $this->process_img( $processor, $context );
+	}


Might as well refactor this a bit so the cases are consistent:

Suggested change

}

if ( 'IMG' !== $tag ) {

return false;

}

return $this->process_img( $processor, $context );

}

} elseif ( 'IMG' === $tag ) {

return $this->process_img( $processor, $context );

}

return false;

}

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

westonruter · 2024-11-30T00:52:44Z

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

+		$steps = explode( '/', $xpath );
+		if ( count( $steps ) < 3 ) {
+			// There is no parent.
+			return null;
+		}
+		$second_last_step = $steps[ count( $steps ) - 2 ];
+		if ( (bool) preg_match( '/\[self::([^\]]+)\]/', $second_last_step, $matches ) ) {
+			return $matches[1];
+		}
+		return null;


There is currently a private OD_HTML_Tag_Processor::get_breadcrumbs() method which returns an array of tuples for the tag name and index for each ancestor tag. I think we could reuse that here. I suggest:

Rename the existing OD_HTML_Tag_Processor::get_breadcrumbs() to get_indexed_breadcrumbs().

Add a new public method OD_HTML_Tag_Processor::get_breadcrumbs() which simply returns $this->open_stack_tags, which would be essentially providing the WP_HTML_Processor::get_breadcrumbs() method in the WP_HTML_Tag_Processor context.

This get_parent_tag_name() method could then be implemented as:

$breadcrumbs = $context->processor->get_breadcrumbs(); $length = count( $breadcrumbs ); if ( $length < 2 ) { return null; } return $breadcrumbs[ $length - 2 ];

westonruter · 2024-11-30T00:59:56Z

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

+		if ( 'PICTURE' === $parent_tag ) {
+			return true;
+		}
+
+		$this->pre_img_process( $processor, $context, $xpath );
+
+		$this->add_preload_link_for_img( $processor, $context, $xpath );


Suggested change

if ( 'PICTURE' === $parent_tag ) {

return true;

}

$this->pre_img_process( $processor, $context, $xpath );

$this->add_preload_link_for_img( $processor, $context, $xpath );

if ( 'PICTURE' !== $parent_tag ) {

$this->pre_img_process( $processor, $context, $xpath );

$this->add_preload_link_for_img( $processor, $context, $xpath );

}

westonruter · 2024-11-30T01:08:33Z

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

+
+		$this->add_preload_link_for_picture( $context, $img_xpath, $collected_sources[0] );
+
+		return true;


I don't believe we need to track the PICTURE in URL Metrics because only the IMG is reported as the LCP element. I don't believe that either PICTURE or SOURCE matter in terms of the intersection observer either.

Suggested change

return true;

return false;

westonruter · 2024-11-30T01:14:36Z

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

+
+				// Ensure that all <source> elements have a type attribute and no media attribute.
+				if ( null !== $media || null === $type ) {
+					return false;


Problem: When this happens, the IMG will no longer get its necessary optimizations.

Were the necessary optimizations happening in the pre_img_process function? if thats the case then as the logic of that function is now moved directly into the process_img function the optimizations will still happening even if this function returns here.

westonruter · 2024-11-30T01:16:11Z

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

+			// Process the <img> element within the <picture>.
+			if ( 'IMG' === $tag && ! $processor->is_tag_closer() ) {
+				// Skip empty src attributes and data: URLs.
+				$src = trim( (string) $processor->get_attribute( 'src' ) );
+				if ( '' === $src || $this->is_data_url( $src ) ) {
+					return false;
+				}
+
+				$img_xpath = $processor->get_xpath();
+
+				$this->pre_img_process( $processor, $context, $img_xpath );
+			}


Do we need this here? Couldn't we just rather leave the existing IMG handling in place when __invoke() lands on an IMG tag? We would just modify the logic there to only add preload links if the parent is not a PICTURE.

Suggested change

// Process the <img> element within the <picture>.

if ( 'IMG' === $tag && ! $processor->is_tag_closer() ) {

// Skip empty src attributes and data: URLs.

$src = trim( (string) $processor->get_attribute( 'src' ) );

if ( '' === $src || $this->is_data_url( $src ) ) {

return false;

}

$img_xpath = $processor->get_xpath();

$this->pre_img_process( $processor, $context, $img_xpath );

}

I have removed the pre_img_process as this processed directly in process_img but the below logic is still there as it is needed for images xpath which is required for adding preload links in process_picture.

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

westonruter · 2024-11-30T01:17:58Z

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

+	 * @param OD_Tag_Visitor_Context $context Tag visitor context.
+	 * @param string                 $xpath XPath of the element.
+	 */
+	private function pre_img_process( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_Context $context, string $xpath ): void {


Per above, I don't think this needs to be in another method. It can remain in process_img(), with one change: it should only proceed to add preload links for the IMG if the parent is not a PICTURE.

westonruter · 2024-11-30T01:19:06Z

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

+	 * @param string                                                                                                                                    $xpath   XPath of the element.
+	 * @param array{srcset?: string|true|null,sizes?: string|true|null,type?: string|true|null,media?: string|true|null,crossorigin?: string|true|null} $source  Collected sources from the <source> elements.
+	 */
+	private function add_preload_link_for_picture( OD_Tag_Visitor_Context $context, string $xpath, array $source ): void {


I think this logic could be put inside process_picture(), unless there is an opportunity to refactor it so that it can be reused both for PICTURE and IMG.

westonruter · 2024-11-30T01:20:12Z

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

+	 * @param OD_Tag_Visitor_Context $context   Tag visitor context.
+	 * @param string                 $xpath     XPath of the element.
+	 */
+	private function add_preload_link_for_img( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_Context $context, string $xpath ): void {


Per above, I think this logic can remain in process_img but just to wrap it in a check like if ( 'PICTURE' !== $parent_tag ).

Also, I think this logic could be put inside process_img(), unless there is an opportunity to refactor it so that it can be reused both for PICTURE and IMG.

…me get_breadcrumbs to get_indexed_breadcrumbs

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

plugins/optimization-detective/class-od-html-tag-processor.php

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

westonruter · 2024-12-02T19:32:40Z

This is looking pretty good! The biggest remaining piece is to add test cases for the new optimization code.

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

…sample data

… and desktop

… attributes

westonruter · 2024-12-04T23:24:50Z

plugins/optimization-detective/class-od-html-tag-processor.php

@@ -296,7 +296,7 @@ public function next_token(): bool {
 				$i = array_search( 'P', $this->open_stack_tags, true );
 				if ( false !== $i ) {
 					array_splice( $this->open_stack_tags, (int) $i );
-					array_splice( $this->open_stack_indices, count( $this->open_stack_tags ) );
+					array_splice( $this->open_stack_indices, count( $this->open_stack_tags ) + 1 );


Fixing a classic off-by-one error.

westonruter

🎉

b1ink0 added 2 commits November 27, 2024 18:46

Fix preloading of LCP images for <picture> elements

43525c0

Add media attribute support for <source> elements in image prioritiza…

c20943c

…tion

b1ink0 requested review from westonruter and felixarntz as code owners November 27, 2024 17:52

westonruter requested changes Nov 27, 2024

View reviewed changes

westonruter added this to the image-prioritizer n.e.x.t milestone Nov 27, 2024

westonruter added [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Type] Enhancement A suggestion for improvement of an existing feature labels Nov 27, 2024

westonruter changed the title ~~Image Prioritizer fails to preload LCP images for picture elements~~ Add preload links LCP picture elements Nov 27, 2024

Break logic into multiple methods for handling img and picture tag pr…

83c19a2

…eload

westonruter reviewed Nov 28, 2024

View reviewed changes

b1ink0 added 2 commits November 29, 2024 18:31

Remove handling of picture with no source scenario

2b812fb

Ensure source elements have a type attribute and no media attribute, …

df1d90e

…Only add preload link for first source

westonruter mentioned this pull request Nov 29, 2024

Allow fetchpriority=high to be added to IMG when it is the LCP element on desktop and mobile with other viewport groups empty #1710

Closed

westonruter requested changes Nov 30, 2024

View reviewed changes

b1ink0 and others added 7 commits December 2, 2024 15:27

Refactor conditional logic so the cases are consistent

b8a17c0

Refactor get_parent_tag_name to use breadcrumbs from context and rena…

194075c

…me get_breadcrumbs to get_indexed_breadcrumbs

Move pre_img_process logic directly into process_img method

ff8df96

Refactor to use single function for adding preload links

bc178ba

Merge branch 'trunk' into fix/preload-lcp-images-for-picture-elements

ed2e2da

Refactor how preload links are added

664c228

Align phpdoc in get_breadcrumbs() with WP_HTML_Processor

d76759e

westonruter reviewed Dec 2, 2024

View reviewed changes

westonruter mentioned this pull request Dec 2, 2024

Preload image URLs for LCP elements with external background images #1697

Merged

2 tasks

westonruter added 3 commits December 2, 2024 13:53

Add test for type attribute on link

a767385

Fix sourcing of crossorigin and referrerpolicy attributes

34272cd

Only capture the first source

b468896

westonruter reviewed Dec 2, 2024

View reviewed changes

plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php Outdated Show resolved Hide resolved

westonruter and others added 9 commits December 2, 2024 14:47

Use upper-case tag names rather than markup in phpdoc

93a0f1b

Add test for get_breadcrumbs() and fix bug in implicit P closing

bf3932f

Merge branch 'trunk' into fix/preload-lcp-images-for-picture-elements

71c198c

Add test case for picture element with LCP image and fully populated …

79d5fc3

…sample data

Add test case for picture element with missing LCP metrics for tablet…

9f593a2

… and desktop

Add test case for picture element with missing type attribute in source

f3c2b6b

Add tests for picture element with media attribute in source

ac9f499

Add test case for picture element with crossorigin and referrerpolicy…

71adb58

… attributes

Merge branch 'trunk' into fix/preload-lcp-images-for-picture-elements

d7d24b4

b1ink0 requested a review from westonruter December 3, 2024 17:57

westonruter added 3 commits December 4, 2024 14:54

Improve normalization of crossorigin attribute value

b4660de

Harden processing of PICTURE tag

e7cb4a7

Supply missing array key

ebbf0d3

westonruter reviewed Dec 4, 2024

View reviewed changes

westonruter added 4 commits December 4, 2024 15:35

Factor out common logic to obtain valid src attribute

7d2e654

Update test case to test for multiple sources

a92b899

Remove largely duplicate test case by merging

8f6550a

Reuse get_valid_src to obtain valid srcset

ece0618

westonruter approved these changes Dec 5, 2024

View reviewed changes

westonruter merged commit 3c5dd7f into WordPress:trunk Dec 5, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add preload links LCP picture elements #1707

Add preload links LCP picture elements #1707

b1ink0 commented Nov 27, 2024 •

edited by westonruter

Loading

github-actions bot commented Nov 27, 2024

westonruter left a comment

westonruter Nov 27, 2024

westonruter Nov 27, 2024

westonruter Nov 27, 2024

westonruter Nov 27, 2024 •

edited

Loading

b1ink0 Nov 28, 2024

b1ink0 Nov 28, 2024

westonruter Nov 28, 2024

westonruter Nov 28, 2024

b1ink0 Nov 29, 2024

westonruter Nov 29, 2024

westonruter Nov 29, 2024

westonruter Nov 27, 2024

westonruter Nov 27, 2024

westonruter commented Nov 27, 2024

westonruter Nov 28, 2024

b1ink0 Nov 29, 2024

westonruter Nov 30, 2024

westonruter Nov 30, 2024

westonruter Nov 30, 2024

westonruter Nov 30, 2024

westonruter Nov 30, 2024

b1ink0 Dec 2, 2024

westonruter Nov 30, 2024

b1ink0 Dec 2, 2024

westonruter Nov 30, 2024

westonruter Nov 30, 2024

westonruter Nov 30, 2024

westonruter commented Dec 2, 2024

westonruter Dec 4, 2024 •

edited

Loading

westonruter left a comment


		$this->add_preload_link_for_picture( $context, $img_xpath, $collected_sources[0] );

		return true;

Add preload links LCP picture elements #1707

Add preload links LCP picture elements #1707

Conversation

b1ink0 commented Nov 27, 2024 • edited by westonruter Loading

github-actions bot commented Nov 27, 2024

westonruter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Dec 2, 2024

westonruter Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

westonruter left a comment

Choose a reason for hiding this comment

b1ink0 commented Nov 27, 2024 •

edited by westonruter

Loading

westonruter Nov 27, 2024 •

edited

Loading

westonruter Dec 4, 2024 •

edited

Loading