Wp-rocket: Lazy load JS overrides second background-image property in inline style

[DahmaniAdame]

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

• [x] Made sure you’re on the latest version
• [x] Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug
When elements have two inline background-image properties, the lazy load overrides the second one when applying the image to the background.

To Reproduce
Steps to reproduce the behavior:

1. Enable WP Rocket lazy loading
2. Add an element with 2 inline background-image properties
3. Check the optimized page, and notice that the second background-image property is overriden

Expected behavior
If a second background-image property exists, WP Rocket should insert the image replacement above or below it.

Screenshots
https://drive.google.com/file/d/1D6CxfCqhL_2kvKZsOnvjFwhzVVMtDLm1/view?usp=sharing

Additional context
Code to test:

<div style='
height: 500px;
width: 100%;
background-color: rgba(255, 255, 255, 0);
background-image: url("https://playground.adame.io/wp-content/uploads/2020/10/51bc9ec2-5b4b-3358-9cd9-d21d10968f9f.jpg");
background-image:linear-gradient(90deg, rgba(247, 244, 243, 0.89) 59%, rgba(247, 244, 243, 0.21) 85%),
url(https://playground.adame.io/wp-content/uploads/2020/10/51bc9ec2-5b4b-3358-9cd9-d21d10968f9f.jpg);
background-position: center center;
background-repeat: no-repeat;
            '></div>

Backlog Grooming (for WP Media dev team use only)

• [ ] Reproduce the problem
• [ ] Identify the root cause
• [ ] Scope a solution
• [ ] Estimate the effort

[piotrbak]

I think it's this one, or related:
https://github.com/wp-media/wp-rocket/issues/2708

[iCaspar]

Also, may be related to this one: #2588

[DahmaniAdame]

Not sure if it's related to any of the two.
The background processing removes the first background-image property.
And the JS execution part overrides the then existing (second) background-image property.
The JS execution part should consider the presence of another background-image property in some cases (the first one is used as a fallback in some cases).

[DahmaniAdame]

Related ticket - https://secure.helpscout.net/conversation/1433598403/241634
Testing environment - https://playground.adame.io/double-inline-background

[piotrbak]

And the JS execution part overrides the then existing (second) background-image property.
Yes, it seems that this is exactly what's happening in the #2708

[Tabrisrp]

It's 2708, but also in addition the double background-image property, and since it's CSS, the second one takes priority over the first one, and replace it in the output.

This specific part is an edge case, having this property twice for the same selector doesn't really make sense.

[DahmaniAdame]

@Tabrisrp this how Avada is implementing their overlay background color on sections. We might stumble on this issue quite often (Avada has 600k installs).

From their demo - https://avada.theme-fusion.com/fitness/ (check the first hero section)

The double background-image property is a half-backed fix instead of using prefixes or using an additional div with an absolute position for the gradient overlay.

Reference - https://stackoverflow.com/questions/2504071/how-do-i-combine-a-background-image-and-css3-gradient-on-the-same-element

[Tabrisrp]

Yeah I figured it was for backward compatibility, with the first value as a fallback. It's kind of unnecessary still, as the browser support for multiple backgrounds is very very good.

But it's something to take into account when working on this, we should use the last property, not the first one.

[crystinutzaa]

Reproduce the issue ✅

Reproduced on local.

Identify the root cause ✅

Root cause is this preg_match:
https://github.com/wp-media/wp-rocket/blob/569969673f5f81b2c9d6e5c86e2ebc0d0c5366fa/inc/Dependencies/RocketLazyload/Image.php#L65
Because it matches the first background-image property.

Scope a solution ✅

The solution is to use preg_match_all and always replace the last background-image property identified.
Also, in casethe background-image property has other data beside the URL, then it should keep it and simply replace the URL :
Eg:

background-image:linear-gradient(90deg, rgba(247, 244, 243, 0.89) 59%, rgba(247, 244, 243, 0.21) 85%),
url(https://playground.adame.io/wp-content/uploads/2020/10/51bc9ec2-5b4b-3358-9cd9-d21d10968f9f.jpg);

Current code removes fully the background-image including the gradient. New code should keep the linear-gradient and remove only the url().

Estimate the effort ✅

Effort [S]