If a css file is included with an @import statement and that file is not in the same directory as the including file,
the base path changes to the path of the included file for futher parsing (which is desired).

When the included file is through, parsing continues in the including file but the base path is still
the one from the included file. This is a bug.

As a simple example let's assume the following files

/www/themes/mytheme/css/main.css
/www/themes/mytheme/css/a/a.css
/www/themes/mytheme/css/b/b.css

With main.css as follows:

/*
* Base dir is '/www/themes/mytheme/css/'.
* The following import will work as expected.
*/
@import url(a/a.css);
/*
* Base dir is now '/www/themes/mytheme/css/a/'.
*
* The following import will not work!
*/
@import url(b/b.css);
/*
* Instead this would work:
*/
@import url(../b/b.css);

This can be fixed by introducing a stack in drupal_load_stylesheet() (includes/common.inc) that saves and restores the base paths
for each recursive call.

Patch follows.

Files: 
CommentFileSizeAuthor
#34 drupal_load_stylesheet-1198904-34.patch7.17 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 40,439 pass(es).
[ View ]
#32 drupal_load_stylesheet-1198904-32.patch4.65 KBdas-peter
FAILED: [[SimpleTest]]: [MySQL] 40,261 pass(es), 0 fail(s), and 4 exception(s).
[ View ]
#27 d8-CssOptimizer-fix-import-handling-1198904-27.patch4.07 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 57,989 pass(es).
[ View ]
#27 interdiff-1198904-23-27-do-not-test.diff689 bytesdas-peter
#23 d8-CssOptimizer-fix-import-handling-1198904-tests-only.patch2.81 KBdas-peter
FAILED: [[SimpleTest]]: [MySQL] 57,581 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#23 d8-CssOptimizer-fix-import-handling-1198904.patch4.03 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 57,606 pass(es).
[ View ]
#21 drupal_load_stylesheet-1198904-21.patch7.41 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 58,188 pass(es).
[ View ]
#16 drupal_load_stylesheet-1198904-16.patch6.04 KBjox
PASSED: [[SimpleTest]]: [MySQL] 35,052 pass(es).
[ View ]
#11 drupal_load_stylesheet-1198904-11.patch6.04 KBjox
PASSED: [[SimpleTest]]: [MySQL] 35,049 pass(es).
[ View ]
#11 drupal_load_stylesheet-1198904-11-test-only.patch4.9 KBjox
FAILED: [[SimpleTest]]: [MySQL] 35,043 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#9 drupal_load_stylesheet-not-complying-with-standards-1198904-9-d7.patch1.12 KBdas-peter
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal_load_stylesheet-not-complying-with-standards-1198904-9-d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#6 drupal_load_stylesheet-1198904-6.patch6.78 KBjox
PASSED: [[SimpleTest]]: [MySQL] 34,378 pass(es).
[ View ]
#6 drupal_load_stylesheet-1198904-6-test-only.patch4.88 KBjox
FAILED: [[SimpleTest]]: [MySQL] 34,373 pass(es), 4 fail(s), and 0 exception(es).
[ View ]
#3 drupal_load_stylesheet-not-complying-with-standards-1198904-3.patch1.46 KBjox
PASSED: [[SimpleTest]]: [MySQL] 34,378 pass(es).
[ View ]
#1 drupal_load_stylsheet-import-basepath-1198904-1.patch1.44 KBjox
PASSED: [[SimpleTest]]: [MySQL] 34,236 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.44 KB
PASSED: [[SimpleTest]]: [MySQL] 34,236 pass(es).
[ View ]

Is that true? Own issue posts can not be edited? Only comments?

Here is a correction. The line
* Base dir is now '/www/themes/mytheme/css/otherdir/'.
should be
* Base dir is now '/www/themes/mytheme/css/a/'.

Title:drupal_load_stylsheet base path mess with @importdrupal_load_stylsheet() does not comply with standards when using @import
Version:7.x-dev» 8.x-dev
Component:base system» file system
Priority:Normal» Major
StatusFileSize
new1.46 KB
PASSED: [[SimpleTest]]: [MySQL] 34,378 pass(es).
[ View ]

This is six months old now. Giving this another go and targeting D8 first. Changed title to what this is really about.

Is there anything else needed?

Issue tags:+Needs tests

Thanks for the patch! Based on the issue priority documentation, I believe this is a normal bug.

