LazyLoad for CSS background images breaks for stylesheets added from a URL that doesn't point to a file

sandyfigueroa commented 9 months ago

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

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

Describe the bug When a stylesheed is included in the page with this format:

<link rel="stylesheet" id="custom-php-css" href="http://wp-rocket.esy.es/?code-snippets-css=1&ver=7" media="all">

(https://wp-rocket.esy.es/?code-snippets-css=1&ver=7)

LazyLoad for CSS background images fails, and the related images to this CSS are broken.

No file is created in /wp-content/cache/background-css/domain.com folder

And this kind of URL is used for the stylesheet: http://wp-rocket.esy.es/wp-content/cache/background-css/wp-rocket.esy.es/?code-snippets-css=1&ver=7&wpr_t=1705500659, which is incorrect and doesn't load any stylesheet.

Also, the page might be broken (broken styles) if the stylesheet included more CSS than the background image

To Reproduce Steps to reproduce the behavior (These were the steps I did):

Add an element (a div for example) with a selector like ".my-test-background-1"
```
<div class="my-test-background-1"></div>
```

Throw a CSS from PHP from a URL like: http://domain.com/?code-snippets-css=1&ver=7 I did this in the WordPress' index.php file:

/** Loads the WordPress Environment and Template */
if (isset($_GET['code-snippets-css'])) {
header('Content-Type: text/css');
echo '.my-test-background-1 {
    width: 500px;
    height: 500px;
    margin-left: auto;
    margin-right: auto;
    background-image: url("http://wp-rocket.esy.es/wp-content/uploads/2020/10/rose-165819_1920-1024x678.jpg");
}';
} else {
require __DIR__ . '/wp-blog-header.php';
}

Enable the LazyLoad for CSS background images
Clear WP Rocket's cache
Check the page

Expected behavior These kinds of stylesheets should't break the lazyload option. Maybe the URL can be used to get a hash and use it as the name of the file.

Additional context This happened to this user: https://secure.helpscout.net/conversation/2482565836/469421/, and I could reproduce it on my test site following the steops above.

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

camilamadronero commented 7 months ago

New ticket: https://secure.helpscout.net/conversation/2539850593/480890/

Khadreal commented 7 months ago

After doing some investigation on this, I noticed that the function is expecting a file url which will be formatted but url like this http://domain.com/?code-snippets-css=1&ver=7 aren't a file url per se.

in `Engine/Media/Lazyload/CSS/Subscriber.php`

https://github.com/wp-media/wp-rocket/blob/631fc90017805c8063340c771598b1ccec713772/inc/Engine/Media/Lazyload/CSS/Subscriber.php#L513-L515

We can change the token from ? maybe @ or something else.

A quick change of the token, fixed the error but still need to do more test.

Effort [S]

remyperona commented 4 months ago

The proposed solution is not enough in my opinion. We would need product direction on this, what do we want to do with dynamic CSS like this in that case? @piotrbak

suzoutlet commented 1 month ago

The file path we are trying to access: /_jb_static?-eJytVtGWmyAQ/Zs_laUmm27zsKfP/QyE0UyDwAGMm7/f0aQJ6apRkxePwNzLzGVmgDeOSWsimMjjDioIXIToBX0DxMBlCLxCgwWC4pVA80KjF5r9zhOk03WJJvCwsw4M/UKKv84yV_ca5RR8g6psCWjvmJKd5lnhO6gapAJNwZho/T_wxpwXZMREA8FW0LEJrS8MaKSuFQnwl0IGheJMcTNwWhzBMw2lkMcbLQbhtJaORwUstBWRzE3NT1Ld6BiPGqbC_YHUsT50Mf8f8hhHHSg_DyW2aRDRmtSFL4tfaO4mkbSVI2iOGuORN9bSRAVeQvrPSGZbR1Z6VL3ePmmbQf7BNGo3uSTfbOQ5eyN8REY5QauLObAS5Xg2XNDMedvnRashO1uFeY70VlOSak9gC1ZPPZyBAI04dMWwWOMdCIWmnI8nIzpdqg_h264mHMzUNz1oMmaaKu4hLQrrqyVnrEV0KPf__jefRBhK1laNcw8jM3aZY2Hn0ewXyxOsRKE7lRZofHNQwjkNrIF8jxPF7vPHCQlPEakQCv6MX7i9TNBp0fl0zYoF_rRcoUEHnh9_nUqyGz2oMrARmtrRFaZSmhbvbIgsW81P/nmdeqh0NDqW24/FKaHwgGqqbgNOSOEt3b79d/eIaqvNLOv1epZ9tn3dzrL/kf1cLEFycU_RMlAVU9_MwCur6q7rJAtdV6VS3acbAa2cCmXsvcHkDrU6P8cmO3HnQXuq_cfpPARHAeDhrmt3rt/cC6MuHeN39Z69rd6y9WazXX2T_Xv2Cfc_qRg&wpr_t=1727194505

Screenshot of the error.

This file is related to Jetpack Boost plugin.

wp-media / wp-rocket