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

CSS3 media queries should be supported. #108

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

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Apr 6, 2015

CSS styles including { *border-radius: a / b; } syntax are not being 'minified':
e.g.

  .border8 {
    -moz-border-radius: 36px / 12px;
    border-radius: 36px / 12px;
  }

What version of the product are you using? X-Mod-Pagespeed: 0.9.8.1-215

On what operating system? Debian GNU/Linux

Which version of Apache? Apache/2.2.9 

URL that exhibits the problem:

http://www.the-art-of-web.com/css/border-radius/?ModPagespeed=off
http://www.the-art-of-web.com/css/border-radius/

Original issue reported on code.google.com by dun...@chirp.com.au on 22 Nov 2010 at 11:27

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks for the report, I suspect that this is a syntax we cannot currently 
parse. Is this a CSS3 syntax? Or is it also a CSS2 syntax?

When we get a find a CSS file we cannot parse correctly, we do not try to do 
anything with it, to make sure we don't break it.

Original comment by sligocki@google.com on 22 Nov 2010 at 4:41

  • Changed state: Accepted
  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

border-radius is a CSS3 property, but it has been (more or less) finalised:
http://www.w3.org/TR/2010/WD-css3-background-20100612/#the-border-radius

I've also identified some other (proprietary) CSS3 styles that aren't
being recognised by your parser.  They're a bit more complicated:

- background gradients

  fieldset {
    background: -webkit-gradient(linear, 0 80%, 0 100%, from(white), to(#eee));
    background: -moz-linear-gradient(top, white 80%, #eee);
  }

  input[type=submit] {
    background: #f7fafc -webkit-gradient(linear, left top, left bottom, from(#fff), to(#dae6f1));
    background: #f7fafc -moz-linear-gradient(top, #fff, #dae6f1);
  }

- input placeholder formatting for WebKit

  input::-webkit-input-placeholder {
    color: #ababab;
  }

The parser does recognise the Mozilla equivalent, with a single colon:

  input:-moz-placeholder {
    color: #ababab;
  }

Original comment by dun...@chirp.com.au on 22 Nov 2010 at 4:52

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks. Our parser is not yet CSS3 capable. But we'd like to improve that. 
These examples are helpful to keep around for when we get a chance to look into 
this.

And if you happen to find that any of your styles get broken by CSS 
minification, please open a new bug for them.

Original comment by sligocki@google.com on 22 Nov 2010 at 5:03

  • Changed title: Parse CSS3 syntax (at least commonly accepted parts)
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I've run tests on a selection of our style sheets and collected the lines that 
weren't being parsed into a single file (attached).  Barring any typos, they 
should all be valid syntax.

Original comment by dun...@chirp.com.au on 27 Nov 2010 at 1:22

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Issue 120 has been merged into this issue.

Original comment by jmara...@google.com on 1 Dec 2010 at 11:43

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

From Maksim, common things that we can't parse:

1)  The star hack. This looks like:
*width: 500px;  --- it's discarded by normal parsers, but handled as
width: by some IE versions.
Extremely common.

2)  Some browser-proprietary property values:
cursor: hand; cursor: col-resize; cursor: not-allowed; (IE)
display: -moz-inline-box; display: -moz-inline-stack; (Gecko)

3)  IE filter mess:
_filter:progid:DXImageTransform.Microsoft.AlphaImageLoader(src='http://t.douban.
com/pics/nav/ui_sl_bn.png',
sizingMethod='scale');
filter:alpha(opacity=60);
filter:Alpha(Opacity=0);
filter:mask();

4) Some CSS3 stuff --- I think there is an issue on that already?
rgba() color syntax (semi-common, and @font-face with  src:local()
[only saw it once]

5) There is also an another parser hack, that looks like this:
margin-top: 8px\9; --- not sure what to make of it.

Original comment by sligocki@google.com on 5 Jan 2011 at 7:28

  • Changed title: Parse CSS3 and common proprietary syntaxes
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Trunk revision r449, allowed many proprietary values (like -moz-foo-bar). More 
coming.

