search

matthiasmullie / minify

CSS & JavaScript minifier, in PHP. Removes whitespace, strips comments, combines files (incl. @import statements and small assets in CSS files), and optimizes/shortens a few common programming patterns.
https://matthiasmullie.github.io/minify/
MIT License
1.97k stars 309 forks source link

Minified CSS, broken since version 1.3.72 #422

Closed CreativeNative closed 7 months ago

CreativeNative commented 7 months ago

Unfortunately, I can't identify the problem. I can only assure that since version 1.3.72 my minified CSS is broken. With version 1.3.71 and 1.3.70 everything works fine.

matthiasmullie commented 7 months ago

Hi! Can you describe the problem you're seeing? Or post the CSS that fails to minify properly? Or you can email me at minify[at]mullie[dot]eu if you wish to keep the CSS private.

axllent commented 7 months ago

I am experiencing an issue too which I think is related, if not a sign of the cause (which hopefully can help you identify where this is happening). When compiling bootstrap (5) minify <= 1.3.71 produces:

color:rgba(var(--bs-link-color-rgb),var(--bs-link-opacity,1)); // correct

With 1.3.72 the same code produces the following invalid css:

color:rgb(var(--bs-link-color-rgb) var(--bs-link-opacity 1)); // invalid

This may not be limited to just this, but it is definitely something I can pinpoint.

CreativeNative commented 7 months ago

Yes, it's definitely a problem with the bootstrap framework and color. I'm experiencing the same issue. Grids and everything else are working fine, but colors don't work.

@matthiasmullie I will send you our CSS minified with version 1.3.71 and 1.3.72 via e-mail, so you can see the difference.

I compared both files and saw following changes (1.3.71 -> 1.3.72)

"transparent" went to "#fff0"

"rgba(0,0,0,.125)" went to "rgb(0 0 0 / .125)"

"RGBA(var(--bs-primary-rgb),var(--bs-bg-opacity,1))" went to "RGB(var(--bs-primary-rgb) var(--bs-bg-opacity 1)"

matthiasmullie commented 7 months ago

Hi - thanks again for your reports of this bug.

AFAICT, the patch that introduced some color conversions went a little haywire here - it essentially split rgb/hsl values on comma, but bootstrap makes extensive use of var() for color values, and var() itself can also be used with a 2nd argument (for default value), which introduces an unexpected comma.

I submitted a new commit that is supposed to tighten this up and only operate on literal values (not var())

Could any of you please test current master branch & report whether the fix is adequate before I publish a new release?

Thanks!

CreativeNative commented 7 months ago

I will check in the afternoon and let you know. 😉

axllent commented 7 months ago

@matthiasmullie I can confirm the fix in the master branch resolves color issue for me (color:rgba(var(--bs-link-color-rgb),var(--bs-link-opacity,1));) . But...

"rgba(0,0,0,.125)" went to "rgb(0 0 0 / .125)"

I'm still seeing rgb(0 0 0 / .125) multiple times in my compressed version, so I do not believe it is entirely fixed (unless that is actually valid?).

Thank you for looking into this so quickly.

matthiasmullie commented 7 months ago

That last change is actually valid; the rgb(0 0 0 / .125) is actually the "modern" format, while the comma-separated format is considered legacy (see https://drafts.csswg.org/css-color/#color-syntax-modern)

I just released a new version - should any more issues crop up, don't hesitate to file a bug report! :) Thanks again to both of you!

axllent commented 7 months ago

Awesome, you're a star! Thank you again.