Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

css_combine overly pessimistic (should combine even if CSS cannot be fully parsed) #565

Closed
GoogleCodeExporter opened this issue Apr 6, 2015 · 5 comments

Comments

@GoogleCodeExporter
Copy link

With the new 1.1.23.1-beta release, I'm seeing various entries like these in my 
log:

[info] [mod_pagespeed 1.1.23.1-2169 @20563] Failed to combine 
https://www.example.com/wp-admin/css/wp-admin.css?ver=3.4.2 because of parse 
error.
[info] [mod_pagespeed 1.1.23.1-2169 @20563] Cannot combine: not combinable

It appears that some CSS 3 syntax is still not supported.

----

 $ wget http://core.trac.wordpress.org/export/22550/tags/3.4.2/wp-admin/css/wp-admin.dev.css
 $ /usr/local/src/modpagespeed/src/out/Release/css_minify_main wp-admin.dev.css > /dev/null
 # /usr/local/src/modpagespeed/src/out/Release/css_minify_main wp-admin.dev.css > /dev/null
 CSS unparseable sections mask 134
 Ignoring chars in value. at byte 91346 "...ition-property: left, right, top, bottom..."
 Failed to parse values for property -webkit-transition-property at byte 91347 "...tion-property: left, right, top, bottom,..."
 Ignoring chars in value. at byte 91417 "...on-property:    left, right, top, bottom..."
 Failed to parse values for property -moz-transition-property at byte 91418 "...n-property:    left, right, top, bottom,..."
 Ignoring chars in value. at byte 91488 "...n-property:     left, right, top, bottom..."
 Failed to parse values for property -ms-transition-property at byte 91489 "...-property:     left, right, top, bottom,..."
 Ignoring chars in value. at byte 91559 "...-property:      left, right, top, bottom..."
 Failed to parse values for property -o-transition-property at byte 91560 "...property:      left, right, top, bottom,..."
 Ignoring chars in value. at byte 91630 "...operty:         left, right, top, bottom..."
 Failed to parse values for property transition-property at byte 91631 "...perty:         left, right, top, bottom,..."
 Could not parse selector: illegal char ~ at byte 127470 "...en;
 }

 input.newtag ~ div.taghint {
         vis..."
 Failed to parse selector at byte 127484 "...ewtag ~ div.taghint {
         visibility: visib..."
 Could not parse selector: illegal char ~ at byte 127530 "...
 input.newtag:focus ~ div.taghint {
         vis..."
 Failed to parse selector at byte 127544 "...focus ~ div.taghint {
         visibility: hidde..."

... and also ...

 $ wget http://core.trac.wordpress.org/export/22550/tags/3.4.2/wp-includes/css/admin-bar.dev.css
 $ /usr/local/src/modpagespeed/src/out/Release/css_minify_main admin-bar.dev.css > /dev/null
 CSS unparseable sections mask 130
 Ignoring chars in value. at byte 10314 "...tion-property: width, background;
         -webk..."
 Failed to parse values for property -webkit-transition-property at byte 10315 "...ion-property: width, background;
         -webki..."
 Ignoring chars in value. at byte 10437 "...tion-property: width, background;
         -moz-..."
 Failed to parse values for property -moz-transition-property at byte 10438 "...ion-property: width, background;
         -moz-t..."
 Ignoring chars in value. at byte 10553 "...tion-property: width, background;
         -o-tr..."
 Failed to parse values for property -o-transition-property at byte 10554 "...ion-property: width, background;
         -o-tra..."

----

Reference issue 108.

Original issue reported on code.google.com by amat...@gmail.com on 12 Nov 2012 at 10:56

@GoogleCodeExporter
Copy link
Author

Summary was: CSS 3 parser errors aren't skipped over

It will skip over these unparseable sections in normal CSS minification, 
however, they will stop CSS combining because we ran into issues where if one 
CSS file was actually broken, combining with the rest would break them too.

However, this is overly pessimistic. We should be able to parse the structure 
well enough to successfully combine even when there are unparseable sections.

Original comment by sligocki@google.com on 12 Nov 2012 at 11:02

  • Changed title: css_combine overly pessimistic (should combine even if CSS cannot be fully parsed)
  • Changed state: Accepted
  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect

@GoogleCodeExporter
Copy link
Author

Forgot to mention, css_minify_main was built from revision 2173.

Original comment by amat...@gmail.com on 12 Nov 2012 at 11:02

@GoogleCodeExporter
Copy link
Author

Issue 566 has been merged into this issue.

Original comment by sligocki@google.com on 12 Nov 2012 at 11:03

@GoogleCodeExporter
Copy link
Author

This issue was closed by revision r2274.

Original comment by sligocki@google.com on 7 Dec 2012 at 11:44

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

I have a fix checked into svn trunk (that will go into the next release). I 
don't know that it specifically solves your combiner case, but it should make 
the CSS combiner much more lenient. Now it will only fail to combine CSS files 
which are really broken (like mismatched {}s or trailing chars at the end, etc.)

Original comment by sligocki@google.com on 7 Dec 2012 at 11:46

  • Added labels: Milestone-v24, release-note

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant