Preload image for LCP element with background-image#914
Conversation
westonruter
added
[Plugin] Optimization Detective
westonruter
force-pushed
the
add/ilo-background-image-optimization
branch
from
d54c6c2 to
0942fa5
Compare
westonruter
force-pushed
the
add/ilo-background-image-optimization
branch
from
fb86490 to
c6ff3c0
Compare
…om/WordPress/performance into add/ilo-background-image-optimization * 'feature/image-loading-optimization' of https://github.com/WordPress/performance: (177 commits) Add additional line breaks in phpdoc Add line break before dataProvider tag Add missing since tag Move ilo_can_optimize_response() to optimization.php Update ilo_verify_url_metrics_storage_nonce() to return bool Re-run composer update with PHP 8.1 Run composer update Fix PHPStan errors in tests Remove scheduler from globals since not yet used Fix or ignore eslint rules Remove modules/images/webp-uploads/fallback.js from ignorePatterns Run format-js on JS files Cherry pick fixes to JS linting from feature/image-loading-optimization Unset REQUEST_URI in tests that call go_to Reset SERVER global after each test Unrevert 2181d88 for audit-enqueued-assets-test.php Prevent sending header during test Unrevert 2181d88 for server-timing-tests.php Revert test changes moved to #924 Manually fix remaining phpcs issues ...
* Include screen media type * Remove whitespace padding from media features * Omit needless min-width:0 media feature
westonruter
requested review from
felixarntz,
joemcgill,
mukeshpanchal27 and
swissspidy
Co-authored-by: Pascal Birchler <pascalb@google.com>
| */ | ||
| $background_image = null; | ||
| if ( $style && preg_match( '/background(-image)?\s*:[^;]*?url\(\s*[\'"]?(?!data:)(?<background_image>.+?)[\'"]?\s*\)/', $style, $matches ) ) { | ||
| $background_image = $matches['background_image']; |
There was a problem hiding this comment.
nice use of a named sub-pattern, I forgot that was possible.
| $processor->remove_attribute( 'loading' ); | ||
| // Never include loading=lazy on the LCP image common across all breakpoints. | ||
| if ( 'lazy' === $processor->get_attribute( 'loading' ) ) { | ||
| $processor->set_attribute( 'data-ilo-removed-loading', $processor->get_attribute( 'loading' ) ); |
There was a problem hiding this comment.
nice, I like how the change is marked with a data attribute
| $processor->set_attribute( 'data-ilo-removed-loading', $processor->get_attribute( 'loading' ) ); | ||
| $processor->remove_attribute( 'loading' ); | ||
| } | ||
| } elseif ( $all_breakpoints_have_url_metrics && $processor->get_attribute( 'fetchpriority' ) ) { |
There was a problem hiding this comment.
this bit is unclear to me. if all breakpoints have data, you are falling back to the server heuristics?
There was a problem hiding this comment.
If all breakpoints have data, then we can reliably use the preload links to prioritize loading of the LCP image. In that case, we can remove the fetchpriority attribute from the LCP image here.
There was a problem hiding this comment.
I see, so you are favoring preload over fetchpriority? maybe we discussed this before, couldn't you use both of them? or is high priority already implied when using preload?
There was a problem hiding this comment.
The preload links also include fetchpriority=high, so having it on the element is redundant. Also, it can be counter productive to be on the img in the case where there are different LCP images for different viewports. If there is an image with fetchpriority=high but it is only for the desktop viewport then it will get loaded with high priority even for mobile users. The preload links also include media attribute with media queries so they enable fetchpriority=high to be conditional on the viewport size.
| </head> | ||
| <body> | ||
| <div class="header desktop" style="background: red no-repeat center/80% url(\'https://example.com/desktop-bg.jpg\'); width:100%; height: 200px;">This is the desktop background!</div> | ||
| <div class="header tablet" style=\'background-image:url( "https://example.com/tablet-bg.jpg" ); width:100%; height: 200px;\'>This is the tablet background!</div> |
There was a problem hiding this comment.
the quoting on this line is reversed from all of the others which makes it stand out. is that required for the test to pass?
There was a problem hiding this comment.
It's just testing that single-quoted and double-quoted strings both work in the CSS url().
| </head> | ||
| <body> | ||
| <div class="header desktop" style="background: red no-repeat center/80% url(\'https://example.com/desktop-bg.jpg\'); width:100%; height: 200px;">This is the desktop background!</div> | ||
| <div class="header tablet" style=\'background-image:url( "https://example.com/tablet-bg.jpg" ); width:100%; height: 200px;\'>This is the tablet background!</div> |
There was a problem hiding this comment.
Ditto. Just to make sure that the regex works with single quotes as well as double quotes.
adamsilverstein
left a comment
adamsilverstein
left a comment
There was a problem hiding this comment.
Looks great, left some small comments/questions
3 participants
Summary
Add image
preloadlinks for thebackground-imageon the LCP element.Doing a quick test with the parallax Cover block, the changes in this PR reduce median LCP-TTFB from 71.1ms to 64.8ms over 25 requests respectively: almost a 9% reduction.
Relevant technical choices
Adding
preloadlinks was already being done for LCPimgelements. When all breakpoints had the sameimgas the LCP element, it also addsfetchpriority=highto thatimg. Since a CSSbackground-imagecannot getfetchpriority=high, this is only added via thepreloadlink.Both the
background-imageproperty andbackgroundshorthand property are supported. Aurl()containing a data: URL is skipped.Note that CSS allows for a
background/background-imageto have multipleurl()CSS functions, resulting in multiple background images being layered on top of each other. This ability is not employed in core. Here is a regex to search WPDirectory for instances of this:background(-image)?:[^;}]+?url\([^;}]+?[^_]url\(. It is used in Jetpack with the second background image being a gradient. To support multiple background images, the logic would need to be modified to make$background_imagean array and to have a more robust parser of theurl()functions from the property value.The
preloadlinks are improved to add ascreenmedia type and to omit the needlessmin-width:0pxmedia feature.This PR also adds server timing for
image-loading-optimization. When checking withbenchmark-server-timingover 100 requests, enabling Image Loading Optimization only increases the response time by ~2ms.Also, this removes the workaround for browsers that don't support
imagesrcsetsince >95% do. See #117 (comment).Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.