I turned on CSS aggregation for the first time, and all my background images in the theme disappeared!
Specifically,
body {
background: #fff url(images/headerBg.gif) repeat-x 0 -20px;
}was "compressed" into this:
body{background:#fff url(/sites/all/themes/images/headerBg.gif) repeat-x 0 -20px;}
On closer inspection, drupal somehow converted the url images/headerBg.gif into /sites/all/themes/zen/ew/images/headerBg.gif and then again into /sites/all/themes/images/headerBg.gif. It's the last transformation that was problematic.
After a bit of digging, the problematic code is includes/common.inc, in function _drupal_build_css_path, reproduced here:
/**
* Helper function for drupal_build_css_cache().
*
* This function will prefix all paths within a CSS file.
*/
function _drupal_build_css_path($matches, $base = NULL) {
static $_base;
// Store base path for preg_replace_callback.
if (isset($base)) {
$_base = $base;
}
// Prefix with base and remove '../' segments where possible.
$path = $_base . $matches[1];
$last = '';
while ($path != $last) {
$last = $path;
$path = preg_replace('`(^|/)(?!../)([^/]+)/../`', '$1', $path);
}
return 'url('. $path .')';
}
After a bit of experimentation, it turned out the regular expression is at fault. It forgets to escape the two periods with backslashes. Note that (?!../) refers to the look-ahead negative assertion as described here:
http://ca.php.net/manual/en/regexp.reference.php#regexp.reference.assert...
Because the periods aren't escaped, this regular expression replaces "/zen/ew" by "/". The obvious patch:
Index: includes/common.inc
===================================================================
--- includes/common.inc (revision 1348)
+++ includes/common.inc (working copy)
@@ -1810,7 +1810,7 @@
$last = '';
while ($path != $last) {
$last = $path;
- $path = preg_replace('`(^|/)(?!../)([^/]+)/../`', '$1', $path);
+ $path = preg_replace('`(^|/)(?!\.\./)([^/]+)/\.\./`', '$1', $path);
}
return 'url('. $path .')';
}
The problematic code was introduced in the controversial path for this issue: http://drupal.org/node/113607
A comment by chx even suggested that instead of a regular expression, some php function (perhaps realpath?) should be used.
http://drupal.org/node/113607#comment-500017
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | css-compressor-regex.patch | 590 bytes | Jamie Holly |
Comments
Comment #1
dergachev commentedComment #2
bsdunx commentedI tried to reproduce this with my own zen sub-theme but was not able to, it worked just fine.
body {
background: #fff url(images/test.gif) repeat-x 0 -20px;
}
to
body{background:#fff url(/sites/all/themes/zen/bsdunx/images/test.gif) repeat-x 0 -20px;}
Comment #3
damien tournoud commentedThe bug only occurs when one directory has only two characters (like "ew" in the example above). The solution outlined by ew looks sane. Could someone please roll a proper patch?
Because the bug is also in 7.x-dev, bumping release. Also I don't believe this is as critical as other issues :)
Comment #4
Jamie Holly commentedPatch file attached. I tested it with a two character directory and it converted the URL properly.
Comment #5
kscheirerworked for me, applies cleanly with 14 line offset.
Comment #6
kscheirer#4 and #5 reported success with this patch.
Comment #7
dergachev commentedbump
Comment #8
webchickThat regex is terrifying. Can we please have a comment there that indicates what the heck it's checking so we don't mess it up again?
Comment #9
lilou commentedMark #258846: CSS aggregation fails to parse some url()'s as duplicate.
Comment #10
hass commentedAs one of the drivers of the linked @import patch I can say that I've tested this CSS compressing functions in all "thinkable" variants of URL's... So subscribing here to learn.
Comment #11
Jamie Holly commentedThere's actually a comment about the purpose of the regex a couple of lines above where the patch occurs:
// Prefix with base and remove '../' segments where possible.
All the patch does is simply escape the .'s. It was nothing more than a simple oversight. I'm not really sure how to comment it much better than what's already there (if anyone has a suggestion, I'll gladly make a new patch).
Comment #12
hass commentedIf someone change the directory mangle regex... don't forget to change
$before = preg_replace('`(^|/)(?!../)([^/]+)/../`', '$1', $before);in color.module line 432, please.Comment #13
dergachev commentedTurns out that this was a duplicate of this issue, which was just committed to core.
http://drupal.org/node/258846
However, I'm not sure comment #12 has been addressed.
Comment #14
casey commentedtagging
Comment #15
hargobindsubscribing