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

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

Closed
djgilcrease opened this issue Mar 29, 2013 · 74 comments

Comments

Projects
None yet
@djgilcrease
Copy link
Contributor

commented Mar 29, 2013

.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

@ghost ghost assigned tml Mar 31, 2013

@tml

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2013

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

This comment has been minimized.

Copy link

commented Apr 4, 2013

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

This comment has been minimized.

Copy link

commented May 12, 2013

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

This comment has been minimized.

Copy link

commented Jul 4, 2013

Same here! Vote for this!

@danielsamuels

This comment has been minimized.

Copy link

commented Jul 4, 2013

Me too.

@kmxz

This comment has been minimized.

Copy link

commented Aug 7, 2013

Same problem troubled me

@kris-o3

This comment has been minimized.

Copy link

commented Sep 3, 2013

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

This comment has been minimized.

Copy link

commented Sep 28, 2013

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

This comment has been minimized.

Copy link

commented Sep 30, 2013

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 djgilcrease referenced this issue Sep 30, 2013

Merged

Issue 59 #107

@djgilcrease

This comment has been minimized.

Copy link
Contributor Author

commented Sep 30, 2013

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

@kris-o3

This comment has been minimized.

Copy link

commented Sep 30, 2013

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

@tml

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2013

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

@djgilcrease

This comment has been minimized.

Copy link
Contributor Author

commented Sep 30, 2013

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

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2013

See the Travis status on #107

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

@kris-o3

This comment has been minimized.

Copy link

commented Sep 30, 2013

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

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2013

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

@eaoliver

This comment has been minimized.

Copy link

commented Sep 30, 2013

What's the ETA on 2.4.9?

@djgilcrease

This comment has been minimized.

Copy link
Contributor Author

commented Sep 30, 2013

Ok, will get the build passing again

@djgilcrease

This comment has been minimized.

Copy link
Contributor Author

commented Sep 30, 2013

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

@kris-o3

This comment has been minimized.

Copy link

commented Oct 21, 2013

fixed upon fixing issue #59?

#107

@jlong

This comment has been minimized.

Copy link

commented Dec 2, 2013

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

@djgilcrease

This comment has been minimized.

Copy link
Contributor Author

commented Jan 23, 2014

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

@patrick91

This comment has been minimized.

Copy link

commented Feb 18, 2014

Any update on this?

@beaktor

This comment has been minimized.

Copy link

commented Feb 19, 2014

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

This comment has been minimized.

Copy link

commented Feb 19, 2014

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

@djgilcrease

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2014

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

@timbru31

This comment has been minimized.

Copy link

commented Oct 1, 2015

@jamespero Thanks for the hint. Kudos to you 🍻
This saved some headache in our team, since a v2.4.9 won't be released soon :(

bschwarzent added a commit to eclipse/scout.rt that referenced this issue Nov 3, 2015

