Lazyload CSS background image compatibility issue with Chrome 119+ #6269

Mai-Saad · 2023-11-14T13:14:34Z

Before submitting an issue please check that you’ve completed the following steps:

Made sure you’re on the latest version => 3.15.4 (and all previous 3.15)
Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug
Multiple request is sent to the same image using a specific page template https://new.rocketlabsqa.ovh/ll-bgi-oct-ar/

To Reproduce
Steps to reproduce the behavior:

using ch Version 119.0.6045.123 (Official Build) (64-bit) on ubuntu 23.10 or using browserstack windows 11 + ch 119
Enable LL BG image
Visit the mentioned page template using env in 1 and notice console / network tab while scrolling into the page

Expected behavior
Single request sent to testnotExist.png and file_example_TIFF_1MB.tiff
Console error for the 404 testnotExist happens only once as it was used once in the page

Screenshots

Additional context
Add any other context about the problem here.

Other images have single request though

Acceptance Criteria (for WP Media team use only)
Clear instructions for developers, to be added before the grooming

The text was updated successfully, but these errors were encountered:

jeawhanlee · 2023-11-21T09:43:39Z

Reproduce the problem ✅

I was able to reproduce the problem

Use chrome 119+
Enable Lazyload Background CSS Images
Load a template that has a 404 background image
scroll through the page with console open.

Identify the root cause ✅

The problem happens when an element having a 404 background image is scrolled into viewport, When this happens, we will append the style to the wpr-lazyload-bg styles in the head, we do this here:
https://github.com/wp-media/wp-rocket/blob/cfc26a39c9c42378e20acbdfdd99a998c5c34460/src/js/global/lazyload-css.js#L26C17-L26C17
When a style with a 404 image is loaded like this into the style in the head, Subsequent scrolls of other elements into viewport will trigger the browser to load these kind of 404 resources for some reason.

Scope a solution ✅

I suggest that we do a check right before we append new styles to load. We will check if the image can be loaded and if not we don't append these kind of styles so as not to trigger unnecessary console errors.
We can use the Image object for this kind of check and perform a regex on the pair.style to extract the url and parse this to img.src then we only append the style when that image can be loaded.

img.onload = () => {
    styleElement.innerHTML += pair.style;
}

Estimate the effort ✅
[XS]

CrochetFeve0251 · 2023-11-21T12:38:11Z

@Mai-Saad @jeawhanlee Does something has be changed on the page https://new.rocketlabsqa.ovh/ll-bgi-oct-ar/? Cause I came with Version 119.0.6045.159 (Official Build) (64-bit) on Ubuntu and I can't reproduce the issue.

Ok I succeeded to reproduce it.
Mmm to be fair I am divided about this.
On one hand this is due to chrome reloading all images without a 200 response code when we modify the tag.
On another it is indeed an error coming back on the console from the user.

I think first step would be to check if it is a behavior Chrome plan to fix or if it is on purpose.

On the grooming itself I don't like the fact we totally hide the error to the user. There should be not multiple errors but there should not have none either.

I gonna first dig a bit into Chrome to see if I can find if this is categorized as a bug for them.

jeawhanlee · 2023-11-21T13:06:51Z

@CrochetFeve0251 We don't totally hide the error from the user with this solution, the error will only show up once as opposed to how it pops up now with every scroll.

Also Irrespective of whether chrome would fix this kind of behaviour, I think we should have this kind of check just to be on the safe side to avoid lazy loading broken image links or images like file_example_TIFF_1MB.tiff that are least supported by browsers.

CrochetFeve0251 · 2023-11-21T13:19:04Z

@jeawhanlee I recreated a template to test the issue and looks like Firefox is also affected:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Title</title>
    <style id="style">
        :root{--wpr-bg-71417af1-5007-4ecf-a3bb-a534f60e8822: url('https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/test_internal0.webp');}:root{--wpr-bg-72a04f35-a851-4bf3-814e-5cec5ada235d: url('https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/image/test3.webp');}:root{--wpr-bg-e5a58dd6-df11-4dbf-bf04-3a313b24e71f: url('https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/image/file_example_JPG_100kB.jpg');}:root{--wpr-bg-99f15daa-f841-4699-98c7-f13eb833031b: url('https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/testnotExist.png');}
    </style>
</head>
<body>
    <div style="height:100px; width:100px;    background-image: var(--wpr-bg-71417af1-5007-4ecf-a3bb-a534f60e8822);">

    </div>
    <div style="height:100px; width:100px;    background-image: var(--wpr-bg-99f15daa-f841-4699-98c7-f13eb833031b);">

    </div>

    <button id="btn">Click</button>
    <script type="application/javascript">
        document.getElementById('btn').onclick = () => { document.getElementById('style').innerHTML += ":root{--wpr-bg-71417af1-5007-4ecf-a3bb-a534f60e8822: url('https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/test_internal0.webp');}"; }
    </script>
</body>
</html>

Seems like the issue is coming from the fact we modify the same style tag. (Looks like I am wrong all the style tags are reloaded when we add some content in one of them Q.q)
I am not a big fan of using a regex too or to load the image twice.

jeawhanlee · 2023-11-21T13:38:32Z

