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

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

Open
wants to merge 17 commits into
base: trunk
Choose a base branch
from

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Nov 22, 2024

Fixes #1584

This captures the URL for the background image of the LCP element which is defined in a CSS stylesheet and not in an inline style attribute. A new client-side extension module from Image Prioritizer is introduced to implement this. A new root property is added to the URL Schema called lcpElementExternalBackgroundImage which includes not only the url of the background image but also the tag name, id, class for the LCP element that has the background image. When the Image_Prioritizer_Background_Image_Styled_Tag_Visitor iterates over tags in the document, it checks to see if there is a matching tag (and thus the original element with the background image is still present). If so, and if all URL Metrics in the group are in agreement on that being the LCP element with that same background image, then the URL for the background image is added as a fetchpriority=high preload link for that viewport group. The requirement that all gathered URL Metrics be in agreement helps ensure that if a randomized background image is used (e.g. as can be done in core's header image), then no preloading will be done (as it is likely the wrong image will be preloaded).

Accepting the LCP background image URL from the untrusted client is dangerous as it could lead to abuse. The JSON Schema to enforce the validity of the URL Schema with such a background image URL is not robust enough to ensure that the URL is safe and that it actually references a valid image file. Therefore, a new od_store_url_metric_validity filter is introduced in the REST API endpoint. After the user-supplied URL Metric has been validated according to the JSON Schema, the instance is passed as the second argument to this filter. Callbacks can then return false or a non-empty WP_Error to block the URL Metric from being stored, which is implemented here to ensure that the supplied background image is valid.

Twenty Thirteen Example

Given the Twenty Thirteen theme which has a CSS background image in the header and some images in the post content, on desktop the header is the LCP element whereas on mobile the first image is the LCP element:

Desktop Mobile
2013-desktop 2013-mobile

With the changes in this PR, the CSS background image gets a fetchpriority=high preload link for desktop, whereas the first image in the content gets a preload link for mobile:

<link
  data-od-added-tag
  rel="preload"
  fetchpriority="high"
  as="image"
  href="http://localhost:8888/wp-content/themes/twentythirteen/images/headers/circle.png"
  media="screen and (min-width: 783px)"
>
<link
  data-od-added-tag
  rel="preload"
  fetchpriority="high"
  as="image"
  href="http://localhost:8888/wp-content/uploads/2024/06/bison2-1024x673-jpg.webp"
  imagesrcset="http://localhost:8888/wp-content/uploads/2024/06/bison2-1024x673-jpg.webp 1024w, http://localhost:8888/wp-content/uploads/2024/06/bison2-300x197-jpg.webp 300w, http://localhost:8888/wp-content/uploads/2024/06/bison2-768x505-jpg.webp 768w, http://localhost:8888/wp-content/uploads/2024/06/bison2-1536x1010.jpg 1536w, http://localhost:8888/wp-content/uploads/2024/06/bison2-2048x1347.jpg 2048w, http://localhost:8888/wp-content/uploads/2024/06/bison2-1568x1031.jpg 1568w"
  imagesizes="(max-width: 1024px) 100vw, 1024px"
  media="screen and (max-width: 480px)"
>

The impact of preloading the background image on mobile is reflected in the network log, where without the optimization the circle.png image is loaded with an initial low priority long after the other assets on the page have started loading:

image

In contrast to when the image is preloaded, it is the initial resource loaded and has an initial high priority:

image

Here are the before/after metrics testing with benchmark-web-vitals (with GoogleChromeLabs/wpp-research#164) without any page caching:

  Desktop Before Desktop After Mobile Before Mobile After
FCP (median) 121.55 143.9 113.25 113.6
LCP (median) 151.95 143.9 113.25 113.6
TTFB (median) 30.1 32.55 32 33.75
LCP-TTFB (median) 122.1 111.35 80.6 79.9
Test setup

I created a site in Local and set the theme to Twenty Thirteen. I then created a post which had a 3-columns block with the center column being wider. I put an image in each column, so the middle image was larger. I visited the URL in a desktop viewport and mobile viewport multiple times to ensure the URL Metrics were fully populated. I then created a text file called preload-bgimage-urls.txt which contained:

http://localhost:10053/bison-columns/?optimization_detective_disabled
http://localhost:10053/bison-columns/

I then ran 100 iterations before/after on desktop and mobile:

npm run research benchmark-web-vitals -- --file=preload-bgimage-urls.txt -n 100 -w "1920x1080" -o csv
npm run research benchmark-web-vitals -- --file=preload-bgimage-urls.txt -n 100 -w "360x800" -o csv

So on desktop, the LCP-TTFB is improved by 8.8%. On mobile, the LCP-TTFB is improved by a lesser degree by 0.87% because WordPress was already adding fetchpriority=high to the first image, so the marginal improvement is gained by the preload link.

Elementor Example

As another test, I created a site with Elementor and the Hello Elementor theme. I used the Ceramic Studio "website kit" to create a page, and I added a mobile-specific image to the hero's second container:

Desktop Mobile
elementor-desktop elementor-mobile

Elementor implements the images here as background images pulled from an external CSS file (http://localhost:10053/wp-content/uploads/elementor/css/post-67.css?ver=1732401999):

.elementor-67 .elementor-element.elementor-element-35a28f8:not(.elementor-motion-effects-element-type-background), .elementor-67 .elementor-element.elementor-element-35a28f8 > .elementor-motion-effects-container > .elementor-motion-effects-layer {
    background-image: url("http://localhost:10053/wp-content/uploads/2024/11/HomePage-Hero.jpg");
    background-position: center center;
    background-repeat: no-repeat;
    background-size: cover;
}
/*...*/
@media(max-width: 767px) {
    /*...*/
    .elementor-67 .elementor-element.elementor-element-35a28f8:not(.elementor-motion-effects-element-type-background), .elementor-67 .elementor-element.elementor-element-35a28f8 > .elementor-motion-effects-container > .elementor-motion-effects-layer {
        background-image: url("http://localhost:10053/wp-content/uploads/2024/11/WorkshopPage-WorkGallery-img_3.jpg");
    }
    /*...*/
}

The element being targeted is:

<div
  class="elementor-element elementor-element-35a28f8 e-con-full e-flex e-con e-child"
  data-id="35a28f8"
  data-element_type="container"
  data-settings='{"background_background":"classic"}'
></div>

Running the same tests as before with 100 iterations on both desktop and mobile, before and after the optimizations, yields the following results (again where mobile is 360x800 and desktop is 1920x1080):

  Desktop Before Desktop After Mobile Before Mobile After
FCP (median) 120.45 122.4 121.3 112.8
LCP (median) 158.85 122.75 145.65 113.25
TTFB (median) 54.5 39.1 55 39.4
LCP-TTFB (median) 103.35 84.55 91.55 71.8

The LCP-TTFB on desktop is improved by 18.19% and on mobile it is improved by 21.57%! 🎉 (What is surprising to me as well is that TTFB is reduced when the optimizations are applied, which doesn't make any sense since the HTML Tag Processor spends cycles doing work.)

It's important to note that this page doesn't just have CSS background images. Further down the page outside the viewport of both desktop and mobile, there are three IMG tags in another section:

image

Elementor is adding fetchpriority=high to this IMG even though it is not even displayed in any initial viewport:

<figure class="elementor-image-box-img">
  <img
    fetchpriority="high"
    decoding="async"
    width="327"
    height="293"
    src="http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_1.jpg"
    class="elementor-animation-sink attachment-full size-full wp-image-70"
    alt=""
    srcset="
      http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_1.jpg         327w,
      http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_1-300x269.jpg 300w
    "
    sizes="(max-width: 327px) 100vw, 327px"
  >
</figure>

The Elementor code responsible is the maybe_add_fetchpriority_high_attr() method which appears to be heavily inspired by what WordPress core does.

Here's a diff of the page (with Prettier formatting). Note how Image Prioritizer is adding responsive preload links for the two different background images while at the same time removing fetchpriority=high from the IMG that Elementor added it to. In addition, Image Prioritizer is adding loading=lazy and sizes=auto to all of these images since none of them appear in the initial viewport on desktop or mobile:

--- /tmp/disabled.html	2024-11-23 21:33:35.088604290 -0800
+++ /tmp/enabled.html	2024-11-23 21:33:42.844604516 -0800
@@ -712,6 +712,22 @@
       }
     </style>
     <meta name="generator" content="image-prioritizer 0.2.0" />
+    <link
+      data-od-added-tag
+      rel="preload"
+      fetchpriority="high"
+      as="image"
+      href="http://localhost:10053/wp-content/uploads/2024/11/HomePage-Hero.jpg"
+      media="screen and (min-width: 783px)"
+    />
+    <link
+      data-od-added-tag
+      rel="preload"
+      fetchpriority="high"
+      as="image"
+      href="http://localhost:10053/wp-content/uploads/2024/11/WorkshopPage-WorkGallery-img_3.jpg"
+      media="screen and (max-width: 480px)"
+    />
   </head>
   <body
     class="home page-template page-template-elementor_header_footer page page-id-67 elementor-default elementor-template-full-width elementor-kit-23 elementor-page elementor-page-67"
@@ -970,7 +986,11 @@
                   <div class="elementor-image-box-wrapper">
                     <figure class="elementor-image-box-img">
                       <img
-                        fetchpriority="high"
+                        data-od-added-loading
+                        data-od-removed-fetchpriority="high"
+                        data-od-replaced-sizes="(max-width: 327px) 100vw, 327px"
+                        data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[3][self::DIV]/*[2][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]"
+                        loading="lazy"
                         decoding="async"
                         width="327"
                         height="293"
@@ -981,7 +1001,7 @@
                           http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_1.jpg         327w,
                           http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_1-300x269.jpg 300w
                         "
-                        sizes="(max-width: 327px) 100vw, 327px"
+                        sizes="auto, (max-width: 327px) 100vw, 327px"
                       />
                     </figure>
                     <div class="elementor-image-box-content">
@@ -1032,6 +1052,10 @@
                   <div class="elementor-image-box-wrapper">
                     <figure class="elementor-image-box-img">
                       <img
+                        data-od-added-loading
+                        data-od-replaced-sizes="(max-width: 327px) 100vw, 327px"
+                        data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[3][self::DIV]/*[2][self::DIV]/*[2][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]"
+                        loading="lazy"
                         decoding="async"
                         width="327"
                         height="293"
@@ -1042,7 +1066,7 @@
                           http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_2.jpg         327w,
                           http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_2-300x269.jpg 300w
                         "
-                        sizes="(max-width: 327px) 100vw, 327px"
+                        sizes="auto, (max-width: 327px) 100vw, 327px"
                       />
                     </figure>
                     <div class="elementor-image-box-content">
@@ -1093,6 +1117,10 @@
                   <div class="elementor-image-box-wrapper">
                     <figure class="elementor-image-box-img">
                       <img
+                        data-od-added-loading
+                        data-od-replaced-sizes="(max-width: 327px) 100vw, 327px"
+                        data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[3][self::DIV]/*[2][self::DIV]/*[2][self::DIV]/*[3][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]"
+                        loading="lazy"
                         decoding="async"
                         width="327"
                         height="293"
@@ -1103,7 +1131,7 @@
                           http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_3.jpg         327w,
                           http://localhost:10053/wp-content/uploads/2024/11/HomePage-Offers_3-300x269.jpg 300w
                         "
-                        sizes="(max-width: 327px) 100vw, 327px"
+                        sizes="auto, (max-width: 327px) 100vw, 327px"
                       />
                     </figure>
                     <div class="elementor-image-box-content">
@@ -1388,6 +1416,9 @@
                 >
                   <figure class="swiper-slide-inner">
                     <img
+                      data-od-added-loading
+                      data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[5][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]"
+                      loading="lazy"
                       decoding="async"
                       class="swiper-slide-image"
                       src="http://localhost:10053/wp-content/uploads/2024/11/HomePage-WorkGallery-img_1.jpg"
@@ -1403,6 +1434,9 @@
                 >
                   <figure class="swiper-slide-inner">
                     <img
+                      data-od-added-loading
+                      data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[5][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[2][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]"
+                      loading="lazy"
                       decoding="async"
                       class="swiper-slide-image"
                       src="http://localhost:10053/wp-content/uploads/2024/11/HomePage-WorkGallery-img_2.jpg"
@@ -1418,6 +1452,9 @@
                 >
                   <figure class="swiper-slide-inner">
                     <img
+                      data-od-added-loading
+                      data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[5][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[3][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]"
+                      loading="lazy"
                       decoding="async"
                       class="swiper-slide-image"
                       src="http://localhost:10053/wp-content/uploads/2024/11/HomePage-WorkGallery-img_3.jpg"
@@ -1433,6 +1470,9 @@
                 >
                   <figure class="swiper-slide-inner">
                     <img
+                      data-od-added-loading
+                      data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[5][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[4][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]"
+                      loading="lazy"
                       decoding="async"
                       class="swiper-slide-image"
                       src="http://localhost:10053/wp-content/uploads/2024/11/HomePage-WorkGallery-img_4.jpg"
@@ -1722,5 +1762,32 @@
       src="http://localhost:10053/wp-content/plugins/elementor/assets/js/frontend.min.js?ver=3.25.9"
       id="elementor-frontend-js"
     ></script>
+
+    <script type="module">
+      import detect from "http:\/\/localhost:10053\/wp-content\/plugins\/optimization-detective\/detect.min.js?ver=0.9.0-alpha";
+      detect({
+        minViewportAspectRatio: 0.40000000000000002,
+        maxViewportAspectRatio: 2.5,
+        isDebug: false,
+        extensionModuleUrls: [
+          "http:\/\/localhost:10053\/wp-content\/plugins\/image-prioritizer\/detect.min.js?ver=0.2.0",
+        ],
+        restApiEndpoint:
+          "http:\/\/localhost:10053\/wp-json\/optimization-detective\/v1\/url-metrics:store",
+        currentUrl: "http:\/\/localhost:10053\/",
+        urlMetricSlug: "d751713988987e9331980363e24189ce",
+        cachePurgePostId: 67,
+        urlMetricHMAC: "35086facb74f518514588b7e83751043",
+        urlMetricGroupStatuses: [
+          { minimumViewportWidth: 0, complete: true },
+          { minimumViewportWidth: 481, complete: false },
+          { minimumViewportWidth: 601, complete: false },
+          { minimumViewportWidth: 783, complete: true },
+        ],
+        storageLockTTL: 60,
+        webVitalsLibrarySrc:
+          "http:\/\/localhost:10053\/wp-content\/plugins\/optimization-detective\/build\/web-vitals.js?ver=4.2.4",
+      });
+    </script>
   </body>
 </html>

Network log without the optimizations up until the LCP element's background image is loaded:

image

Compared with after the optimizations applied:

image

Note how the LCP element's background image is now loaded as early as possible with initial high priority, whereas without the optimizations the image is loaded very late and has an initial priority of low.

Takeaway

This represents an critical performance advancement for optimizing LCP in WordPress because on the web a DIV is the second most common LCP element after IMG. Since images account for the LCP type 82% desktop and and 72% on mobile, it's likely that most of the DIV LCP elements represent background images. Additionally, page builders like Elementor and Divi leverage external background images extensively, including separate background images for desktop and mobile.

Top LCP element types Top LCP content types
Top LCP element types Top LCP content types

Todo

  • Add additional validation constraints on the new root property (e.g. require same-origin).
  • Add tests.

@westonruter 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 Nov 22, 2024
@westonruter westonruter added this to the image-prioritizer n.e.x.t milestone Nov 22, 2024
@westonruter
Copy link
Member Author

westonruter commented Nov 23, 2024

@felixarntz This is getting close! I'm excited about this one since it unlocks a very common use case which we've seen especially among page builders. This unlocks potential optimization of the DIV (assuming it has a background image) which is the second most common LCP element.

@westonruter
Copy link
Member Author

I've just updated the description with findings on how this impacts Elementor. In short, I'm seeing a ~20% improvement in LCP on both desktop and mobile!

Comment on lines +130 to +135
// This needs to be captured early in case the user later resizes the window.
const viewport = {
width: win.innerWidth,
height: win.innerHeight,
};

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this here to capture the initial width/height when the page loads. Later when about to submit the metrics for storage, it checks to see if the viewport width or height has changed, and then it aborts. This is important because if the viewport window size changes during the life of the page, all bets are off as to what is the expected LCP element.

Comment on lines 14 to 33

/**
* Gets the script to lazy-load videos.
*
* Load a video and its poster image when it approaches the viewport using an IntersectionObserver.
*
* Handles 'autoplay' and 'preload' attributes accordingly.
*
* @since 0.2.0
*/
function image_prioritizer_get_lazy_load_script(): string {
$script = file_get_contents( __DIR__ . sprintf( '/lazy-load%s.js', wp_scripts_get_suffix() ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.

if ( false === $script ) {
return '';
}

return $script;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This function was just moved to helper.php.

};

export type InitializeCallback = ( args: InitializeArgs ) => void;
export type InitializeCallback = ( args: InitializeArgs ) => Promise< void >;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now consistent with the FinalizeCallback.

Comment on lines 79 to 97

/**
* Gets the script to lazy-load videos.
*
* Load a video and its poster image when it approaches the viewport using an IntersectionObserver.
*
* Handles 'autoplay' and 'preload' attributes accordingly.
*
* @since 0.2.0
*/
function image_prioritizer_get_lazy_load_script(): string {
$script = file_get_contents( __DIR__ . sprintf( '/lazy-load%s.js', wp_scripts_get_suffix() ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.

if ( false === $script ) {
return '';
}

return $script;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This function was just moved from hooks.php.

@westonruter westonruter marked this pull request as ready for review November 25, 2024 17:47
Copy link

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]>

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

@westonruter
Copy link
Member Author

The LCP-TTFB on desktop is improved by 18.19% and on mobile it is improved by 21.57%! 🎉 (What is surprising to me as well is that TTFB is reduced when the optimizations are applied, which doesn't make any sense since the HTML Tag Processor spends cycles doing work.)

There seems to be a problem with the TTFB reporting in benchmark-web-vitals. I ran benchmark-server-timing (with GoogleChromeLabs/wpp-research#165):

npm run research -- benchmark-server-timing --file=preload-bgimage-urls.txt --number 100 -o csv

And I got:

Metric OD Disabled OD Enabled
Response Time (median) 29.13 31.53
wp-before-template (median) 12.31 12
wp-template (median) 15.22 18.27
wp-total (median) 27.99 30.41
wp-optimization-detective (median)   3.15

This is what I was expecting for the impact of Optimization Detective to have on TTFB: ~3 milliseconds.

I just re-ran benchmark-web-vitals:

npm run research benchmark-web-vitals -- --file=preload-bgimage-urls.txt -n 100 -o csv

And now I'm getting a more expected result where TTFB is slower when OD is enabled (without page caching):

Metric OD Disabled OD Enabled
FCP (median) 110 124.95
LCP (median) 141.3 124.95
TTFB (median) 36.1 42.05
LCP-TTFB (median) 103.2 82.05

The LCP-TTFB remains at a ~20% improvement, and here the absolute LCP including TTFB is a 11.57% improvement.

Note that in these commands, preload-bgimage-urls.txt contains:

http://localhost:10053/?optimization_detective_disabled=1
http://localhost:10053/

@westonruter westonruter changed the title Preload image URLs for elements with external background images Preload image URLs for LCP elements with external background images Nov 25, 2024
… add/external-bg-preload

* 'trunk' of https://github.com/WordPress/performance:
  Explain why perflab_query_plugin_info() isn't used in perflab_install_and_activate_plugin()
  Explicitly request download_link and requires_plugins fields
  Bump wp-phpunit/wp-phpunit from 6.7.0 to 6.7.1
  Remove download_link from data explicitly requested in perflab_query_plugin_info()
  Bump typescript from 5.6.3 to 5.7.2
  Avoid transient cache for plugin installation to ensure latest version
  Update regex to handle version tags like `-rc` and `-beta` in plugin download links
  Remove plugin version number from download link
  Update other plugins to warn when minified asset is missing
  Eliminate local env check
  Change perflab_get_asset_url() to return path instead of URL
  Introduce perflab_get_asset_src()
  Add missing JS minification for new PL script
Comment on lines +175 to +176
$r = wp_safe_remote_head(
$data['url'],
Copy link
Member Author

@westonruter westonruter Nov 26, 2024

Choose a reason for hiding this comment

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

The wp_safe_remote_head() function passes the supplied URL through wp_http_validate_url(), but that doesn't necessarily mean the request is safe. I'm thinking there should perhaps be an allowlist of hosts that this is allowed to connect to. Maybe we query for the first image attachment and pass it into wp_get_attachment_image_url() to then extract the hostname from the image. This would be better than having to try to come up with a list of all image CDNs that are used by plugins.

The list of allowed hosts could also be sent to the frontend so as to avoid sending the invalid URL in the first place. See related todo:

// TODO: There should to be a way to pass additional args into the module. Perhaps extensionModuleUrls should be a mapping of URLs to args.
if ( extension.initialize instanceof Function ) {
const initializePromise = extension.initialize( {
isDebug,
webVitalsLibrarySrc,
} );

Then again, would be a shape to continuously reject all of the URL Metrics in the case that an unexpected host is being used for a given background image. In that case, perhaps we should allow OD_URL_Metric to be mutable, so in the case where an invalid host is provided this callback could instead do:

$url_metric->set( 'lcpElementExternalBackgroundImage', null );

or rather

$url_metric->unset( 'lcpElementExternalBackgroundImage' );

When mutating an instance of OD_URL_Metric it should re-trigger the validation so that the mutation would cause the JSON Schema to be made invalid that it would throw an exception.

Also, whenever mutation happens then it should call a new $this->group->clear_cache() method which empties out $group->result_cache and which would in turn call $collection->clear_cache().

Copy link
Member Author

Choose a reason for hiding this comment

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

So this od_store_url_metric_validity filter could be used in two ways:

  1. A filter callback could return false or a non-empty WP_Error instance to block the URL Metric from being stored at all.
  2. A filter callback could mutate the supplied OD_URL_Metric object to apply custom sanitizations or validation constraints which then would end up getting persisted in storage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image Prioritizer should be able to store the LCP (background) image URL for prioritization
1 participant