yui / yuicompressor

YUI Compressor
http://yui.github.com/yuicompressor/
Other
3.01k stars 664 forks source link

YUI Compressor is breaking some CSS, specifically calc(100% + ##px) #59

Closed djgilcrease closed 10 years ago

djgilcrease commented 11 years ago
.issue-59 {
    width:100%;
    width: -webkit-calc(100% + 30px);
    width: -moz-calc(100% + 30px);
    width: calc(100% + 30px);
}

turns into .issue-59{width:100%;width:-webkit-calc(100%+30px);width:-moz-calc(100%+30px);width:calc(100%+30px)}

instead of

.issue-59{width:100%;width:-webkit-calc(100% + 30px);width:-moz-calc(100% + 30px);width:calc(100% + 30px)}

Notice the missing spaces

tml commented 11 years ago

calc() is specifically called out as "at risk of being dropped during the CR period", but we should probably still address this one, as the blasted spec does - in fact - require at least one whitespace character around + and - operators.

filipewl commented 11 years ago

I'm having exactly the same problem. I'm using Jammit to compress the CSS for production and my calc() expressions are breaking because the compressor is removing, sometimes, spaces before and after operators and parenthesis.

Examples: Original: calc(100% * 2 + 20px * (2 - 1)) Compressed: calc(100% * 2+20px *(2 - 1))

Does anybody know a workaround for this? I see no other option but to exclude this part of the CSS from Jammit's pipeline.

alfredxing commented 11 years ago

I'm having the same issue. Although calc is considered "experimental", it's a great tool to use (and it does work with major browsers). It would be great to have this fixed; I can't seem to find a workaround.

yetithefoot commented 11 years ago

Same here! Vote for this!

danielsamuels commented 11 years ago

Me too.

kmxz commented 11 years ago

Same problem troubled me

kris-o3 commented 11 years ago
background-position: calc(65% - (100px - 350px * 0.65)) 0;

gets compressed as:

background-position:calc(65% -(100px - 350px * .65)) 0;

i currently have to add a space after the - before the inner expression. like so:

background-position:calc(65% - (100px - 350px * .65)) 0;

i have to do this every time i commit css-related changes. it is making me very sad indeed.

eaoliver commented 11 years ago

Has there been any progress on this bug? It's maddening. Nested expressions are still being compressed, which breaks the calc function under Safari and Firefox. It works under Chrome.

Example: (source calcs had spaces been operations)

left: -moz-calc((100% - 106px)/2+64px); left: -webkit-calc((100% - 106px)/2+64px); left: calc((100% - 106px)/2+64px);

kris-o3 commented 11 years ago

i attempted to locate which of the many regex's might need tweaking for this but was caught up in the muck and mire of a very long if( ) statement indeed... surely someone who worked on it or familiar with it can have a look?

djgilcrease commented 11 years ago

We had fixed this in our local version, I just forgot to commit it back for upstream pull request #107

kris-o3 commented 11 years ago

great! let us know when the merge has been approved / is complete?

tml commented 11 years ago

The PR breaks the build, but it's a huge step forward. Thanks, @djgilcrease!

djgilcrease commented 11 years ago

How does it break the build? I was able to run the ant build and all the tests that were passing before changes were still passing.

tml commented 11 years ago

See the Travis status on https://github.com/yui/yuicompressor/pull/107

Or just check the build: https://travis-ci.org/yui/yuicompressor/builds/11968385

kris-o3 commented 11 years ago

not familiar with 'checking the build' but i see that it "failed" ? will it be tagged w/ a new version # when this fix is ready for prime-time?

tml commented 11 years ago

This will likely be part of 2.4.9, unless there's a strong objection from the user community.

eaoliver commented 11 years ago

What's the ETA on 2.4.9?

djgilcrease commented 11 years ago

Ok, will get the build passing again

djgilcrease commented 11 years ago

Build is passing for pull request #107 again https://travis-ci.org/yui/yuicompressor/builds/11977764

kris-o3 commented 11 years ago

fixed upon fixing issue #59?

https://github.com/yui/yuicompressor/pull/107

jlong commented 10 years ago

Can we get this merged please? Looks like it's ready to go.

djgilcrease commented 10 years ago

Just checking on the status of this since the build tests have been passing for 4 months now

patrick91 commented 10 years ago

Any update on this?

beaktor commented 10 years ago

One nice option: use simple math and do a double negative. The bug doesn't seem to affect the - operator, just +.

Replace this:

width: calc(100% + 20px)

With this:

width: calc(100% - -20px);
kris-o3 commented 10 years ago

interesting (?) workaround -- supported by all browsers?

djgilcrease commented 10 years ago

it is, but it does not fix multiplication or division issues

beaktor commented 10 years ago

@kris-o3 Tested Chrome, IE9, and FF without issue.

kris-o3 commented 10 years ago

in our case it was an issue with a space after -, before a parenthetical expression. came up with our own "dumb" workaround that also appears to work in the above a-grade browsers...

original expression: calc(65% - (100px - 350px * 0.65))

as altered by yuicomp, causing syntax error: calc(65% -(100px - 350px * 0.65))

this remains intact: calc(65% - 1*(100px - 350px * 0.65))

danielsamuels commented 10 years ago

What's the status of getting this merged? Seems like it's been fixed for quite a long time.

Aeon commented 10 years ago

+1 - any chance of getting this merged?

istro commented 10 years ago

homer

da2x commented 10 years ago

Progress?

kris-o3 commented 10 years ago

don't know, don't particularly care. have since switched to uglifyjs and cleancss. strongly advise doing same.

danielsamuels commented 10 years ago

Yeah, I'm looking to do the same pretty soon just because of this bug.

frankie-loves-jesus commented 10 years ago

@kris-o3 what is this cleancss, and can one use it in Rails with Sprockets?

kris-o3 commented 10 years ago

it's a nodejs package providing similar (and more modern) css minification. https://www.npmjs.org/package/clean-css

frankie-loves-jesus commented 10 years ago

Awesome! Looks like it's available for Rails as well:

https://github.com/joseph/ruby-clean-css

Thanks @kris-o3

cfalzone commented 10 years ago

+1 This needs to be merged already. Don't suppose anyone knows an alternative that I can use programmatically in Java Filter?

rediris commented 10 years ago

+1 Any update on a merge?

redfox05 commented 10 years ago

This is still an issue, and a serious one at that. We need a reply from the dev team on when this will be addressed. Unless they want us all to start using another solution?

To hopefully speed things up, im going to quite clearly state to YUI, and others reading this, that we will be looking for an alternative. Do others have any other suggestions? I encourage users to suggest alternatives, which will both help the users, and highlight this as a serious issue for YUI team.

If you are reading this right now as a new user, and are having this issue, I urge you to create a github account and post that you are also having this issue.

frankie-loves-jesus commented 10 years ago

@redfox05 nobody's using YUI anymore. Plenty of suggestions have already been made - read the thread.

kris-o3 commented 10 years ago

Gulp with Uglify as a first pass, and Packer as a second pass, results in the smallest possible javascript code to-date.

https://www.npmjs.org/package/gulp-uglify https://www.npmjs.org/package/gulp-packer

cfalzone commented 10 years ago

Well the fix has been merged now we just need to wait for a release, which should be coming soon. See #107

frankie-loves-jesus commented 10 years ago

Pretty neat @kris-o3. What's so special about Packer though? Its website doesn't have any info.

Also, have you tried other minification combos? Perhaps even with three or four passes, or more?

kris-o3 commented 10 years ago

it's older technology made relevant w/ ports written for both Grunt and Gulp.

in a nutshell it's huffman-coding meets code obfuscation/compression... it builds a data dictionary of your code across six dimensions (p,a,c,k,e,r) which results in a single eval() statement that expands it into the javascript runtime vm when executed. very crazy stuff. and it works rather well.

http://dean.edwards.name/packer/

tml commented 10 years ago

@cfalzone You can also just pull HEAD and start using it; that would be especially useful, as it would give us some idea what other issues might be lurking. The more people willing to throw their code through HEAD, the better our releases will be.

redfox05 commented 10 years ago

@frankie-loves-jesus, I have been reading the thread, obviously. Thanks for that... advise...

"Nobody" ? Unfortunately, you are wrong, a lot of people ARE still using it, otherwise you wouldn't have any of those other recent comments that you can see above 'if you read the thread'.

Please think before you criticise someone, and make sure it is constructive and factually correct before you post.

On Wed, Nov 19, 2014 at 12:06 PM, frankie-loves-jesus < notifications@github.com> wrote:

@redfox05 https://github.com/redfox05 nobody's using YUI anymore. Plenty of suggestions have already been made - read the thread.

— Reply to this email directly or view it on GitHub https://github.com/yui/yuicompressor/issues/59#issuecomment-63630415.

tml commented 10 years ago

@redfox05 The change to fix this problem has been merged. If you need help figuring out how to pull/build HEAD so you can run your own CSS through it, let me know - but I have just run this issue's example CSS through HEAD, and it's working exactly as the original reporter told me it needed to. If there are other issues that need to be fixed, please open a new ticket. Thanks all!

kris-o3 commented 10 years ago

59 was opened Mar 29, 2013

by comparison...

in the time that this ONE issue was unresolved, an entirely different build system using a brand new (and greatly improved) CSS minification library has now come into existence.

notably, even the YUI team started down the road of officially deprecating use of YUI Compressor. see this blog post from oct 2012: http://www.yuiblog.com/blog/2012/10/16/state-of-yui-compressor/

tml commented 10 years ago

@kris-o3 If you have 705 additional commits to YUICompressor you've been keeping in your back pocket, I'd be happy to look over the PRs.

The PR that addressed this issue changed a lot of other code, and since I don't even use the CSS minifier (I use YUICompressor strictly as a JS minifier), it took a lot of time for me to get comfortable with all the changes.

YUICompressor is maintained entirely in my free, personal time. I give it every second I can spare; unfortunately, those seconds are increasingly rare. If you're unhappy with the pace of adoption, I invite you to maintain your own fork that keeps a pace you're more comfortable with; GH will even help you do it, just click the "Fork" button in the top right corner of this page.

Thanks for your feedback.