Fix handling NOSCRIPT and IMG tags with JS-based lazy-loading #1745
Conversation
|
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. |
westonruter
added
[Type] Bug
| return false; | ||
| } | ||
|
|
||
| // Abort if the srcset is a data: URL since there is nothing to optimize. |
There was a problem hiding this comment.
Well, there could be something to optimize. We can't preload the URL, but we could still add fetchpriority=high to the IMG.
| private function process_img( OD_HTML_Tag_Processor $processor, OD_Tag_Visitor_Context $context ): bool { | ||
| $src = $this->get_valid_src( $processor ); | ||
| if ( null === $src ) { | ||
| if ( ! $this->is_img_with_valid_src_and_srcset( $processor ) ) { |
There was a problem hiding this comment.
As noted below, this may be too conservative. We could still process this IMG. It's just we would skip any parts related to preloading the src or srcset.
westonruter
modified the milestones:
image-prioritizer 0.3.0,
image-prioritizer n.e.x.t
|
I'm bumping this to the next release. |
|
I've split out the first part of this into a standalone PR: #1783 |
|
The second part I've split into another PR: #1785 |
When analyzing sites using Image Prioritizer in HTTP Archive, I came across some cases that hadn't been accounted for.
Tags inside
NOSCRIPTFirst of all, any tag which appears in a
NOSCRIPTshould always be excluded from visiting by any tag visitor. For example:This was always showing up as:
Image Prioritizer adds the
data-od-unknown-tagattribute to any visited tagIMGwhich is not located in URL Metrics. However, such anIMGcan never be added to URL Metrics since it is not located in the DOM.This PR skips over visiting any tag which is inside of a
NOSCRIPTelement.Such
NOSCRIPTtags are often found with JS-based lazy-loading implementations.JS-based Lazy-Loading
On quite a few sites I found JS-based lazy-loading causing problems with Image Prioritizer. In particular, the Avada theme's Fusion Images lazy-loading functionality replaces
srcsetwithdata-srcsetbut it doesn't change thesrc, for example:This was resulting in an erroneous preload link being added, in particular the
imagesrcsetis pointing to adata:URL:So this PR adds checks to make sure that if an
IMGhas such asrcsetit is also excluded from processing.In a future PR we could consider undoing such JS-based lazy-loading, rewriting
data-srctosrc,data-srcsetback tosrcsetand so on, and then to process theIMGas normal. This would have a dramatic improvement to LCP especially when the such JS-based lazy-loading is being done for images in the initial viewport, especially for anIMGwhich is the LCP element. See #1746.