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

CSS Minification - 0 followed by a unit is rewritten to 0 without a unit. #1538

Closed
dhaupin opened this issue May 17, 2017 · 3 comments
Closed

CSS Minification - 0 followed by a unit is rewritten to 0 without a unit. #1538

dhaupin opened this issue May 17, 2017 · 3 comments

Comments

@dhaupin
Copy link

@dhaupin dhaupin commented May 17, 2017

I ran into this today with mod_pagespeed 1.11.33.4-0 that is packaged in cPanel experimental repo.

When using CSS minifier, 0 followed by a unit, such as 0px, is re-written into 0 without a unit. This breaks some CSS directives that always require a unit. Example is a dynamic construction of CSS that drops a 0px into a calc() when an offset is not required.

Example:
left: calc(50% - $offset);

Since its not always used, $offset is set to 0px by default, which renders as:
left: calc(50% - 0px);

Upon minification, Pagespeed truncates the px resulting in:
left: calc(50% - 0);

This is invalid and causes errors. Besides Pagespeed, this issue seems to be found in other minification applications as well. Is there any possibility to always include the unit of measurement? The spec claims unit is optional, so perhaps there should be a config for it rather than assuming "no unit" on rewrite: https://www.w3.org/TR/css3-values/#lengths

Thanks.

@jmarantz
Copy link
Contributor

@jmarantz jmarantz commented May 17, 2017

I think this is fixed in mod_pagespeed 1.12. The source-code was fixed here:

7c30b7e

Can you switch to the beta channel? We should be promoting this to stable soon.

@dhaupin
Copy link
Author

@dhaupin dhaupin commented May 18, 2017

Nice, that is a similar issue. Don't have much time this eve to check further, but that looks like that commit is dealing with percents and seconds. Its def the same kinda hiccup, however this is every unit that CSS requires specifically in areas like calc(). Like transition, its invalid to use a non-unit 0 and invalids the style rule.

As far as the beta, I personally would be down, but in this case, we are relying on cPanel experimental branch for "EA4", which pulls down a stable release they have gone through. Wish i had time to go through and try to figure out a pull, but its super busy lately. Maybe i can do a vanilla beta on a new instance without cPanel.

Anyways, thanks for your time looking into this. Appreciate it.

@Suranex
Copy link

@Suranex Suranex commented Jul 3, 2017

We use mod_pagespeed version 1.12.34.2-0 on Debian and have still this issue. calc(100% - 0px); is optimized to calc(100% - 0).

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

Successfully merging a pull request may close this issue.

None yet
3 participants