CSS aggregator produces invalid code and directory names for @import files Breaks IE, Internet explorer does not style page.

have same issue, just have in original css:

	background-image: url(grdot.gif);

in aggregated:

background-image:url(/sites/all/grdot.gif);

Yes, this seems to work great, although technically you should be allowing white space after the 'url(' and before the pathname as well. See: http://www.w3.org/TR/REC-CSS1/#url

And actually, it doesn't seem like W3C allows whitespace between the 'url' and '('.

This problem exists in D7 as well, so it should probably be addressed there first (and will likely get more attention that way). The patch doesn't apply cleanly there, but it needs some work in any case, I think.

Testing report using patch in #1 on Drupal 6.12...

Here are a couple of errors I was experiencing with CSS Optimization:
- Paths were being stripped or truncated (e.g. url(../images/img.gif) would become url(css/))
- Quote characters would be semi-stripped (e.g. url("images/img.gif") would become url("images/img.gif))

The patch in #1 appears to have fixed this, and the site is now loading correctly.

Thank you!

tagging

This is a *really* bad bug that wasted *a lot* of my time today.

We found this using jQuery UI, it's a wonder it hasn't been found by many people.

To illustrate what the description says, if you have a css file like this:

ui.all.css:

@import ui.theme.css

and
ui.theme.css

.ui-icon{background-image: url(images/icon.png);}

it will become (in the aggregated file):

.ui-icon{background-image: url(path/to/aggregated/css/./{nextCSS rule

This could be harmless in FF, but actually causes IE to not render the CSS at all! I took many hours of debugging to figure this out - and then the issue was easier to find., so let's prevent that in the future.

Attached is a patch for Drupal 7, I didn't test the D6 patch, but this patch is just a copy of it, and looks RTBC.

Obviously we're missing some test coverage here.

+  // Determine the correct directory (we don't need . as current directory).
+  $directory = dirname($filename) == '.' ? '' : dirname($filename) .'/';

This could easily be written without the double dirname.

+  // Alter all url() paths, but not external. We don't need to normalize
+  // paths here (remove folder/... segements) because that will be done later.
+  return preg_replace('/url\s*\(([\'"]?)(?![a-z]+:|\/+)/i', 'url(\1'. $directory, $file);

Comment should also state that it's not altering absolute paths.

I resurrected some @import tests rfay wrote in #472820: drupal_load_stylesheet_content() removes whitespace improperly, creating invalid CSS and added the line Jacob pointed out is breaking. They pass with the patch, and fail without it. Yay tests. Please credit rfay on commit. I also incorporated casey's feedback and improved some comments.

Excellent! Thanks for the quick turnaround on those tests, ksenzee (and rfay!)

Committed to HEAD.

Oops. And this needs porting to 6.x.

Here's a D6 port. I included a line that was in kkaefer's original D6 patch but that Jacob left out of the D7 version. It's an edit to the regex at line 1928 of common.inc (inside drupal_build_css_cache()) that allows whitespace in a path like "url (someimage.jpg)". Without this edit, paths have to look like "url(someimage.jpg)". Jacob, was that intentional because we fixed it elsewhere in D7?

The "Save" and "Attach" buttons are very close together. :)

That last patch is very close but needs a minor cleanup. I would do it myself but I don't know how to create a patch file yet.

The change is on line 30 of that file:

+  return preg_replace('/url\s*\(([\'"]?)(?![a-z]+:|\/+)/i', 'url(\1'. $directory, $file);}

...needs to become:

+  return preg_replace('/url\s*\(([\'"]?)(?![a-z]+:|\/+)/i', 'url(\1'. $directory, $file);
+}

And you could even concatenate the comment on line 23 & 24 into a single line comment.

I've tested this code and it works great on my 6.14 site where I regularly get 50K hits a day.

I have rerolled a patch from #14 against latest Drupal 6 dev version. Tested on a site where I have a same problem like on #11 and now css aggregation works.

And here is an answer to #19

I included a line that was in kkaefer's original D6 patch but that Jacob left out of the D7 version. It's an edit to the regex at line 1928 of common.inc (inside drupal_build_css_cache()) that allows whitespace in a path like "url (someimage.jpg)". Without this edit, paths have to look like "url(someimage.jpg)". Jacob, was that intentional because we fixed it elsewhere in D7?

This is mentioned in #6 - it is not allowed to have space between url and (, so a path like "url (someimage.jpg)" is not a valid syntax.

@pivica good call on the spacing issue with "url (".

Tested here too. Now what will it take to roll this into 6.x HEAD?

Hi

I suffered also the jQuery UI CSS Optimization Bug. This is really critical, because Optimization will break the CSS file as soon as you have the jQuery UI Module enabled!

I tested the patch #22 and it fixes the issue. Please commit this patch to the next Drupal release.

Adding new tags

Better use more generic tags.

Please commit this to the next drupal release. Everyone using jQuery UI and CSS Optimization will suffer from wrong css code!

OMG thank you so much. I was going crazy haha. patch from #22 worked :) thanks alot

Thanks for testing, committed.

Yeah! Thanks Gabor!

The patch applies cleanly against Pressflow 6.16 (!):

# patch -p0 < 265719_css_import_fix_D6.patch
patching file includes/common.inc
Hunk #1 succeeded at 2040 (offset 10 lines).

...and also it seems to fix this issue: #833070: Acquia Slate 3.x incompatible with core CSS optimization (whichs appears with Opera 10.53/Win).

Thanks & greetings,
-asb