HtmlUi: Enable minifyCss (add workaround for YUI bug)
YUI has a bug that breaks calc() expressions when minifying
CSS:yui/yuicompressor#59
As a workaround, we protected the whitespace in calc expressions by a
special token, run the minifier, and then replace the placeholders by
spaces again. (Bugfix is inspired by
yui/ycssmin#7)
@victole

This comment has been minimized.

Copy link

commented Feb 24, 2016

hi all, does this has already been solved?

@MorrisJohns

This comment has been minimized.

Copy link

commented May 26, 2016

Since this fix wasn't released (2.4.7 was last release of YUI compressor), I use a Python function to fix up the file by reinserting spaces around +:

def fixYuiCompressorBug(oldCss):
    calcRx = re.compile('calc[(]([^ )\r\n;]+[+][^ ])+', re.MULTILINE)
    plusRx = re.compile('[+]', re.MULTILINE)
    def fixCss(match):
        return 'calc(' + plusRx.sub(' + ', match.group(1))
    return calcRx.sub(fixCss, oldCss)

stapelberg added a commit to Debian/dcs that referenced this issue Jul 5, 2016

big client refactoring for faster page loads
There are a couple of high-level changes which all contribute to
decreasing the page load time, both for initial page loads and for any
subsequent page loads (with cached assets):

1. Instead of having the /search?q=query endpoint serve a non-javascript
   version with javascript that redirects to /results/query/page_0, the
   response now is used for both non-javascript and javascript: all
   javascript-specific elements are hidden, all non-javascript elements
   are wrapped in <noscript>. This eliminates an entire round-trip for
   all requests that did not use the search form (e.g. requests from the
   URL bar, or people following links).

   The old URL patterns are redirected server-side, i.e. a request to
   /results/query/page_0 results in an HTTP 302 to /search?q=query. This
   keeps old URLs working.

2. All CSS which is required for the initial page rendering is now
   inlined in the responses (see static/critical.css). Previously we
   used to ship both debian.css and debcodesearch.css and let the
   browser apply the directives. critical.css however contains no
   ignored/superseded directives, bringing down the size of the
   stylesheet even more.

3. All further CSS has been moved to non-critical.css, which is loaded
   in a non-blocking way.

4. We are now using the EventSource API¹ instead of WebSockets (for
   browsers which don’t support EventSource, there is a fallback to
   WebSockets). The original motivation to use WebSockets was to
   eliminate the round-trip for submitting the search form. Structuring
   our application like that implied a single-page app.

   With the EventSource API, we no longer use a single-page app design,
   i.e. when submitting the search form, the browser actually navigates
   to /search?q=query. This simplifies the JavaScript code (we no longer
   need to do auto-complete state handling, for example) and makes the
   javascript/non-javascript serving in point 1 easier.

   Conceptually, EventSource and WebSocket are much alike, but
   EventSource is uni-directional. This suits us well, as the only
   message we sent to the server was the search query. With EventSource,
   we just include the query in the EventSource request (e.g.
   /events/query).

   An advantage of the EventSource API is that it supports HTTP2 (which
   WebSockets don’t!). This effectively eliminates the round trip which
   we used to eliminate using WebSockets in HTTP1.

5. The progress bar animation has been changed to not animate the
   background-position property anymore, but rather use the translate
   directive which can be offloaded to the GPU entirely. This results in
   smooth animations which do not slow down the rest of the page while
   scrolling or loading results.

Step 1-3 mean that with the first response being loaded, we immediately
render the progress bar. This typically happens within about 100ms
(excluding network latency), resulting in feedback that feels instant.

In my measurements (with a 100ms latency, 6Mbit/s down, 1Mbit/s up
throttling profile in Chrome), the time to first paint is reduced from
800ms to 30ms when loading a /search?q=query URL (e.g. via the address
bar).

Sending a search query via the query form on the index page is a bit
slower with the new architecture because of the additional round trip
(the request for /search?q=query and then /events/query). This will be
addressed by service workers (see issue #69) which can handle
/search?q=query locally. Let’s be prudent and implement service workers
after the new architecture has been rolled out, though.

We use 2 Polyfills (JavaScript code which backports new functionality to
browsers which don’t support it yet):
• URLSearchParams (for Microsoft Edge):
  https://github.com/WebReflection/url-search-params
• loadCSS (for non-chrome):
  https://github.com/filamentgroup/loadCSS

We had to use 2 workarounds for issues in the minify tooling:
• rspivak/slimit#52yui/yuicompressor#59https://developer.mozilla.org/en-US/docs/Web/API/EventSource
  (which is available in all major browsers except for IE/Edge, see
   http://caniuse.com/#feat=eventsource)

related to #69
@isafin

This comment has been minimized.

Copy link

commented Sep 23, 2016

How this may be closed?! This problem still alive :\

And that solving calc(X% - -Xpx) — shit CSS code.

@msunasra

This comment has been minimized.

Copy link

commented Sep 26, 2016

This is an issue of the optimizer.

To avoid the miniffier to remove the whitespaces, group the affected element with parenthesis. That workarrounds the issue.

margin-left: calc((50%) - 480px);

Source: StackOverFlow

@nineoclick

This comment has been minimized.

Copy link

commented Feb 9, 2017

Encountered this bug and found a solution that works great for me.
My setup is: PhpStorm with LESS and YUI Compressor CSS watchers. So usually I work with a less file, that outputs to css file and then YUI Compressor as watcher minifies the css.

My solution

Less File:

.@{css_root}-mainContentRight {
  -webkit-box-sizing: border-box;
  -moz-box-sizing: border-box;
  box-sizing: border-box;
  display: inline-block;
  height: 200px;
  width: calc(~"100% -  " @mainContentDivDefaultMargin*4 + @mainContentLeftWidth);
  background: #bacd63;
  padding: @mainContentDivDefaultPadding;
  margin: @mainContentDivDefaultMargin;
  -webkit-border-radius: 3px;
  -moz-border-radius: 3px;
  border-radius: 3px;
}

Now: on width rule, I put two spaces between the minus sign AND one space after closing the unescaped string. See below:
width: calc(~"100% -SpaceSpace"Space@mainContentDivDefaultMargin*4 + @mainContentLeftWidth);

CSS not compressed that results from LESS

.jimlibmus-mainContentRight {
  -webkit-box-sizing: border-box;
  -moz-box-sizing: border-box;
  box-sizing: border-box;
  display: inline-block;
  height: 200px;
  width: calc(100% -   220px);
  background: #bacd63;
  padding: 5px;
  margin: 5px;
  -webkit-border-radius: 3px;
  -moz-border-radius: 3px;
  border-radius: 3px;
}

width: calc(100% -SpaceSpaceSpace220px);

Phpstorm watcher see changes in css file and then starts YUI. The result:

.jimlibmus-mainContentRight{-webkit-box-sizing:border-box;-moz-box-sizing:border-box;box-sizing:border-box;display:inline-block;height:200px;width:calc(100% - 220px);background:#bacd63;padding:5px;margin:5px;-webkit-border-radius:3px;-moz-border-radius:3px;border-radius:3px}

Now YUI compresses it right.

width:calc(100% -Space220px);

It's simple to become familiar with this multiple-space practice, and helps me a lot not to banging my head on the desk.

Hope this helps.

@himanshuwt

This comment has been minimized.

Copy link

commented Sep 5, 2017

Still Facing Issue.
Please tell me work around for this

.target {
width: calc(100% + 20px);
}
turns into
.target{width:calc(100%+20px)}

instead of

.target {width: calc(100% + 20px);}

@manishmp7

This comment has been minimized.

Copy link

commented Sep 5, 2017

facing same issue
I'm using "2.4.8" version.

Team, Please fix it or tell me how can i fix it without (- -) solution

@svaponi

This comment has been minimized.

Copy link

commented Sep 19, 2017

Compression examples:

calc(50% - 3px)      => calc(50% - 3px)   // VALID
calc(50% + 3px)      => calc(50%+3px)     // INVALID
calc(50% + +3px)     => calc(50%++3px)    // INVALID
calc((50%) + (3px))  => calc((50%)+(3px)) // INVALID
calc(50% + (+3px))   => calc(50%+(+3px))  // INVALID
calc(50% + (3px))    => calc(50%+(3px))   // INVALID
calc(50% - (-3px))   => calc(50% -(-3px)) // INVALID
calc(50% - -3px)     => calc(50% - -3px)  // VALID

@PeterLenahan

This comment has been minimized.

Copy link

commented Nov 1, 2017

Before compression spaces are there
.file-item {
width: calc(100vw - 40px);
height: calc(((100vw - 40px) * .73) + 40px);
}
After compression, the spaces are missing.

.file-item{width:calc(100vw - 40px);height:calc(((100vw - 40px) * .73)+40px)}

@sunnybear

This comment has been minimized.

Copy link

commented Apr 18, 2018

Any news on this? YUI Compressor breaks CSS calc for 5 years!

@himanshuwt

This comment has been minimized.

Copy link

commented Apr 18, 2018

@sunnybear agree with you. 5 years done. And, still I faced the issue "+" then again i replace it with calc(x - - y). :D

@sunnybear

This comment has been minimized.

Copy link

commented Apr 18, 2018

Does anybody have a list of issues with calc? We've faced with
100% + 6px -> 100%+px
100% - (10% * 6) -> 100% -(10% * 6)
It seems also
(100vw - 40px) * .73) + 40px -> (100vw - 40px) * .73)+ 40px

@MarjanAshofteh

This comment has been minimized.

Copy link

commented Jun 4, 2018

@beaktor
using two - beside + works for me too.
thanks

PaulCapron added a commit to PaulCapron/pwa2uwp that referenced this issue Nov 14, 2018

Work around a YUICompressor 2.4.8 bug in CSS calc
See yui/yuicompressor#59

The spec mandates spaces around ‘+’ and ‘-’. Chrome is lax on that rule tho,
but Firefox (and possibly others) ain’t!
@ramarro123

This comment has been minimized.

Copy link

commented Nov 15, 2018

it's yui compressor still mantained?

siovene added a commit to astrobin/astrobin that referenced this issue Mar 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.