I've read the summary several times and I still don't quite understand the circumstances needed to trigger this bug, so maybe it would help to add some inline comments to the code in the patch. We'll also need an automated test based on the bug that fails without the patch and passes with it. Could you provide steps to reproduce the issue, or a test theme that demonstrates it?

Priority:Major» Normal

StatusFileSize
new4.88 KB
FAILED: [[SimpleTest]]: [MySQL] 34,373 pass(es), 4 fail(s), and 0 exception(es).
[ View ]
new6.78 KB
PASSED: [[SimpleTest]]: [MySQL] 34,378 pass(es).
[ View ]

Thanks for your input!

Ok, sorry, it's a bit difficult to describe. I'm trying it more generally:

These are the requirements:

  1. For any (also nested) @import statement, the path that is used as base path (or current directory) to resolve a relative url, must be the path (directory) of the file containing the @import statement.
  2. This base path must not be modified by any preceding @import statements (in the same file).

Both cases are not garanteed in the current state.

There is already some tests checking drupal_load_stylesheet() (System -> CSS Unit Tests). It's funny though (not really), the tests contain some serious mistakes:

  1. Wrong filenames are given to drupal_load_stylesheet() in two places.
  2. One variable is named incorrectly.
  3. The content of one "expected output" file (to compare the results with) is faulty. But since it gets compared to itself (see 1.) the test will always succeed anyway.

So I fixed those and added one round (4 tests) for my case. Also added some inline comments.

A patch with everything and one with only the (fixed/new) tests is attached.

Is there still need for "steps to reproduce the issue, or a test theme that demonstrates it"?

Status:Needs review» Needs work

The last submitted patch, drupal_load_stylesheet-1198904-6-test-only.patch, failed testing.

Status:Needs work» Needs review

Fail was expected.

StatusFileSize
new1.12 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal_load_stylesheet-not-complying-with-standards-1198904-9-d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Just hit this problem in D7 and solved it slightly different.
I don't see a need for an additional static variable. Because the function runs recursive it should be just fine to reset the base path to the parent one as soon as the file is processed.
All I've added is another variable to keep the parent base path and reset the base path later with it.

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new4.9 KB
FAILED: [[SimpleTest]]: [MySQL] 35,043 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
new6.04 KB
PASSED: [[SimpleTest]]: [MySQL] 35,049 pass(es).
[ View ]

@das-peter: Yes you're right, thanks. Just slapped my forhead.

I lost the focus on the recursiveness somehow due to the nested function calling. And since we're recursive we can implicitly use _that_ stack (simply by using a local variable).

One more slap.

This issue is set to D8. That's why your patch failed.

I updated my patch against current 8.x-git, which also includes the fixed test cases.

Status:Needs review» Needs work

The last submitted patch, drupal_load_stylesheet-1198904-11-test-only.patch, failed testing.

Issue tags:-Needs tests

+++ b/core/includes/common.incundefined
@@ -3576,17 +3576,22 @@ function drupal_load_stylesheet($file, $optimize = NULL, $reset_basepath = TRUE)
+  // Set the current base path to process prossible child imports.  ¶

Bit of trailing whitespace here.

Untagging as it has a test. Thanks!

@jox Thanks for re-rolling :)
Did something change with the testbot? I added "-d7" as suffix to the patch name. This should exclude the patch from testing as documented: "...Patches are skipped if the file name (before the .patch or .diff extension) ends with something like -d5 or _d4. For example somefile-d9.patch and somefile_d2.patch is skipped..."

Advertising :P : Drupal Code Sniffer really helps to avoid coding standard violations and thus annoying patch re-rolls just because of silly whitespaces.

Thanks for finding that documentation, which is no longer accurate. This was recently changed to "do-no-test" which is supposed to make a lot more sense. :)

Updated the docs. The description on the actual filefield is correct.

StatusFileSize
new6.04 KB
PASSED: [[SimpleTest]]: [MySQL] 35,052 pass(es).
[ View ]

#13

Status:Needs work» Needs review

Any news on this? This is especially annoying when working with CSS preprocessors, like LESS.

Some related issues:
#1350646: @import from another directory not being included
#1331144: Multiple @import does not use expected pathing

Status:Needs review» Needs work
Issue tags:+needs backport to D7, +Needs reroll

