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
fix(postcss-colormin): prevent interpreting all numbers as hex colors. #1118
Conversation
@omgovich do you agree with this solution? I wonder if the colord parser is not a bit too lenient. The interesting part is that the cssnano code never checks if color can be at that position, I guess because it is a bit complicated with all the shorthand declarations, so it relies on the color library failing to parse to decide whether to continue or not. Because colord accepts more values than color, cssnano ends up transforming any number with three digits. |
Thanks for the quick turnaround on this. |
Hey guys. Sad to see these issues. Colord was created to help people to type a color values (for example, in color picker fields) and fix possible mistakes there. It produces valid CSS colors, but parses even broken ones. I've never aimed to handle any other input except possible color value before. The current workaround is good enought I guess. |
We could use more readable integration test. The regression was visible in the generated output, and I even looked at it, but because the result is a single minified string, it is very easy to miss details. Maybe we should also run one plugin at a time on the sample CSS. |
Codecov Report
@@ Coverage Diff @@
## master #1118 +/- ##
=======================================
Coverage 96.37% 96.37%
=======================================
Files 115 115
Lines 3582 3585 +3
Branches 1054 1056 +2
=======================================
+ Hits 3452 3455 +3
Misses 121 121
Partials 9 9
Continue to review full report at Codecov.
|
Already working on |
@ludofischer Do you think we should merge this and do fast release or better to migrate on v2 in other PR? |
I think we should copy the tests from this in the other PR and release the
other because this one is not that well tested either, so it's better to
concentrate our attention on one PR
|
Just to get a sense for timing, any thoughts when this might make it into a release? Just trying to guage whether or not i should use a workaround or wait for the fix. |
Colord v2 is released and PR that fixes everything is ready to merge #1122 |
Superseded by #1122 cssnano 5.0.4 with fixed color parsing is out, please give it a try. |
The issue is that the colord parser is so lenient it accepts single numbers
as hex colors.
Fix #1113, fix #1115