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

rewrite_css filter incompatible with CSS3 media queries #50

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 6 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Apr 6, 2015

CSS3 media queries get destroyed with the rewrite_css filter using 0.9.0.0-128.

For instance, this CSS:

@media screen and (max-width: 290px), screen and (max-device-width: 290px) {
    .selector {
        display: none;
    }
    .otherselector {
        display: block;
    }
}


turns into this after passing through the rewrite_css filter:

@media screen,and,screen,and{.selector{display:none}}@media 
screen,and,screen,and{.otherselector{display:block}}


Instead, it should be written like this I believe:

@media screen and (max-width:290px),screen and 
(max-device-width:290px){.selector{display:none};.otherselector{display:block}}


These media queries are very important for developing pages that render nicely 
on a wide range of devices (e.g. phones), so I hope that this gets fixed soon! 
In the meantime, I have disabled the rewrite_css filter.

Original issue reported on code.google.com by joe.lencioni on 8 Nov 2010 at 8:35

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks for the bug report & you hit on the right workaround.  We'll get this 
assigned and fixed as soon as possible.

Original comment by jmara...@google.com on 8 Nov 2010 at 8:38

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks for the fast response!

Original comment by joe.lencioni on 8 Nov 2010 at 8:39

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks for the bug report, our parser does not support all of CSS3 right now. 
However, our goal is to not break good content, so we're working on noticing 
this situation and not breaking it.

Original comment by sligocki@google.com on 8 Nov 2010 at 11:38

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

That makes a lot of sense. Thanks!

Original comment by joe.lencioni on 9 Nov 2010 at 1:55

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

These should be fixed by 
http://code.google.com/p/modpagespeed/source/detail?r=175 which also adds 
checks for this situation.

If you build from source, please let me know if this fixes you. If you install 
from binary, we'll send an announcement to the list when the next binary is 
released.

Thanks for the fix, Yi-An!

Original comment by sligocki@google.com on 9 Nov 2010 at 11:17

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

If you build from source, be sure to build from the trunk, not the branch that 
was cut on Nov 8.

E.g.

gclient config http://modpagespeed.googlecode.com/svn/trunk/src

Original comment by jmara...@google.com on 9 Nov 2010 at 11:31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment