Add fetchpriority=low to occluded initial-viewport images #1482

westonruter · 2024-08-19T05:05:17Z

This is a follow-up PR (sub-PR) to #1373 which is branched off of add/embed-optimizer-min-height-reservation.

When an image is in the initial viewport (i.e. its boundingClientRect.top is less than window.innerHeight) and yet its intersectionRatio is 0.0, at present such images are getting loading=lazy incorrectly. Such images are likely part of subsequent carousel slides and getting loading=lazy could result in them not being loaded by the time the carousel slide is displayed. Acccording to the Web.dev article on fetch priority, such images should actually get fetchpriority=low. This is what is implemented by this PR. In particular, see #1309 (comment) where this PR implements these points:

If an element has an intersectionRatio of zero and yet its boundingClientRect.top is less than the viewport height, then it should also get fetchpriority=low.

Alternatively, if an element has an intersectionRatio of zero and its boundingClientRect.top is greater than the viewport height (or if all of the values in boundingClientRect are zero, indicating a display:none element), it should get loading=lazy.

If an element has an intersectionRatio of zero and its boundingClientRect indicates that it is in the negative area of the viewport to the top, left or right, then it should get fetchpriority=low since it may be part of an off-screen submenu that gets displayed.

It does not implement the following point, which will come in a subsequent PR (to address #1587):

If an element has a non-zero intersectionRatio and the element has a computed style of visibility:hidden or opacity:0, then it should get fetchpriority=low.

Two new methods are introduced which determine whether an element or elements are positioned in the initial viewport (that is, they are not below the initial viewport):

OD_URL_Metric_Group_Collection::get_all_elements_positioned_in_any_initial_viewport()
OD_URL_Metric_Group_Collection::is_element_positioned_in_any_initial_viewport()

Carousel Example

Given the Ultimate Blocks plugin's Carousel block with markup as follows, with simply 3 images added as carousel slides:

Block Markup

<!-- wp:ub/image-slider {
  "blockID": "1427237e-b7fb-48da-9ca5-8e8021466dc0",
  "pics": [
    {
      "sizes": {
        "thumbnail": {
          "height": 150,
          "width": 150,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison1-150x150.jpg",
          "orientation": "landscape"
        },
        "medium": {
          "height": 196,
          "width": 300,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison1-300x196.jpg",
          "orientation": "landscape"
        },
        "large": {
          "height": 668,
          "width": 1024,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison1-1024x668.jpg",
          "orientation": "landscape"
        },
        "full": {
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison1-scaled.jpg",
          "height": 1670,
          "width": 2560,
          "orientation": "landscape"
        }
      },
      "mime": "image/jpeg",
      "type": "image",
      "subtype": "jpeg",
      "id": 13,
      "url": "http://localhost:8888/wp-content/uploads/2024/06/bison1-scaled.jpg",
      "alt": "",
      "link": "http://localhost:8888/2024/06/10/bison/bison1/",
      "caption": ""
    },
    {
      "sizes": {
        "thumbnail": {
          "height": 150,
          "width": 150,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison2-150x150.jpg",
          "orientation": "landscape"
        },
        "medium": {
          "height": 197,
          "width": 300,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison2-300x197.jpg",
          "orientation": "landscape"
        },
        "large": {
          "height": 673,
          "width": 1024,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison2-1024x673.jpg",
          "orientation": "landscape"
        },
        "full": {
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison2-scaled.jpg",
          "height": 1684,
          "width": 2560,
          "orientation": "landscape"
        }
      },
      "mime": "image/jpeg",
      "type": "image",
      "subtype": "jpeg",
      "id": 12,
      "url": "http://localhost:8888/wp-content/uploads/2024/06/bison2-scaled.jpg",
      "alt": "",
      "link": "http://localhost:8888/2024/06/10/bison/bison2/",
      "caption": ""
    },
    {
      "sizes": {
        "thumbnail": {
          "height": 150,
          "width": 150,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison3-150x150.jpg",
          "orientation": "landscape"
        },
        "medium": {
          "height": 200,
          "width": 300,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison3-300x200.jpg",
          "orientation": "landscape"
        },
        "large": {
          "height": 683,
          "width": 1024,
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison3-1024x683.jpg",
          "orientation": "landscape"
        },
        "full": {
          "url": "http://localhost:8888/wp-content/uploads/2024/06/bison3-scaled.jpg",
          "height": 1708,
          "width": 2560,
          "orientation": "landscape"
        }
      },
      "mime": "image/jpeg",
      "type": "image",
      "subtype": "jpeg",
      "id": 11,
      "url": "http://localhost:8888/wp-content/uploads/2024/06/bison3-scaled.jpg",
      "alt": "",
      "link": "http://localhost:8888/2024/06/10/bison/bison3/",
      "caption": ""
    }
  ],
  "descriptions": [
    {
      "id": 13,
      "text": "",
      "link": ""
    },
    {
      "id": 12,
      "text": "",
      "link": ""
    },
    {
      "id": 11,
      "text": "",
      "link": ""
    }
  ],
  "sliderHeight": 432,
  "paginationType": "bullets"
} /-->

With this Carousel being the only block in a post where the initial slide's image is the LCP element:

The Prettier-formatted rendered markup is as follows without the changes in this PR:

<div
  class="ub_image_slider swiper-container wp-block-ub-image-slider"
  id="ub_image_slider_2fbf05fe-0c97-41d7-84d1-891c40278575"
  data-swiper-data='{"speed":300,"spaceBetween":20,"slidesPerView":1,"loop":true,"pagination":{"el": ".swiper-pagination" , "type": "bullets", "clickable":true}
            ,"navigation": {"nextEl": ".swiper-button-next", "prevEl": ".swiper-button-prev"}, "keyboard": { "enabled": true },
            "effect": "slide","simulateTouch":false}'
