Issue with minifying some CSS combined assets

[arvislacis]

Winter CMS Build

dev-develop

PHP Version

8.0

Database engine

SQLite

Plugins installed

No response

Issue description

Asset combine and minify tool has problems when trying to combine and minify some CSS files, for example, TailwindCSS - https://unpkg.com/tailwindcss@2.2.16/dist/tailwind.min.css (original file)

• Current output after combine and minify operation can be seen here
• Expected output can be seen here

Steps to replicate

1. Download https://unpkg.com/tailwindcss@2.2.16/dist/tailwind.min.css file into your project's CSS asset directory;
2. enableAssetMinify config option should be enabled (production environment of option value set to true) because this only happens if minify operation is executed after file combination;
3. Include mentioned CSS file into your project using $this->addCss(['assets/css/tailwind.min.css']); - doesn't matter if you pass single file to addCss([ .. ]) or try to combine it also with other CSS files etc. For simplicity I am mentioning only this case where you pass single file.
4. After page rendering and execution has been done, the result file looks like this.
5. Generated output file afterwards doesn't work as expected - defined styles in original file are not working in page.

Workaround

If you remove /*! tailwindcss v2.2.16 | MIT License | https://tailwindcss.com *//*! modern-normalize v1.1.0 | MIT License | https://github.com/sindresorhus/modern-normalize */ from file then everything works as expected. So it suggests that cause of the problem may be in this string.

Any further details?

No response

[bennothommo]

@arvislacis I've taken a quick look, and I think this might be an issue with the CSSMin library we are using, not specifically anything in Winter. We'll have to investigate further, but it might be the case that we will need to switch that library out.

[LukeTowers]

This was affecting the wintercms.com website as well, 48b7a8f637e588aede82e2cac9871afb-1633501296.txt should have been the output but instead in production it was generating e2e8581af99b56bea6d20576b582b86b-1633502607.txt

[bennothommo]

@LukeTowers we might need to swap out natxet/cssmin with mrclay/minify (which we already use for JSMin) or tubalmartin/cssmin in Assetic.

[LukeTowers]

@bennothommo sounds good to me, we already need to revert the changes Sam made where he pulled in the entirety of Assetic into the library and pull in assetic through composer anyways.

[bennothommo]

Turns out it is probably related to Winter after all - October, in their infinite wisdom, is rocking a custom stylesheet minifier filter which we inherited: https://github.com/wintercms/storm/blob/develop/src/Assetic/Filter/StylesheetMinify.php

[arvislacis]

Cause of my original problem is probably in https://github.com/wintercms/storm/blob/b774511b6f0cc3220a431507d6f13f27745c1dc9/src/Assetic/Filter/StylesheetMinify.php#L37 line which handles different types of comments etc.

But, yes, it looks like it's better to consider replacing this all minify() function with something else...

[bennothommo]

@arvislacis I'm trying out a replacement at the moment and will let you know how it goes.

[github-actions[bot]]

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[github-actions[bot]]

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[mjauvin]

@bennothommo I wonder if this is related to the issue I was having with js file minification from bootstrap5.

[bennothommo]

@mjauvin I'd say you're right. We're going to swap out the custom CSS minifier with one brought in through Assetic, so hopefully that'll sort out your B5 bug as well as this one.

[github-actions[bot]]

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[arvislacis]

Issue is probably related with https://github.com/assetic-php/assetic/issues/31 and potential fix (pull request) - https://github.com/assetic-php/assetic/pull/32

[LukeTowers]

Fixed by https://github.com/assetic-php/assetic/commit/f5fd3415fdb0a280b86eb50e2f0ad9754f842706, thanks to @damsfx!