CssOptimizer silently drop the attached library when a CSS file @include an external CSS.

Created on 26 July 2023, over 1 year ago

Problem/Motivation

While investigating πŸ’¬ CssCollectionGrouper should respect media property from CSS asset definition Closed: outdated we found under certain circumstances aggregated CSS does not include some of the attached files/libraries. I found that If a library requests a CSS files which @imports an other CSS from a 3rd party (e.g CSS from CDN, or a google font) none of the CSS files of the library will be included in the aggregated CSS file.

Steps to reproduce

Create and attach a CSS file which contains an @import from 3rd party.

style.css:

@charset "UTF-8";
@import url("https://fonts.googleapis.com/css2?family=Fraunces:opsz,wght@9..144,400;9..144,600&display=swap");
body {
  backround: lightblue;
  font-family: "Fraunces", sans-serif;
}

MY_THEME.libraries.yml

global:
  css:
    theme:
      style.css: {}

MY_THEME.libraries.yml

global:
  css:
    theme:
      style.css: {}

MY_THEME.info.yml

name: 'MY THEME'
type: theme
base theme: false
core_version_requirement: ^10

libraries:
  - MY_THEME/global

Proposed resolution

One solution can be that @import should be part of the aggregated CSS without any modification.

If this is not possible the CSS optimizer should throw an error and/or log that this CSS file contains a 3rd party @import which should be a separated library or the library should define it as an external asset.

πŸ› Bug report
Status

Postponed: needs info

Version

10.1 ✨

Component
Asset libraryΒ  β†’

Last updated about 22 hours ago

No maintainer
Created by

πŸ‡­πŸ‡ΊHungary tikaszvince

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

Comments & Activities

  • Issue created by @tikaszvince
  • Status changed to Needs work over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    The logic here was originally added in:

    commit 5d8cf1b865b1a926957bbc8802618dad08506fe0
    Author: GΓ‘bor Hojtsy <gabor@hojtsy.hu>
    Date:   Fri Dec 7 11:14:05 2007 +0000
    
        #113607 by Steven, chx, hass, catch and dvessel: proper inclusion of style sheets when/where @import is used
    
    

    Looks like it was last fixed in #2936067: CSS aggregation fails on many variations of @import β†’ which seems similar to your issue and added test coverage.

    I've added your example google fonts URL to the existing unit test in case there was something in there that was confusing the regexp, and that test still passes.

  • last update over 1 year ago
    29,449 pass, 2 fail
  • Status changed to Postponed: needs info over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    I also did some manual testing with Olivero.

    Added this diff:

    diff --git a/core/themes/olivero/css/base/base.css b/core/themes/olivero/css/base/base.css
    index 70471f960e..6fab540f31 100644
    --- a/core/themes/olivero/css/base/base.css
    +++ b/core/themes/olivero/css/base/base.css
    @@ -1,3 +1,5 @@
    +@charset "UTF-8";
    +@import url("https://fonts.googleapis.com/css2?family=Fraunces:opsz,wght@9..144,400;9..144,600&display=swap");
     /*
      * DO NOT EDIT THIS FILE.
      * See the following change record for more information,
    

    Then I checked the CSS aggregates (it was the second of two on the front page), and the first bit of the aggregate looks like this:

    @import url("https://fonts.googleapis.com/css2?family=Fraunces:opsz,wght@9..144,400;9..144,600&display=swap");
    /* @license GNU-GPL-2.0-or-later https://www.drupal.org/licensing/faq */
    @font-face{font-family:metropolis;src:url(/core/themes/olivero/fonts/metropolis/Metropolis-Regular.woff2) format("woff2");font-weight:normal;font-style:normal;font-display:swap;}@font-face{font-family:metropolis;src:url(/core/themes/olivero/fonts/metropolis/Metropolis-Bold.woff2) format("woff2");font-weight:700;font-style:normal;font-display:swap;}@font-face{font-family:metropolis;src:url(/core/themes/olivero/fonts/metropolis/Metropolis-SemiBold.woff2) format("woff2");font-weight:600;font-style:normal;font-display:swap;}@font-face{font-family:Lora;src:local("Lora Regular"),local("Lora-Regular"),url(/core/themes/olivero/fonts/lora/lora-v14-latin-regular.woff2) format("woff2");font-weight:400;font-style:normal;font-display:swap;}@font-face{font-family:Lora;src:local("Lora Italic"),local("Lora-Italic"),url(/core/themes/olivero/fonts/lora/lora-v14-latin-italic.woff2) format("woff2");font-weight:400;font-style:italic;font-display:swap;}@font-face{font-family:Lora;src:local("Lora Bold"),local("Lora-Bold"),url(/core/themes/olivero/fonts/lora/lora-v14-latin-700.woff2) format("woff2");font-weight:700;font-style:normal;font-display:swap;}
    :root{--font-sans:"m [... snip ..]
    

    As you can see it's preserved.

    I think we need actual steps to reproduce here from a clean install of the Drupal core standard profile - i.e. either an example module or theme, or a patch that can be applied to an existing module or theme that shows the issue. It's clear that this case is accounted for in CssOptimizer, there's test coverage, and it also works for me manually testing the steps described in the issue summary.

Production build 0.71.5 2024