>
  <div class="swiper-wrapper">
    <figure class="swiper-slide">
      <img
        decoding="async"
        src="http://localhost:8888/wp-content/uploads/2024/06/bison1-scaled.jpg"
        alt=""
      />
      <figcaption class="ub_image_slider_image_caption"></figcaption>
    </figure>
    <figure class="swiper-slide">
      <img
        decoding="async"
        src="http://localhost:8888/wp-content/uploads/2024/06/bison2-scaled.jpg"
        alt=""
      />
      <figcaption class="ub_image_slider_image_caption"></figcaption>
    </figure>
    <figure class="swiper-slide">
      <img
        decoding="async"
        src="http://localhost:8888/wp-content/uploads/2024/06/bison3-scaled.jpg"
        alt=""
      />
      <figcaption class="ub_image_slider_image_caption"></figcaption>
    </figure>
  </div>
  <div class="swiper-pagination"></div>
  <div class="swiper-button-prev"></div>
  <div class="swiper-button-next"></div>
</div>

Notice in particular that Ultimate Blocks does not add width and height attributes to the images in the carousel, so WordPress core's server-side heuristics does not add fetchpriority=high to the initial image. Additionally, note the priorities of these images in the DevTools network log:

All three images have an initial priority of medium and the third carousel image is then elevated by Chrome to a high priority even though it is not even displayed (and it is not the LCP element).

Compare this with when the changes in this PR are applied and URL Metrics have been gathered. In the network log, now the LCP image element now has an initial and final priority of high and the subsequent slides both have initial/final priorities of low.

The Prettier-formatted rendered markup is as follows:

<div
  class="ub_image_slider swiper-container wp-block-ub-image-slider"
  id="ub_image_slider_2fbf05fe-0c97-41d7-84d1-891c40278575"
  data-swiper-data='{"speed":300,"spaceBetween":20,"slidesPerView":1,"loop":true,"pagination":{"el": ".swiper-pagination" , "type": "bullets", "clickable":true}
            ,"navigation": {"nextEl": ".swiper-button-next", "prevEl": ".swiper-button-prev"}, "keyboard": { "enabled": true },
            "effect": "slide","simulateTouch":false}'
>
  <div class="swiper-wrapper">
    <figure class="swiper-slide">
      <img
        data-od-added-fetchpriority
        fetchpriority="high"
        decoding="async"
        src="http://localhost:8888/wp-content/uploads/2024/06/bison1-scaled.jpg"
        alt=""
      />
      <figcaption class="ub_image_slider_image_caption"></figcaption>
    </figure>
    <figure class="swiper-slide">
      <img
        data-od-added-fetchpriority
        fetchpriority="low"
        decoding="async"
        src="http://localhost:8888/wp-content/uploads/2024/06/bison2-scaled.jpg"
        alt=""
      />
      <figcaption class="ub_image_slider_image_caption"></figcaption>
    </figure>
    <figure class="swiper-slide">
      <img
        data-od-added-fetchpriority
        fetchpriority="low"
        decoding="async"
        src="http://localhost:8888/wp-content/uploads/2024/06/bison3-scaled.jpg"
        alt=""
      />
      <figcaption class="ub_image_slider_image_caption"></figcaption>
    </figure>
  </div>
  <div class="swiper-pagination"></div>
  <div class="swiper-button-prev"></div>
  <div class="swiper-button-next"></div>