Looks like the patch in #16 is a year stale -- the first step is to update it to apply to the current 8.x branch. The test changes will need to be made manually, as all tests have moved around following the PSR-0 conversion.

Issue tags:-Needs reroll

Removing reroll tag

Status:Needs work» Needs review
StatusFileSize
new7.41 KB
PASSED: [[SimpleTest]]: [MySQL] 58,188 pass(es).
[ View ]

Here's a re-rolled version.

Status:Needs review» Needs work

This needs to be re-rolled based on CssOptimizer. @see #352951: Make JS & CSS Preprocessing Pluggable

Status:Needs work» Needs review
StatusFileSize
new4.03 KB
PASSED: [[SimpleTest]]: [MySQL] 57,606 pass(es).
[ View ]
new2.81 KB
FAILED: [[SimpleTest]]: [MySQL] 57,581 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here we go.
Patch with tests only should fail.
Full patch should pass.

Status:Needs review» Needs work
Issue tags:-needs backport to D7

The last submitted patch, d8-CssOptimizer-fix-import-handling-1198904.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+needs backport to D7

Status:Needs review» Needs work
Issue tags:+CSS aggregation

Awesome!

Only a tiny nitpick. Please change:

+      // Subfolder file. Tests:
+      // - CSS import path is properly interpreted.

to:
+      // File in subfolder. Tests:
+      // - CSS import path is properly interpreted. (https://drupal.org/node/1198904)

Then it's consistent with the other test cases. Once that is changed, I'll RTBC :)

Status:Needs work» Needs review
Issue tags:-CSS aggregation
StatusFileSize
new689 bytes
new4.07 KB
PASSED: [[SimpleTest]]: [MySQL] 57,989 pass(es).
[ View ]

@Wim Leers: Thanks for the review!
And here we go, nitpick fixed.

Status:Needs review» Reviewed & tested by the community
Issue tags:+CSS aggregation

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed/pushed to 8.x.

Moving to 7.x for backport.

Yay, thanks! :)

Status:Patch (to be ported)» Needs review
StatusFileSize
new4.65 KB
FAILED: [[SimpleTest]]: [MySQL] 40,261 pass(es), 0 fail(s), and 4 exception(s).
[ View ]

D7 Backport.

Status:Needs review» Needs work

The last submitted patch, drupal_load_stylesheet-1198904-32.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.17 KB
PASSED: [[SimpleTest]]: [MySQL] 40,439 pass(es).
[ View ]

Oh, looks like I missed some parts :)

Version:7.x-dev» 7.9
Component:file system» forms system
Status:Needs review» Reviewed & tested by the community

This is totally breaking Zen 7.x-5.3 when used with just CSS and not the Sass. See #2058113: choosing "Aggregate and compress CSS files" disrupts styling

The patch in #34 totally fixes the problem. And the code changes to drupal_load_stylesheet() look good to go.

Version:7.9» 7.x-dev
Component:forms system» file system

Not sure how the version/component got changed. fixing.

Title:drupal_load_stylsheet() does not comply with standards when using @importdrupal_load_stylsheet() fails to load @import files in different directories

Yay :)

When will this go into core?

Does this also fix the following case,

Fails when aggregation is enabled (works without):

@import url("//fonts.googleapis.com/css?family=Aclonica:400,700");

Always works:

@import url("https://fonts.googleapis.com/css?family=Aclonica:400,700");

(Difference is // versus https://)

Issue summary:View changes

Fixed wrong path in example.

Issue summary:View changes

Further confirmation that #34 fixes the problem with regards to Zen. Looking forward to seeing this fix in the next core release.

Further confirmation that #34 fixes the problem with regards to Zen. Looking forward to seeing this fix in the next core release.

Title:drupal_load_stylsheet() fails to load @import files in different directoriesdrupal_load_stylesheet() fails to load @import files in different directories

Correcting typo in title for improved searchability.

Status:Reviewed & tested by the community» Fixed

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

In case this may help anyone who's styling seems a bit broken after the upgrade...

My theme uses LESS and lots of @import statements. After the upgrade to 7.25 some of my styling did not come through to the page and after lots of digging I found this issue. In order to fix my problem I have had to go through my LESS files and correct some of the @import statements - although they all worked in 7.24 some of them did not reference correctly in 7.25 and therefore the files were not imported.

melissadvh, could you give us more info? What were the compiled CSS @import commands coming out as?