After updating from advagg version 8.x-2.x to advagg version 8.x-3.x Some parts of the CSS broke.

After commenting out the following code in \Drupal\advagg_css_minify\Asset\CssMinifier::optimize() the CSS was working again.

    // Ensure that $data ends with ; or }.
    if (strpbrk(substr(trim($contents), -1), ';})') === FALSE) {
      $contents .= ';';
    }
CommentFileSizeAuthor
#8 advagg-css-minify-breaking-css-2897869-8.patch697 bytesDumitru Grosul
#6 advagg-css-minify-breaking-css-2897869-6.patch625 bytesDumitru Grosul

Comments

JeroenT created an issue. See original summary.

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented

Do you have an example of some CSS that it breaks?
Hard to fix/test without that.
Thanks;

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Status: Active » Closed (cannot reproduce)

Without knowing the CSS that is having issues, haven't been able to reproduce.

carsonw’s picture

carsonw
handsome / brilliant
commented

I am experiencing the same issue on a D8 site we're building.

A semicolon is prepended to a CSS selector in odd places, causing that selector to be ignored.

After commenting out line 55 in CssMinifier.php, the bug no longer exists.

Here's an example of CSS where this happens:

@media(max-width: 767px) {
  body {
    font-size:15px
  }

  h1 {
    font-size: 24px
  }

  h2 {
    font-size: 20px
  }

  .btn,input.button,.layout-header .search-form-wrapper input[type="submit"] {
    font-size: 14px
  }
}

img {
  max-width: 100%
}

; .front-main-hero {
  background: url("/themes/sitetheme/images/home-hero-bg.jpg") #7d5e97;
  padding: 50px 0;
  background-size: cover
}

.front-main-hero .hero-left {
  padding-bottom: 30px;
  max-width: 580px
}

@media(min-width: 992px) {
  .front-main-hero .hero-left {
    padding-top:220px;
    padding-bottom: 0
  }
}

The line in question is:

; .front-main-hero {

carsonw’s picture

carsonw
handsome / brilliant
commented
Status: Closed (cannot reproduce) » Active
Dumitru Grosul’s picture

Dumitru Grosul commented
Status: Active » Needs review
StatusFileSize
new advagg-css-minify-breaking-css-2897869-6.patch625 bytes

Had the same issue.
Apparently this happens when empty files are part of the aggregation.
Adding a patch here.

Status: Needs review » Needs work

The last submitted patch, 6: advagg-css-minify-breaking-css-2897869-6.patch, failed testing. View results

Dumitru Grosul’s picture

Dumitru Grosul commented
Status: Needs work » Needs review
StatusFileSize
new advagg-css-minify-breaking-css-2897869-8.patch697 bytes

Uploading a new patch with fixes based on testing feedback.

vladimiraus’s picture

vladimiraus
Primary language English
Location Brisbane 🇦🇺
commented
Status: Needs review » Reviewed & tested by the community

Thanks @dimag and everyone!
#8 fixed my issue.

pawandubey’s picture

pawandubey commented

Patch#8 works fine and verified the same.

RTBC+1

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

+1. But we probably need to add a test for this?

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

In this moment I think good solve the issue.
Can you create the tests on #3059999: Test to cover the changes on issue [#2897869]?

  • thalles committed d78099c on 8.x-3.x authored by dimag 
    Issue #2897869 by dimag, JeroenT, NickWilde, carsonw, thalles: Advagg...
thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Reviewed & tested by the community » Fixed

Thanks everyone!
Fixed!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.