</div>

Note the inclusion of fetchpriority=high on the initial carousel slide img and fetchpriority=low on the subsequent ones.

…ent_positioned_in_any_initial_viewport() methods to OD_URL_Metrics_Group_Collection

… add/fetchpriority-low

westonruter · 2024-10-14T19:05:09Z

Still can be implemented here:

If an element has an intersectionRatio of zero and its boundingClientRect indicates that it is in the negative area of the viewport to the top, left or right, then it should get fetchpriority=low since it may be part of an off-screen submenu that gets displayed.

westonruter · 2024-10-14T19:07:35Z

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

@@ -90,17 +90,43 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
 		if ( is_null( $element_max_intersection_ratio ) ) {
 			$processor->set_meta_attribute( 'unknown-tag', true ); // Mostly useful for debugging why an IMG isn't optimized.
 		} else {
-			// Otherwise, make sure visible elements omit the loading attribute, and hidden elements include loading=lazy.
+			// TODO: Take into account whether the element has the computed style of visibility:hidden, in such case it should also get fetchpriority=low.


To be addressed later as part of #1587

github-actions · 2024-10-14T20:06:32Z

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: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: devansh016 <[email protected]>

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

westonruter · 2024-10-14T20:14:36Z

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

-			if ( $is_visible && 'lazy' === $loading ) {
-				$processor->remove_attribute( 'loading' );
-			} elseif ( ! $is_visible && 'lazy' !== $loading ) {
+			if ( true === $context->url_metric_group_collection->is_element_positioned_in_any_initial_viewport( $xpath ) ) {


No need to do a method_exists() check here because Image Prioritizer here now requires Optimization Detective version 0.7.0+, which is the version this method is introduced in.

felixarntz

@westonruter This looks great to me. Only a few minor questions.

felixarntz · 2024-10-15T17:44:04Z

plugins/optimization-detective/class-od-url-metric.php

-						/* translators: 1: current aspect ratio, 2: minimum aspect ratio, 3: maximum aspect ratio */
-						__( 'Viewport aspect ratio (%1$s) is not in the accepted range of %2$s to %3$s.', 'optimization-detective' ),
+						/* translators: 1: current aspect ratio, 2: minimum aspect ratio, 3: maximum aspect ratio, 4: viewport width, 5: viewport height */
+						__( 'Viewport aspect ratio (%1$s) is not in the accepted range of %2$s to %3$s. Viewport dimensions: %4$sx%5$s', 'optimization-detective' ),


Why is it important to include the dimensions in the message here?

Related: Maybe use a single placeholder in the string for WIDTHxHEIGHT? That portion doesn't need to be translatable individually.

It's not important, but it explains how the aspect ratio was computed.

I've combined the width and height in 38f2e70

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

felixarntz · 2024-10-15T17:49:52Z

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

+					// Also prevent the image from being lazy-loaded (or eager-loaded) since it may be revealed at any
+					// time without the browser having any signal (e.g. user scrolling toward it) to start downloading.
+					$processor->remove_attribute( 'loading' );


Not sure this is needed? If I remember correctly, browser's lazy-loading implementations support horizontally offset images too, so shouldn't those still be lazy-loaded where applicable?

Worth noting that we may need to differentiate between "this is hidden by being offset outside the viewport" vs "this is simply hidden but technically would be in the viewport if shown".

In this case, fetchpriority=low is what is recommended as opposed to loading=lazy as per Optimize resource loading with the Fetch Priority API:

And per Optimize Largest Contentful Paint:

In the case of a carousel I believe you are right now that loading=lazy may be OK, since the user can scroll as a signal to the browser that the image should start getting downloaded before displayed. However, the other case I have in mind are images which are hidden as being part of a mega menu, in which case they may be shown immediately in response to a user hovering over a nav menu item. In this scenario, loading=lazy would mean the image is not loaded when when the element is displayed. So I think fetchpriority=low is safer. Also, a carousel may not involve scrolling at all. The slides may be programmatically changed, so again there wouldn't be a user signal to start loading the image before the element is displayed.

… add/fetchpriority-low

felixarntz

Thanks @westonruter!

westonruter added 4 commits August 18, 2024 18:12

Avoid setting meta attribute when value unchanged

7864449

Allow viewport_height to be specified in sample URL metric

7b01495

Add get_all_elements_positioned_in_any_initial_viewport() and is_elem…

9c8799b

…ent_positioned_in_any_initial_viewport() methods to OD_URL_Metrics_Group_Collection

Add fetchpriority=low to occluded elements in initial viewport

c573175

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

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

Update auto-sizes tests to specify out-of-viewport rects

67c6557

westonruter mentioned this pull request Aug 19, 2024

Occluded initial-viewport images should get fetchpriority=low #1309

Closed

macka61 assigned westonruter Sep 30, 2024

Base automatically changed from add/embed-optimizer-min-height-reservation to trunk October 14, 2024 17:38

westonruter added 4 commits October 14, 2024 11:33

Merge branch 'trunk' of https://github.com/WordPress/performance into…

fcdc6eb

… add/fetchpriority-low

Include dimensions in exception error message for aspect-ratio

da3da34

Reuse get_all_denormalized_elements()

785c131

Improve usage of is_element_positioned_in_any_initial_viewport

0e61767

westonruter mentioned this pull request Oct 14, 2024

Hidden (yet non-occluded) images should get fetchpriority=low #1587

Open

westonruter commented Oct 14, 2024

View reviewed changes

Clarify purpose of is_element_positioned_in_any_initial_viewport

882df0c

westonruter marked this pull request as ready for review October 14, 2024 20:06

westonruter requested a review from felixarntz as a code owner October 14, 2024 20:06

westonruter commented Oct 14, 2024

View reviewed changes

westonruter requested a review from adamsilverstein October 14, 2024 20:15

felixarntz reviewed Oct 15, 2024

View reviewed changes

Combine width and height into a single translation string placeholder

38f2e70

westonruter requested a review from felixarntz October 15, 2024 19:53

westonruter added 2 commits October 15, 2024 12:57

Merge branch 'trunk' of https://github.com/WordPress/performance into…

853f8e4

… add/fetchpriority-low

Merge branch 'trunk' of https://github.com/WordPress/performance into…

05293de

… add/fetchpriority-low

westonruter added 2 commits October 18, 2024 07:55

Merge branch 'trunk' of https://github.com/WordPress/performance into…

68dff96

… add/fetchpriority-low

Clarify purpose of setting class name in test

812a20f

felixarntz approved these changes Oct 18, 2024

View reviewed changes

westonruter merged commit 58fd403 into trunk Oct 18, 2024
16 checks passed

westonruter deleted the add/fetchpriority-low branch October 18, 2024 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fetchpriority=low to occluded initial-viewport images #1482

Add fetchpriority=low to occluded initial-viewport images #1482

westonruter commented Aug 19, 2024 •

edited

Loading

westonruter commented Oct 14, 2024

westonruter Oct 14, 2024

github-actions bot commented Oct 14, 2024 •

edited

Loading

westonruter Oct 14, 2024

felixarntz left a comment

felixarntz Oct 15, 2024

westonruter Oct 15, 2024

felixarntz Oct 15, 2024

westonruter Oct 15, 2024

felixarntz left a comment

Add fetchpriority=low to occluded initial-viewport images #1482

Add fetchpriority=low to occluded initial-viewport images #1482

Conversation

westonruter commented Aug 19, 2024 • edited Loading

Carousel Example

westonruter commented Oct 14, 2024

westonruter Oct 14, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 14, 2024 • edited Loading

westonruter Oct 14, 2024

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

felixarntz Oct 15, 2024

Choose a reason for hiding this comment

westonruter Oct 15, 2024

Choose a reason for hiding this comment

felixarntz Oct 15, 2024

Choose a reason for hiding this comment

westonruter Oct 15, 2024

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

westonruter commented Aug 19, 2024 •

edited

Loading

github-actions bot commented Oct 14, 2024 •

edited

Loading