Original comment by sligocki@google.com on 10 Feb 2011 at 6:58

  • Changed state: Started
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Star hack (http://en.wikipedia.org/wiki/CSS_filter#Star_hack) supported in r459.

Original comment by sligocki@google.com on 14 Feb 2011 at 11:56

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Parse CSS3 Pseudo-elements with :: like body::selection in r462.

Original comment by sligocki@google.com on 15 Feb 2011 at 11:33

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Parse generic functions in r468.

Ex: -webkit-gradient(linear, 0 80%, 0 100%, from(white), to(rgb(123, 47, 104)))

Original comment by sligocki@google.com on 18 Feb 2011 at 7:19

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Parse Microsoft proprietary syntax in r473.

Ex: filter:progid:DXImageTransform.Microsoft.Alpha(Opacity=80)

Original comment by sligocki@google.com on 22 Feb 2011 at 3:08

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by sligocki@google.com on 24 Feb 2011 at 9:48

  • Added labels: release-note
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

From bug 220:

-webkit-transition-property:opacity,-webkit-transform;

Notice the comma.

See: https://developer.mozilla.org/en/CSS/-moz-transition-property
and http://www.webkit.org/blog/138/css-animation/

Original comment by sligocki@google.com on 28 Feb 2011 at 4:46

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

We also don't parse parameterized pseudo-selectors:

div:nth-child(1n) { color: red }

Original comment by sligocki@google.com on 19 Apr 2011 at 8:56

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Progress report on this:

 - we continue to add capability to our css3 parsing and it's improved but it's
   a moving target.

 - we have another plan to optimize CSS files that have unrecognized constructs in them, leaving the unrecognized constructs as literals

Original comment by jmara...@google.com on 6 May 2011 at 2:40

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Also, most of the cases we cannot parse are actually illegal CSS, using hacks 
to target specific browsers. If we cannot preserve those hacks, we fail to 
parse the document.

Original comment by sligocki@google.com on 6 May 2011 at 4:05

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I'm experiencing a problem where mod_pagespeed does not seem to be processing 
some CSS files, and I suspect it may be because of this issue. The only reason 
I can think of (mentioned in this issue) is rgba(). Is that implemented yet? Is 
there any way to tell why mod_pagespeed is not processing these files? I have 
been unable to find any logs pertaining to this, and no documentation for how 
to adjust log levels etc. or whether this is even possible.

Original comment by dkeyh...@googlemail.com on 30 Sep 2011 at 11:04

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

With the newest versions of mod_pagespeed, you can set "LogLevel debug" in 
Apache and this will print context to where the CSS parser failed to parse CSS 
files. It's not the nicest user interface, but I can help you interpret the 
result if it's not clear.

rgba() should be working with the most recent release (see 
http://code.google.com/p/modpagespeed/source/browse/trunk/src/net/instaweb/rewri
ter/css_filter_test.cc#389). To get the most up-to-date info on what is and is 
not parsed, see good_examples and fail_examples here: 
http://code.google.com/p/modpagespeed/source/browse/trunk/src/net/instaweb/rewri
ter/css_filter_test.cc#145 and examples and parse_fail_examples here: 
http://code.google.com/p/modpagespeed/source/browse/trunk/src/net/instaweb/rewri
ter/css_filter_test.cc#247

Original comment by sligocki@google.com on 30 Sep 2011 at 3:15

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Several updates have been pushed into the CSS parser to allow it to pass 
through troublesome constructs verbatim when we cannot parse them.

Original comment by sligocki@google.com on 24 Jan 2012 at 10:34

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Since this bug was reported a lot of improvements were made to the CSS parser, 
however CSS3 is still not a ratified standard so it's difficult for us to claim 
we support it (and we're not even close to be honest).

We now pass through many non-standard constructs verbatim so we can still 
optimize the remainder of the file; hopefully this is sufficient to handle CSS3 
files until the standard is ratified.

Marking this fixed for now. If a specific construct is discovered that we are 
not handling, please paste it specifically.

Original comment by matterb...@google.com on 7 Feb 2012 at 3:46

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

From issue 407:

If you create a stylesheet with the below content, the CSS isn't processed.

@font-face {
 font-family: 'RokkittRegular';
 src: url('fonts/rokkitt-webfont.eot');
}

@media queries are problematic as well.

@media (max-width: 800px) {
   #main #content {
     margin: 0 7.6%;
     width: auto;
 }
}

[Fri Mar 30 13:48:28 2012] [info] [mod_pagespeed 0.10.21.2-1381 @21277] 
http://foo.com/:19: CSS parsing error in 
http://foo.com/wp-content/themes/theme/style.css

Using mod_pagespeed 0.10.21.2-1381

Original comment by sligocki@google.com on 30 Mar 2012 at 9:05

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I'm uncomfortable with release-noting this as fixed.  I think we should do a 
more thorough job of (a) skipping over unrecognized constructs and (b) handling 
 media-tags properly since they seem to be common before marking this as 
'fixed'.

Another way of measuring this is whether we can properly rewrite the css for 
commonly used UI packages.

Original comment by jmara...@google.com on 22 May 2012 at 7:59

  • Changed state: Started
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 22 May 2012 at 7:59

  • Removed labels: release-note
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 22 May 2012 at 8:00

  • Added labels: Milestone-v23
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Hi,

Following CSS3 syntax is also causing pagespeed to barf

-webkit-transition: border linear 0.2s, box-shadow linear 0.2s;
     -moz-transition: border linear 0.2s, box-shadow linear 0.2s;
      -ms-transition: border linear 0.2s, box-shadow linear 0.2s;
       -o-transition: border linear 0.2s, box-shadow linear 0.2s;
          transition: border linear 0.2s, box-shadow linear 0.2s;

not sure if this is a know issue. Listing it here anyway. 
I got latest source code from trunk and compiled it and ran the parser against 
the css file.

Let me know if you need anymore details

Thanks! 

Original comment by yeshwant...@gmail.com on 13 Aug 2012 at 6:12

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Another one

    -webkit-box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.1), 0 1px 0 rgba(255, 255, 255, 0.1);
       -moz-box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.1), 0 1px 0 rgba(255, 255, 255, 0.1);
            box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.1), 0 1px 0 rgba(255, 255, 255, 0.1);

