AMP prevents css assets from generating when aggregation is enabled on Drupal 10.1

Created on 17 July 2023, 12 months ago
Updated 23 January 2024, 5 months ago

Problem/Motivation

When aggregation is enabled on Drupal 10.1 while using AMP, css/ assets will not be generated, this is because Drupal has changed its aggregation method to be loaded using a controller, and if the optimized files are not found, they are generated, but AMP prevents this process from happening.

🐛 Bug report
Status

Needs review

Version

3.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @Ahmad Smhan
  • Status changed to Needs review 11 months ago
  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update 11 months ago
    Waiting for branch to pass
  • 🇮🇳India hardikpandya

    I have tried this patch on one of my projects and it works. I am leaving this for others review and to be marked as RTBC.

  • 🇪🇸Spain marcoscano Barcelona, Spain
    +++ b/src/Asset/AmpCssCollectionRenderer.php
    @@ -164,10 +165,14 @@ class AmpCssCollectionRenderer extends CssCollectionRenderer {
    +            \Drupal::messenger()->addWarning('Failed to load the css file');
    

    I feel we need to come up with better wording for this, and maybe use a watchdog message instead of a message to the user?

    In any case, the patch in #2 works for us when aggregation is enabled. I'm going to leave the ticket in "Needs review" so the maintainers can chime in on the above mentioned issue.

    Thanks!

  • 🇭🇺Hungary szato

    The same issue on D10.1.6, patch #2 solved the issue. Thank you.

  • 🇨🇦Canada mcgowanm

    If anyone is still having issues after applying this patch and your environment has htauth on try turning it off.

  • First commit to issue fork.
  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update 5 months ago
    Waiting for branch to pass
  • 🇦🇷Argentina anairamzap Buenos Aires

    I've created a MR that uses the provided patch at #2 🐛 AMP prevents css assets from generating when aggregation is enabled on Drupal 10.1 Needs review with the following changes:

    1. Replacing Drupal static calls with dependency injection
    2. Removing the $needs_rewrite check, so we always send the assets to the doRewrite() method. I've checked and without that, the custom fonts won't be loaded (404) since we were trying to get them using the relative one level up../. Screenshot attached trying to explain this error.
    3. Removing the message inside the catch, replacing it with a proper logger if something goes wrong in the curl guzzle request.
    4. Also added a to-do comment to the guzzle request since this was failing on my local env (using docker localhost)

    I will also provide, in a separate branch another "solution" for this css not loading issue. Being more like a workaround to be able to keep using the file_get_contents approach, simply ignoring aggregation for css (exactly as this module is doing with the javascript) until we can find a working solution that applies to all environments (please not the curl guzzle request will also fail if sites are behind a proxy).

  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update 5 months ago
    Waiting for branch to pass
  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update 5 months ago
    Waiting for branch to pass
  • 🇦🇷Argentina anairamzap Buenos Aires
  • Status changed to Needs work 5 months ago
  • 🇦🇷Argentina anairamzap Buenos Aires

    ok, so... I've tested https://git.drupalcode.org/project/amp/-/merge_requests/17 in one of our sites dev environment that requires to have a cookie setup in order to load the pages and the guzzle approach is failing with a 403 since that request does not have the cookie set. I imagine a similar thing would happen on sites behind a proxy and we know is also happening on a local docker environment.

    I'll try to modify the MR re-writing the render method. Setting back to needs work.

  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update 5 months ago
    Waiting for branch to pass
  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update 5 months ago
    Waiting for branch to pass
  • Status changed to Needs review 5 months ago
  • 🇦🇷Argentina anairamzap Buenos Aires
Production build 0.69.0 2024