[FEATURE] add support for background images

[rommelfreddy]

Hey !

Magento does have a pagebuilder. Some image elements are placed with inline css as background image.

This PR will also handle CSS background images.

[jissereitsma]

Thanks! The NextGenImages is currently under heavy refactoring. I'll look into your PR once that refactoring is done, and I'll make sure the code is put to use. Thanks again!

[jissereitsma]

I thought at first it was a tag like <img src="data:image/gif;base64,foobar"/> but this is already skipped. Are you referring to something like <div style="background-image: url(foobar.png);" />? Because that's skipped as well? Or is there an img tag added with background-image?

I'm trying to proof that this is needed by extending upon a unit test Test\Unit\Util\HtmlReplacerTest, but if you could help me with creating the right HTML that would fail without this PR, but would succeed with this PR, then we have a test in place as well.

[jissereitsma]

By reading your PR (I should have done that already), I seem to understand that with your addition <div style="background-image: url(foobar.png);" /> would be turned into <div style="background-image: url(foobar.webp);" />. But this would modify the final HTML source, which is then cached and used for all browsers, right? And also, what if there are both WebP images and AVIF images added?

[rommelfreddy]

oh, i see what you mean. on devices which does not support webp images, the image should not replaces by the webp image.

my code should be modified so that the background image contains multiple image formats too.

maybe i will find time for that the next two weeks, but i can not promise it.

[jissereitsma]

Exactly, different devices should be supported by the same HTML. With a <picture> tag this is possible. But I'm not sure if this is possible with inline CSS (or any kind of CSS for that matter). For instance, an HTML tag could support media queries:

<div style="background-image: url(foobar.png); @media (max-width:300px){ background-image: url(foobar-small.png); }" />

But to my knowledge there is no media query for matching a WebP image. Before diving into modifying the PR, we should first sort out a possible HTML/CSS combo that overcomes this problem. But I'm not sure if this is possible, if CSS media queries are not useful here.

[jissereitsma]

To add to this, the next version of this WebP module will add a little JS to add a new CSS class to the body, being either webp or no-webp. This would allow for a CSS rule like body.webp div.example { background-image: url(foobar-small.webp); }. But still this can't be done in an inline style (which is only able to influence that HTML element).

[rommelfreddy]

i dont know any possiblities too.

but maybe this would be a nice beta feature which has to be explicit enabled in the administration. How do you think about this?

So the most of all browsers does support webp, so i think it is a good idea to move the decision to the admin of the shop. https://caniuse.com/webp

[jissereitsma]

Actually that sounds simple. But just like the IE discussions in the past, you don't want to block shops for older browsers. Even if the default would be to use WebP, this would still require a fallback. And the fallback is simply an issue with CSS backgrounds.

[sanjayaccorin]

@rommelfreddy Couldn't find $this->getAlternativeImagesByImageUrl($image); function on Util/HtmlReplacer.php

[rommelfreddy]

@rommelfreddy Couldn't find $this->getAlternativeImagesByImageUrl($image); function on Util/HtmlReplacer.php

Hey @sanjayaccorin this function was implemented in the past. i guess it has been removed/renamed in the past.

if i see it correctly, it is required to replace it with $images = $this->imageCollector->collect($imageUrl);

[rommelfreddy]

@sanjayaccorin i have updated the PR :) look forward to hear from you.

[rommelfreddy]

@jissereitsma, @sanjayaccorin i have updated the PR again. look forward to hear from you.

[jissereitsma]

Very nice! Thanks for all the work on this. I'm going to merge this, review it a bit and then include it in the next version, which is going to be released within a few hours.

[jissereitsma]

Version 0.4.0 has now been released with this fix on board. However, it has been made optional (Convert CSS backgrounds) with a default to false.

And a very vital change I made was to drop the additional brackets around the CSS definition. I didn't get why those were in there. It stopped a very simple CSS background from being matched (<div style="background-image: url(foobar.png);" />) and I couldn't duplicate the HTML tag in PageBuilder either.

If I'm wrong, could you please help me out with a sample HTML snippet that should match but currently doesn't?