This makes me think it could be an issue with css properties which take 
multiple set of values.

Original comment by yeshwant...@gmail.com on 13 Aug 2012 at 6:35

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Hi yeshwanthgreddy, note that we will still successfully rewrite CSS files 
which contain these CSS3 constructs.

We fail to parse them, but we are able to recover and pass those sections 
through verbatim.

As you point out, the reason these cause problems is that they have 
comma-separated values (in addition to space-separated values, the CSS2 
standard).

Original comment by sligocki@google.com on 13 Aug 2012 at 6:48

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Hi sligocki,

Its true that it recovers from parse errors but it clears all the pagespeed 
cache each time it encounters a parse error. This is being further amplified in 
a clustered env and causing pagespeed to behave as if it was the first pass and 
loosing all the optimizations. I tried turning off rewrite_css, inline_css, 
outline_css filters and enabling just move_css_above_scripts,move_css_to_head 
in config but does seems to work. It still is trying to parse the files and 
failing every 5mins. Is there any way we can turn parsing of css files but just 
combine and cache them?

Original comment by yeshwant...@gmail.com on 13 Aug 2012 at 9:09

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

This should NOT be clearing your pagespeed cache every time there is an error. 
However, it will fail to combine CSS files if they have unparseable regions 
(like the ones you specified). I don't think there is any way to disable this 
behavior, sorry. But it is not clear to me why you think it is clearing your 
cache each time. Could you explain this in a little more detail. A link to your 
site would also help me to understand.

Original comment by sligocki@google.com on 13 Aug 2012 at 9:19

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

We have changed the log level on our servers and tailed the log file for 
"error" and were running ruby script to hit the page 300 times, get the output 
and scrape the html output for speedified (copy righted :) ) objects and output 
the count and this would run every 20s and we have been seeing a pattern that 
every 5mins when it polls to check for the changes it would throw an parse 
error in the log and the count for speedified object count to go to 1 or 2. 
This is behind a load balancer across 2 servers internally.

We have enabled pagespeed on one of our pages on production on a clustered env 
and were testing it at http://www.webpagetest.org/  and we are seeing for every 
10 passes we are seeing atleast 3-5 calls failing to use pagespeed 
optimizations. 

Here is the url you can hit: http://www.phoenix.edu/beta.html

Original comment by yeshwant...@gmail.com on 13 Aug 2012 at 9:52

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

This behavior is unrelated to this bug. I have forwarded this to the 
mod-pagespeed-discuss email list.

Original comment by sligocki@google.com on 14 Aug 2012 at 4:37

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Hi sligocki,

Thanks for looking into the issue and forwarding it to mod-pagespeed-discuss 
email list. Now how do I get hold of the discussion thread though? I have 
searched and could not find one.

Original comment by yeshwant...@gmail.com on 14 Aug 2012 at 9:02

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Oops, didn't actually press send :/ You should get the email now. Here's the 
discuss list (you need to be a member to reply): 
https://groups.google.com/forum/?fromgroups#!forum/mod-pagespeed-discuss

Original comment by sligocki@google.com on 14 Aug 2012 at 9:06

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Summary was: Parse CSS3 and common proprietary syntaxes

CSS3 is not a finalized standard so we cannot really close the bug as 
previously summarized.  We are now scoping this bug down to support of CSS3 
media tags, which we believe were responsible for the most of the issues with 
our parser. 

Once this is fixed, further CSS issues should get a new bug.

Original comment by jmara...@google.com on 17 Sep 2012 at 7:36

  • Changed title: CSS3 media tag format should be supported.
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 17 Sep 2012 at 7:36

  • Changed title: CSS3 media queries should be supported.
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

CSS @media queries should now be parsed and minified.

Original comment by sligocki@google.com on 31 Oct 2012 at 8:40

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Using the new Beta mod_pagespeed microsoft progid works but this media query 
fails: "@media only screen and (max-device-width: 1024px)".

Was it meant to work?

Original comment by simon.w...@naturalint.com on 7 Nov 2012 at 9:28

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

This will work as of v23, which has been branched now in SVN but is not yet 
released.  We're working on it.

Original comment by jmara...@google.com on 7 Nov 2012 at 12:43

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Great!

Should it work using the beta rpm?

When will the next release be?

Original comment by simon.w...@naturalint.com on 7 Nov 2012 at 1:01

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