I enabled the css preprocessor and certain parts of the Meta theme broke. At the top of the generated preprocessed css file was this:

@import /Drupal/sites/default/themes/meta/meta-paper/url('/Drupal/sites/default/themes/meta/meta-paper/../css/pagestructure.css');@import /Drupal/sites/default/themes/meta/meta-paper/url('/Drupal/sites/default/themes/meta/meta-paper/../css/pagestyles.css');/* $Id: node.css,v 1.2 2006/09/05 03:50:56 unconed Exp $ */

It should also be noted that those two files are imported by the individual css files (style.css in 4 different folder for four different color scheme; meta-paper is one of the folders) as follows:

@import url('../css/pagestructure.css');
@import url('../css/pagestyles.css');

I manually edited the preprocessed css file to the correct syntax and the theme works as usual. Regex not extracting @import properly?

CommentFileSizeAuthor
#8 convert_import_syntax.patch1.25 KBedmund.kwok
#1 import_regex.patch871 bytesedmund.kwok

Comments

edmund.kwok’s picture

edmund.kwok commented
Title: CSS preprocessor breaks Meta theme » CSS preprocessor breaks theme using @import url('filename')
Status: Active » Needs review
StatusFileSize
new import_regex.patch871 bytes

It seems that the regex that is supposed to look for @import that doesn't use url() matches those with url() also:

$data .= preg_replace('/@import\s*([\'"]?)(?![a-z]+:)/i', '@import \1'. $path . '\2', $contents);

Not sure if the regex's purpose was to match @import 'file', @import "file", and @import file. But with that, @import url() gets matched also. Proposed that ([\'"]?) be changed to ([\'"]+) if the purpose is to match valid @import syntax that does not use url().

Changing title as css preprocessor might break other themes that are using @import url().

edmund.kwok’s picture

edmund.kwok commented
Title: CSS preprocessor breaks theme using @import url('filename') » CSS preprocessor breaks theme using @import url()
dvessel’s picture

dvessel commented

I noticed this when reviewing the css aggregator patch. m3vrk wanted to keep it similar to how color.module does it.

Your right. It doesn't consider @import with url(..). Both are legal, it just doesn't understand it both ways.

BioALIEN’s picture

BioALIEN commented

What theme are you using?

Maybe this should be set to critical?

dvessel’s picture

dvessel commented

Don't think it's critical. I'd say, more of a feature to support both options. You just have to make sure your not using url(). Instead just use @import "../path/file.css";

edmund.kwok’s picture

edmund.kwok commented

Main purpose of these process is to convert relative paths to absolute ones. url() were already converted in the line before; thus the regex was meant for @import that don't use url() as mentioned in the code comments. 

// Fix any @import that don't use url() and is not absoslute.

Oh, I just noticed the code comment has a typo :p I think using url() is fine, but the regex needs some tweaking. I'm using the Meta theme as mentioned in #1. Don't think this is critical either; it only breaks certain theme and if css preprocessor is enabled.

Steven’s picture

Steven commented

@import "foo.css" and @import url("foo.css") are both valid CSS and mean exactly the same thing. Why not precede the url() regexp with one that wraps all @import arguments in url(), and get rid of this dodgy one we have now?

edmund.kwok’s picture

edmund.kwok commented
StatusFileSize
new convert_import_syntax.patch1.25 KB

+1 on the suggestion. At least we can standardize the @import syntax in the process. Attached patch converts @import "file", @import 'file' and @import file to @import url("file").

Tested on:

Garland - no @import statements => No change
Meta - @import url('file') => No change
Minnelli - @import "file" => Converted to @import url("file")

dvessel’s picture

dvessel commented

Haven't tested the patch but does it convert from@import url(...). You mentioned it converts to that style but does it work the other way around? I must have missed something as that was the main issue here.

Steven’s picture

Steven commented
Status: Needs review » Fixed

Tested and verified. I tweaked the replace string so it doesn't insert an extra semi-colon. Committed to HEAD, thanks.

bradlis7’s picture

bradlis7 commented
Version: 5.x-dev » 5.0-rc1
Status: Fixed » Active

I'm using drupal-rc1, and my theme was using a statement like this:

@import "base.css";

But, the file was not being imported, until I changed it to:

@import url("base.css");

I'm not sure what the deal is, but I don't think this is fixed.

edmund.kwok’s picture

edmund.kwok commented
Status: Active » Postponed (maintainer needs more info)

Hmm, what theme are you using specifically? If possible, can you take a look at the css file generated in files/css and see what's the difference in the @import statement in the first line, before and after you changed the @import syntax.

bradlis7’s picture

bradlis7 commented

There was no @import in the generated file at all, so I'm not sure what the deal was. Like I said, it simply had

@import "base.css";

I was working on the B7 theme, but I changed it to use the @import url() syntax, and it worked.

edmund.kwok’s picture

edmund.kwok commented

I can't seem to reproduce the error. I tried disabling and enabling the css preprocessor for B7, B7 Brown and B7 Green. B7 used @import url("base.css"); while B7 Brown and B7 Green used @import "../base.css";. All of them worked fine and each of the generated preprocessed css file in files/css directory had the proper @import url("absolute path/base.css") syntax at the top.

bradlis7’s picture

bradlis7 commented

I'm not sure what the problem was, but this is the page with the change I made: http://cvs.drupal.org/viewcvs/drupal/contributions/themes/B7/style.css?r... .

edmund.kwok’s picture

edmund.kwok commented

So when B7's main style.css was @import "base.css", you enabled the css preprocessor, and the theme breaks. Then you changed it to @import url("base.css"); and enabled css preprocessor, and now its working. Correct? Can you look in your drupal/files/css directory and see if there are any differences when @import and @import url() was used. The file should be a css file named some 32 random characters, eg 90bd4ab7320dba7267ade209d114bc33.css

bradlis7’s picture

bradlis7 commented
Status: Postponed (maintainer needs more info) » Fixed

Hm, I can't seem to recreate the problem. I think it originally didn't have any kind of "import" at all in the generated file, but when I added url(), it put the import into the file. Unless this happens to someone else, I guess you can consider this fixed.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
morbus iff’s picture

morbus iff
they/them
commented
Version: 5.0-rc1 » 6.x-dev
Status: Closed (fixed) » Active

This seems to be happening on disobey.com:

  • sites/all/themes/network_wireframe/network_disobey/style.css includes @import "../style.css";
  • The cached CSS includes @import url("/sites/all/themes/network_wireframe/network_disobey/../style.css");.
  • This is *first text* in that file, so it DOES NOT end up overriding the system/node.css of core.
  • Changing to @import url("../style.css") had no effect (yes, a new .css cache was rebuilt.
morbus iff’s picture

morbus iff
they/them
commented

This is reproducible using Garland's Minelli - enable it, and make sure CSS caching is on. The first characters of the cache file is "@import url("/themes/garland/minnelli/../style.css");.node-unpublished,", which means that the import happens, then all the declarations are overridden with the inclusion of the system/node stuff.

morbus iff’s picture

morbus iff
they/them
commented
Version: 6.x-dev » 5.0-rc1
Status: Active » Closed (fixed)

Mine's a different a bug. Resetting back.