Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-exclude Trust Index Google Widget CSS from LazyLoad for CSS Background Images #6957

Closed
webtrainingwheels opened this issue Sep 6, 2024 · 9 comments · Fixed by #7055
Closed
Assignees
Labels
module: lazyload background css images noQA priority: high Issues which should be resolved as quickly as possible quick win severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@webtrainingwheels
Copy link

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 using the Trust Index Google Widget plugin and LazyLoad for CSS background images, it creates a performance problem, especially on mobile.

The CSS file from that plugin contains A LOT of images. The resulting inline JS that we create is therefore very large.
This results in:

  • A high blocking time
  • Significant main thread work
  • High JS execution time

This problem could occur in any scenario where there are an excessive number of images in CSS, but it's very consistently happening with this specific plugin. Disabling the option improves performance in those cases.

To Reproduce
Steps to reproduce the behavior:

  1. Add this CSS file to your site: https://gist.github.com/webtrainingwheels/022e66c2f019142c99403a54d7d0b97d
  2. Enable LazyLoad for CSS Background Images
  3. Check the source code for the big inline script: https://jmp.sh/Ledw0Iu1
  4. Run a PageSpeed test and note the high blocking time, main thread work and JavaScript execution time: https://jumpshare.com/v/opd03dvcDIWVJFApH9kH, https://jumpshare.com/v/3EPMC794PYdLMmh2BNVx

Additional context
Slack convos:
https://wp-media.slack.com/archives/C08N8J6VC/p1724876333198109
https://wp-media.slack.com/archives/C8L4EE8N9/p1725582448549079

Related tickets:
https://secure.helpscout.net/conversation/2699519664/510946/
https://secure.helpscout.net/conversation/2689724995/509272/
https://secure.helpscout.net/conversation/2692407860/509723/
https://secure.helpscout.net/conversation/2681783608/507952?folderId=8127822
https://secure.helpscout.net/conversation/2697455510/510593?folderId=7406897
https://secure.helpscout.net/conversation/2688555094/509064?folderId=8127840

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

@camilamadronero
Copy link

@patrick-7-7-7
Copy link

patrick-7-7-7 commented Sep 18, 2024

My workaround is this in an mu-plugin (well, use it in some plugin or child theme):

<?php
add_filter("rocket_lazyload_css_ignored_urls", "excludeAllOfTrustindexFromLazyloadedCssImages", 10, 1 );

function excludeAllOfTrustindexFromLazyloadedCssImages($orig){
        $added = [
                "trustindex"
        ];
        $orig += $added;

        return $orig;
}

Actually, I use this array, but I have not tested whether there is a performance benefit. A few sites have 50 such images.
$added = [
'plugins/fusion-builder/assets/images/iLightbox',
'trustindex',
'wp-media-folder',
'plugins/all-in-one-event-calendar/public/',
]

Some sanity checking in the WP Rocket code would be nice. The most boring solution is to just set a limit of 50.

[edited to fix formatting]

@camilamadronero
Copy link

Another case https://secure.helpscout.net/conversation/2712580830/513207/
Setting these as LazyLoad for CSS background images exclusions worked:

trustindex-google-widget.css
cdn.trustindex.io

@camilamadronero
Copy link

@piotrbak piotrbak added type: bug Indicates an unexpected problem or unintended behavior priority: high Issues which should be resolved as quickly as possible severity: moderate Feature isn't working as expected but has work around to get same value module: lazyload background css images labels Sep 26, 2024
@piotrbak piotrbak added this to the 3.17.2 milestone Sep 26, 2024
@worldwildweb
Copy link
Contributor

@camilamadronero
Copy link

@camilamadronero
Copy link

@camilamadronero
Copy link

Rel: https://secure.helpscout.net/conversation/2734821096/517921/
The site's loading time was hugely impacted by this
We had to disable LL BG totally to fix that slowness problem

@piotrbak
Copy link
Contributor

AC:
Exclude the following patterns from the LazyLoad Background CSS:

trustindex-google-widget.css
cdn.trustindex.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: lazyload background css images noQA priority: high Issues which should be resolved as quickly as possible quick win severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants