Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid CssMinifier stripping spaces that are followed by colons #214

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

willdean
Copy link
Contributor

@willdean willdean commented Nov 6, 2024

Fixes #139

The CSS Minifier strips spaces if they're followed by a colon. This is not safe for certain selectors, where maintaining the space is vital to separate the parts of the selector. e.g.: .classname :where([class~=otherclassname])

This PR includes a new test (which was failing), and removes the clause that allows a space to be removed if it's followed by a colon.
This will reduce minification to some (probably small) extent, but improves correctness. All existing tests still pass unmodified.

I have not tried, but I think that working out where exactly it is safe to remove <space>: might require a much more sophisticated minifier, with a much more detailed understanding of the CSS structure - my guess is that's not worth it.

@Shazwazza
Copy link
Owner

Thanks! But really folks should be using the Nuglify Smidge package to do all minification. Will probably retire the old CssMin and JsMin implementations since they get out of date too quickly and are near impossible to maintain. I'll get this merged in in the meantime :)

@Shazwazza Shazwazza merged commit 3436b51 into Shazwazza:develop Nov 6, 2024
@willdean
Copy link
Contributor Author

willdean commented Nov 6, 2024

Funny enough I assumed this was a Nuglify bug when I started looking into it (because, despite using Smidge for years, I didn't know how it worked), and there are lots of open issues on Nuglify which sounded like they could be this one.

But, now you've opened the door, I will agree that that the built-in CssMin is probably not the pinnacle of reliable, maintainable software.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smidge removes spaces which result in a different css context
2 participants