apache / incubator-pagespeed-mod

Apache module for rewriting web pages to reduce latency and bandwidth.
http://modpagespeed.com
Apache License 2.0
693 stars 157 forks source link

fallback_rewrite_css_urls doesn't apply to unparsed sections #800

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
From Matthew Jacobi:

I'm having trouble getting the fallback_rewrite_css_urls filter to work. My 
understanding based on the docs is that it will rewrite the urls that it founds 
even if it can't parse the css. Now we have some css3 that it can't parse but 
it isn't rewriting the url. I've knocked up a demo page to show it in action:

http://www.pitchup.com/static/pagespeed_test.html

Now the relevant style section:

<style>.button.alt.css3{background:#f5f5f5 
url(http://www.pitchup.com/static/pitchup/images2/xsprites.png.pagespeed.ic.uzcU
4aSnau.webp) right -559px no-repeat;background: 
url('http://www.pitchup.com/static/pitchup/images2/sprites.png') right -559px 
no-repeat,
 -moz-linear-gradient(top, #ffffff, #e6e6e6);background: url('http://www.pitchup.com/static/pitchup/images2/sprites.png') right -559px no-repeat,
 -webkit-gradient(linear, 0 0, 0 100%, from(#ffffff), to(#e6e6e6));background: url('http://www.pitchup.com/static/pitchup/images2/sprites.png') right -559px no-repeat,
 -webkit-linear-gradient(top, #ffffff, #e6e6e6);background: url('http://www.pitchup.com/static/pitchup/images2/sprites.png') right -559px no-repeat,
 -o-linear-gradient(top, #ffffff, #e6e6e6);background: url('http://www.pitchup.com/static/pitchup/images2/sprites.png') right -559px no-repeat,
 linear-gradient(to bottom, #ffffff, #e6e6e6)}.button.alt.css3:hover,.button.alt.css3:focus,.button.alt.css3:active{background:#e6e6e6 url(http://www.pitchup.com/static/pitchup/images2/xsprites.png.pagespeed.ic.uzcU4aSnau.webp) right -508px no-repeat}</style>

Now you can see it has sprites.png twice, once where it parsed and rewrote it 
to a webp but then it left the one that was with the css3 styles alone. This 
causes 2 versions of sprites.png to be downloaded, and with that being our 
sprites it's already a big file.

Have I misunderstood the meaning of fallback_rewrite_css_urls?

Original issue reported on code.google.com by sligocki@google.com on 7 Oct 2013 at 5:38

GoogleCodeExporter commented 9 years ago
Unfortunately fallback_rewrite_css_urls is only active when we failed to parse 
the entire CSS file. When we added that feature, this was the only/most common 
failure mode.

In your case we are recovering the parse (and just failing to parse a section 
of the CSS). This is what usually happens currently. However, we have not 
updated fallback_rewrite_css_urls to deal with this situation.

Original comment by sligocki@google.com on 7 Oct 2013 at 5:39

GoogleCodeExporter commented 9 years ago
So would a temporary work around be to put css3 extensions into a separate .css 
file?

Original comment by dalo...@gmail.com on 8 Oct 2013 at 8:51

GoogleCodeExporter commented 9 years ago
Here are some things you could do to work around this (None seem particularly 
great, though, sorry :/):

1) ModPagespeedDisallow 
http://www.pitchup.com/static/pitchup/images2/sprites.png
Pro: All versions of that sprite are served with the same URL.
Con: It's the non-rewritten version.

2) Move all CSS3 to a separate file and ModPagespeedDisallow that CSS file.
Pro: All versions of sprite served from same URL (unless you have other refs to 
sprite).
Con: It's still the non-rewritten version.

3) Move all CSS3 to a separate file and add something that will make it 
unparseable. For example append "{" (a single unclosed bracket) to the end of 
the CSS3 file. This will cause the parser to fail for the entire file and 
fallback_rewrite_css will act.
Pro: All versions sprite served from same URL + that is the optimized URL.
Con: Kind of an ugly hack, may break things, depends on undocumented behavior 
of CSS parser.

Original comment by sligocki@google.com on 8 Oct 2013 at 9:59

GoogleCodeExporter commented 9 years ago
An update on what workaround we went for, from our UX person:

Hmm, none of them are great solutions.

I must admit I'm not keen on 3 (or any of them!), just have a feeling 3) will 
give us problems with browsers that are strict with parsing css (IE and Safari 
etc). It's a hack.

I think the best option (until pagespeed can properly parse css3) is simply 
remove the combined background attributes (image and gradient) - afaik it's 
only the map button and arrow button on the homepage - they can be dropped for 
a standard background image for now, with no real loss of visual impact.

Original comment by dalo...@gmail.com on 17 Oct 2013 at 4:09

GoogleCodeExporter commented 9 years ago
Yes, that would be a much better workaround if you don't need those features.

Original comment by sligocki@google.com on 17 Oct 2013 at 5:30