@CrochetFeve0251 Yeah, I understand the costs that might come with using a regex here but it should be as basic as getting the url from a var like this: :root{--wpr-bg-71417af1-5007-4ecf-a3bb-a534f60e8822: url('https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/test_internal0.webp');} something like this /url\(['"]?(.*?)['"]?\)/ and as the image loaded twice, we will only be loading it once when I implemented this basic code below while inspecting the network tab, the image don't load 2x:

const img = new Image();
const matches = pair.style.match(/url\(['"]?(.*?)['"]?\)/);
const imageUrl = matches ? matches[1] : null;
img.src = imageUrl;

img.onload = () => {
    styleElement.innerHTML += pair.style;
}

pair.elements.forEach(el => {
    // Stop observing the target element
    observer.unobserve(el);
    el.setAttribute(`data-rocket-lazy-bg-${pair.hash}`, 'loaded');
});

CrochetFeve0251 · 2023-11-27T13:41:54Z

Scope a solution ✅

First we will have to expose the data we need in the tag for that we will have to change the logic in MappingFormatter:

At this line we will now return:

$variable_placeholder = $datum['selector'] . '{' . $placeholder . ': ' . $url . ';}';
			$formatted_urls[]     = [
				'selector' => $selector,

At this line we will now also return the url field:

'url' => $datum['url']

Then we will now use the Image object for this kind of check and use the pair.url as img.src then we only append the style when that image can be loaded.

img.onload = () => {
    styleElement.innerHTML += pair.style;
}

We will also have to change the tag to prevent the reload from the styles on each time.

For that inside TagGenerator we will have transform the style tag to a div one:

		$loaded_tag = "<div id=\"wpr-lazyload-bg\"></div><style id=\"wpr-lazyload-bg-exclusion\">$loaded_content</style>";

Finally inside the script, we change the logic to add a style on each time:

						styleElement.innerHTML += `<style>${pair.style}</style>`;

Estimate the effort ✅

Effort S

jeawhanlee · 2023-11-27T14:57:34Z

Looks good to me.

…Chrome 119+

…Chrome 119+ (#6300)

Mai-Saad changed the title ~~Lazyload CSS background image compatibility issue with ch 119+~~ Lazyload CSS background image compatibility issue with Chrome 119+ Nov 14, 2023

piotrbak added needs: r&d Needs research and development (R&D) before a solution can be proposed and scoped. needs: grooming labels Nov 15, 2023

jeawhanlee added the effort: [XS] < 1 day of estimated development time label Nov 21, 2023

remyperona added effort: [S] 1-2 days of estimated development time and removed needs: grooming effort: [XS] < 1 day of estimated development time labels Nov 28, 2023

Miraeld self-assigned this Dec 1, 2023

Miraeld added a commit that referenced this issue Dec 1, 2023

Closes #6269: Lazyload CSS background image compatibility issue with …

1fb6c44

…Chrome 119+

Miraeld mentioned this issue Dec 1, 2023

Closes #6269: Lazyload CSS background image compatibility issue with Chrome 119+ #6300

Merged

8 tasks

Miraeld added a commit that referenced this issue Dec 14, 2023

Closes #6269: Lazyload CSS background image compatibility issue with …

2b8185b

…Chrome 119+

piotrbak added this to the 3.15.7 milestone Dec 14, 2023

github-merge-queue bot pushed a commit that referenced this issue Jan 1, 2024

Closes #6269: Lazyload CSS background image compatibility issue with …

29697ac

…Chrome 119+ (#6300)

Mai-Saad closed this as completed in #6300 Jan 1, 2024

piotrbak added the needs: documentation Issues which need to create or update a documentation label Jan 3, 2024

piotrbak mentioned this issue Jan 3, 2024

LL BGCSS markup problem with 3.15.7 #6360

Closed

remyperona mentioned this issue Jan 4, 2024

3.15.7 #6355

Merged

wordpressfan pushed a commit that referenced this issue Mar 15, 2024

Closes #6269: Lazyload CSS background image compatibility issue with …

873e311

…Chrome 119+ (#6300)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lazyload CSS background image compatibility issue with Chrome 119+ #6269

Lazyload CSS background image compatibility issue with Chrome 119+ #6269

Mai-Saad commented Nov 14, 2023

jeawhanlee commented Nov 21, 2023

CrochetFeve0251 commented Nov 21, 2023 •

edited

Loading

jeawhanlee commented Nov 21, 2023 •

edited

Loading

CrochetFeve0251 commented Nov 21, 2023 •

edited

Loading

jeawhanlee commented Nov 21, 2023

CrochetFeve0251 commented Nov 27, 2023 •

edited

Loading

jeawhanlee commented Nov 27, 2023

Lazyload CSS background image compatibility issue with Chrome 119+ #6269

Lazyload CSS background image compatibility issue with Chrome 119+ #6269

Comments

Mai-Saad commented Nov 14, 2023

jeawhanlee commented Nov 21, 2023

Reproduce the problem ✅

Identify the root cause ✅

Scope a solution ✅

CrochetFeve0251 commented Nov 21, 2023 • edited Loading

jeawhanlee commented Nov 21, 2023 • edited Loading

CrochetFeve0251 commented Nov 21, 2023 • edited Loading

jeawhanlee commented Nov 21, 2023

CrochetFeve0251 commented Nov 27, 2023 • edited Loading

Scope a solution ✅

Estimate the effort ✅

jeawhanlee commented Nov 27, 2023

CrochetFeve0251 commented Nov 21, 2023 •

edited

Loading

jeawhanlee commented Nov 21, 2023 •

edited

Loading

CrochetFeve0251 commented Nov 21, 2023 •

edited

Loading

CrochetFeve0251 commented Nov 27, 2023 •

edited

Loading