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

broken @ charset #86

Closed
XhmikosR opened this issue Mar 26, 2013 · 18 comments
Closed

broken @ charset #86

XhmikosR opened this issue Mar 26, 2013 · 18 comments
Assignees
Milestone

Comments

@XhmikosR
Copy link
Contributor

Using the files from the /bench dir.

cleancss TP-demo.css -o TP-demo-Clean-CSS.css --s0

results in:

@charset "."; .ui-helper-hidden{display:none}...

instead of

@charset 'UTF-8';.ui-helper-hidden{display:none}.

Notice the space after @charset.

BTW, are the double quotes replaced with single quotes in the @charset declarations?

@ghost ghost assigned GoalSmashers Mar 26, 2013
@GoalSmashers
Copy link
Contributor

The space happens to me as well but on Windows only.

I wonder what do you mean with @charset "."; as it is correctly preserved and double quotes are not changed to single ones.

@XhmikosR
Copy link
Contributor Author

Shouldn't the minified file have @charset 'UTF-8' like the source file?

@XhmikosR
Copy link
Contributor Author

As for the double quotes, the gain is negligible since only one charset declaration is allowed, but if it's easy can you add it?

@GoalSmashers
Copy link
Contributor

It does keep UTF-8 for me. Tested on node.js 0.8 and 0.10 on Windows and Mac. What version do you test on?

What's the difference between single and double quotes? Both are one byte, right?

@XhmikosR
Copy link
Contributor Author

You are right about the quotes, it was my misunderstanding.

node -v & npm -v
v0.10.1
1.2.15

On Windows 7.

The minified file is UTF-8 but the @charset declaration is what is shown in my first post.
Instead of @charset "UTF-8" it's @charset "."

@XhmikosR
Copy link
Contributor Author

I did some more testing and I have more info.

The space after the @charset line is because the original file has a comment right before and right after @charset. If I remove the comment then I get no space.

Test case

/*whatever*/
@charset "UTF-8";
/*whatever*/
blockquote small:before {
  content: '\2014 \00A0';
}

The weird issue with the @charset content seems to be because the original file has @font-face before the @charset.

This should be a smaller test case:

@font-face {font-family: 'ProximaNova-Regular';src: url('/assets/thirdParty/css/1415F2_1.eot');src: url('/assets/thirdParty/css/1415F2_1IE.eot') format('embedded-opentype'),url('/assets/thirdParty/css/1415F2_1.woff') format('woff'),url('/assets/thirdParty/css/1415F2_1.ttf') format('truetype'),url('/assets/thirdParty/css/1415F2_1.svg') format('svg');font-style: normal;font-weight: normal;}

@charset "UTF-8";
/* (c) 2012 Instagram, Inc, */
blockquote small:before {
  content: '\2014 \00A0';
}

becomes

@charset 'embedded-opentype';@font-face{font-family:ProximaNova-Regular;src:url(/assets/thirdParty/css/1415F2_1.eot);src:url(/assets/thirdParty/css/1415F2_1IE.eot) format('woff'),url(/assets/thirdParty/css/1415F2_1.woff) format('truetype'),url(/assets/thirdParty/css/1415F2_1.ttf) format('svg'),url(/assets/thirdParty/css/1415F2_1.svg) format("UTF-8");font-style:normal;font-weight:400}blockquote small:before{content:'\2014 \00A0'}

@GoalSmashers
Copy link
Contributor

It was because of wrong order of operations (@charset needs to come last). Thanks for spotting the case.

@XhmikosR
Copy link
Contributor Author

Np, thanks for fixing.

But now there is an empty line with --keep-line-breaks or a space without --keep-line-breaks.

@charset "UTF-8";

.ui-helper-hidden{display:none}
@charset "UTF-8"; .ui-helper-hidden{display:none}

@GoalSmashers
Copy link
Contributor

Ok, I'm on it.

@GoalSmashers
Copy link
Contributor

It seems to be fine now. Please close if it is.

@XhmikosR
Copy link
Contributor Author

Still the same here...

cmd /c cleancss bench/TP-demo.css -o bench/TP-demo-Clean-CSS.css --s0 -e --keep-line-breaks

results in

@charset "UTF-8";

.ui-helper-hidden{display:none}

@GoalSmashers
Copy link
Contributor

It should be fine now. Please remember to run against bin/cleancss instead of npm version.

@XhmikosR
Copy link
Contributor Author

I always do npm install -g...

The empty line is gone with --keep-line-breaks but the space is still there when not using --keep-line-breaks.

cmd /c cleancss bench/TP-demo.css -o bench/TP-demo-Clean-CSS.css --s0 -e
@charset "UTF-8"; .ui-helper-hidden{display:none}

@GoalSmashers
Copy link
Contributor

That's the third and the final fix as both transformations are fine now. It's 1.0 release time now.

@XhmikosR
Copy link
Contributor Author

Now everything seems to be perfect. Thanks.

@GoalSmashers
Copy link
Contributor

Cool, thanks!

@XhmikosR
Copy link
Contributor Author

BTW, this last patch fixes another stray space being left at the joining point of multiple css files like fancybox.css and then normalize.css with --s0. I suppose because of the special comments.

@GoalSmashers
Copy link
Contributor

Cool, it makes a lot of sense when looking at the last commit.

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

No branches or pull requests

2 participants