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

Bug : carriage router is breaking the tokenizer #1000

Closed
abarre opened this issue Feb 5, 2018 · 5 comments
Closed

Bug : carriage router is breaking the tokenizer #1000

abarre opened this issue Feb 5, 2018 · 5 comments
Labels

Comments

@abarre
Copy link
Contributor

abarre commented Feb 5, 2018

Our solution minifies CSS from various websites and some CSS have carriage return (\r) without the new line character (\n). This character breaks the tokenizer. The only patch I found is removing all (\r) with a replace statement before applying clean-css.

Environment

  • clean-css version - npm ls clean-css: clean-css@4.1.9
  • node.js version - node -v: v8.9.4
  • operating system: Mac OS

Configuration options

var CleanCSS = require('clean-css');
new CleanCSS({
level: { 1: {all:false}}
})

Input CSS

\r@media only screen and (max-width: 1250px) {#aspot-standard {width: 100%;}}

Please note the '\r' before the '@'.

Actual output CSS

@media only screen and (max-width: 1250px){}

Expected output CSS

@media only screen and (max-width: 1250px){#aspot-standard{width:100%}}
@abarre
Copy link
Contributor Author

abarre commented Feb 5, 2018

congrats for the 1000th issue :)

@jakubpawlowicz
Copy link
Collaborator

I wonder what would happen to source maps when getting rid of \r, but that's not a valid line break, or is it? If not then we can bring it in, but more likely by detecting those faulty carriage returns in the tokenizer itself.

p.s. I would rather have less issues but hey let's 🎉

@abarre
Copy link
Contributor Author

abarre commented Feb 25, 2018

\r alone is a valid newline character as per the CSS spec. However, I agree with you that this is really unexpected case.

@jakubpawlowicz
Copy link
Collaborator

So we should handle it.

@jakubpawlowicz jakubpawlowicz added this to the 4.2.0 milestone Mar 5, 2018
@jakubpawlowicz
Copy link
Collaborator

To be released with 4.2.0 tomorrow.

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

No branches or pull requests

2 participants