crowell / modpagespeed_tmp

Automatically exported from code.google.com/p/modpagespeed
Apache License 2.0
0 stars 0 forks source link

Consider rewriting CSS before combining #286

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
No CSS minification is observed (release note stated that rewrite_css should 
minify). CSS is combined but not minified possibly because of some unsupported 
rules as per earlier discussions.

If so, how about:

1. Each file gets processed individually before combining?

OR

2. Combine files and any rule that passes gets rewritten; any that fails, stays 
as is?

Why does this matter?

Anyone who will try to transition to HTML5/CSS3 will experience problems when 
trying to include new features alongside existing CSS, so why not just process 
CSS rules that already work?

By MPS choosing one of the above routes--if possible--one still gets some 
benefit of processing of rules that pass. Also, rules that fail will be clearly 
identifiable in the resulting stylesheet.

MPS 0.9.17.3-679; Fedora Apache worker MPM

Original issue reported on code.google.com by webmas...@clubsilver.org on 8 May 2011 at 6:57

GoogleCodeExporter commented 9 years ago
Additional benefit to this approach is that we do not have to wait for MPS to 
catch-up/lead CSS standards, etc.

Original comment by webmas...@clubsilver.org on 8 May 2011 at 7:29

GoogleCodeExporter commented 9 years ago
Original summary:   CSS processing suggestion

Note that Issue 108 tracks our goal of fully processing CSS files that contain 
constructs we don't understand.

The drawback of this approach is that that we wind up with more 
pagespeed-annotation in the URL.  Today, if we combine 10 CSS files and then 
rewrite them, we'll get 2 ".pagespeed." annotations in the URL.  If we rewrite 
each one first, then combine, we'll get 11.  This can cause us to bump up 
against URL length restrictions imposed by other Apache modules and by IE.

Note that this used to be more severe than it is now: by default Apache has a 
limit of about 250 characters between the slashes, although we found a way to 
work around this in mod_pagespeed.  However, the IE limitations remain, as do 
limitations imposed by any Apache proxies in the path between server and client.

As a result, I'm hopeful that we'll solve the problem by making rewrite_css 
more robust, rather than changing the order.  But I'll leave this open as an 
enhancement for changing the order.

Original comment by jmara...@google.com on 8 May 2011 at 3:18

GoogleCodeExporter commented 9 years ago
OK, I did not see issue 108; perhaps this can be merged into it.

Under 2nd approach you will have,

.iknowyou{minified:rule}.iknowyou2{minified:rule}
.hellostranger{
-passthru: rule;
}
@strange line that did not want to change
#i know.you from somewhere:soaring{minified:rule}

and get to keep annotations at 2. :-)

It is designers responsibility to write valid syntax, so if MPS chooses to just 
pass through syntactically incorrect rules, it can be done. It will also make 
"offending" rules easily noticeable.

Since we on the subject of pagespeed annotations, I raised the issue of file 
naming before. I suggested to drop all .css but the last one 
(http://groups.google.com/group/mod-pagespeed-discuss/browse_thread/thread/98ec7
0bec3bf8931/7fd6ade184d3cf63).

I understood the explanation of why the file naming was designed the way it is. 
On 2nd thought, though, since the file name base remains in the combined name, 
it is still recognizable in Firebug. Anyone should recognize these are combined 
CSS files. :-) And you would save 40 characters on 10 CSS files.

I find file1+file2+file3.xyz.pagespeed.hash.css name to be just as insightful, 
even though I would go all the way to randomfilename.xyz.pagespeed.hash.css :-)

Original comment by webmas...@clubsilver.org on 8 May 2011 at 4:05

GoogleCodeExporter commented 9 years ago

Original comment by jmara...@google.com on 24 May 2012 at 7:29