-
Notifications
You must be signed in to change notification settings - Fork 101
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
base: trunk
Are you sure you want to change the base?
Conversation
@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 |
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! |
…n validate background-image URL
c872c1b
to
f2959cd
Compare
// This needs to be captured early in case the user later resizes the window. | ||
const viewport = { | ||
width: win.innerWidth, | ||
height: win.innerHeight, | ||
}; | ||
|
There was a problem hiding this comment.
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.
plugins/image-prioritizer/hooks.php
Outdated
|
||
/** | ||
* 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; | ||
} |
There was a problem hiding this comment.
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 >; |
There was a problem hiding this comment.
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
.
plugins/image-prioritizer/helper.php
Outdated
|
||
/** | ||
* 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; | ||
} |
There was a problem hiding this comment.
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
.
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There seems to be a problem with the TTFB reporting in npm run research -- benchmark-server-timing --file=preload-bgimage-urls.txt --number 100 -o csv And I got:
This is what I was expecting for the impact of Optimization Detective to have on TTFB: ~3 milliseconds. I just re-ran 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):
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,
|
… 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
$r = wp_safe_remote_head( | ||
$data['url'], |
There was a problem hiding this comment.
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:
performance/plugins/optimization-detective/detect.js
Lines 361 to 366 in bc81588
// 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()
.
There was a problem hiding this comment.
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:
- A filter callback could return
false
or a non-emptyWP_Error
instance to block the URL Metric from being stored at all. - 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.
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 calledlcpElementExternalBackgroundImage
which includes not only theurl
of the background image but also thetag
name,id
,class
for the LCP element that has the background image. When theImage_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 afetchpriority=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 returnfalse
or a non-emptyWP_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:
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: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:In contrast to when the image is preloaded, it is the initial resource loaded and has an initial high priority:
Here are the before/after metrics testing with
benchmark-web-vitals
(with GoogleChromeLabs/wpp-research#164) without any page caching: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:I then ran 100 iterations before/after on desktop and mobile:
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:
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
):The element being targeted is:
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):
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:Elementor is adding
fetchpriority=high
to thisIMG
even though it is not even displayed in any initial viewport: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 theIMG
that Elementor added it to. In addition, Image Prioritizer is addingloading=lazy
andsizes=auto
to all of these images since none of them appear in the initial viewport on desktop or mobile:Network log without the optimizations up until the LCP element's background image is loaded:
Compared with after the optimizations applied:
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 oflow
.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 afterIMG
. Since images account for the LCP type 82% desktop and and 72% on mobile, it's likely that most of theDIV
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.Todo