Problem/Motivation

When aggregated/optimized CSS contains @import rules, if the file isn't resolved, the rule is stripped completely.

@import url('my-file.css?a-query-string');

in this case my-file.css is excluded.

Proposed resolution

Strip query parameters prior to checking if a file exists.

Remaining tasks

User interface changes

API changes

Original report by @casey

When a stylesheet is aggregated that contains an @import rule that can't be resolved at runtime (e.g. one that is dynamicly generated like imagecache generates image derivatives), the @import rule is removed. This isn't proper behaviour.

CommentFileSizeAuthor
#24 css-aggregation-imports-646862-19.patch3.21 KBmgifford
#19 css-aggregation-imports-646862-19.patch3.21 KBjhedstrom
#19 css-aggregation-imports-646862-19-TEST-ONLY.patch2.46 KBjhedstrom
#16 protoindependent.patch604 bytesXenopathic
#12 snapshot.png5.23 KBvalthebald
#5 cssimportresolvefail.patch2.25 KBcasey
#4 cssimportresolvefail.patch2.34 KBcasey
#1 cssimportresolvefail.patch920 bytescasey

Comments

casey’s picture

casey commented
Status: Active » Needs review
StatusFileSize
new cssimportresolvefail.patch920 bytes
casey’s picture

casey commented

Example

mystylesheet.css

@import url(not-in-file-system-existing-stylesheet.css);
body {
  background: drupalblue;
}

drupal_add_css('path/to/mystylesheet.css');

Results in:

body {
  background: drupalblue;
}

"@import url(not-in-file-system-existing-stylesheet.css);" should however not be removed as it is not properly resolved and replaced by its contents.

kkaefer’s picture

kkaefer commented
Status: Needs review » Needs work

can we have a test for this?

casey’s picture

casey commented
Status: Needs work » Needs review
StatusFileSize
new cssimportresolvefail.patch2.34 KB

With test.

casey’s picture

casey commented
StatusFileSize
new cssimportresolvefail.patch2.25 KB

Reroll

casey’s picture

casey commented

bump

casey’s picture

casey commented

#5: cssimportresolvefail.patch queued for re-testing.

casey’s picture

casey commented

Still applies

aspilicious’s picture

aspilicious commented

Just out of curiosity

Why is

+@import url(non-existing-file.css);
+EOS;

pushed to the left side without spacing?

casey’s picture

casey commented

That's how the Heredoc syntax works; else the string will include whitespace, which it shouldn't.

Patch still applies by the way.

casey’s picture

casey commented

#5: cssimportresolvefail.patch queued for re-testing.

valthebald’s picture

valthebald
Location Sofia
commented
Version: 7.x-dev » 8.x-dev
Status: Needs review » Closed (cannot reproduce)
StatusFileSize
new snapshot.png5.23 KB

Does it still apply?
I used the following files:

include1.css:

@import url(include2.css);
div.content {background-color: red}

include2.css:
div.content { font-size:200%}

and finally, in module:

/**
 * page callback
 */
function test_callback() {
  drupal_add_css(drupal_get_path('module', 'test') . '/include1.css');
  return 'It worked';
}

Result (text is both red (include1.css) and enlarged (include2.css))
snapshot.png
Tested with the latest 8.x and 7.x, with and without CSS aggregation

Wtower’s picture

Wtower commented
Issue summary: View changes
Status: Closed (cannot reproduce) » Active

Sorry to reopen this after so long but I believe the issue is still there.

The original post states specifically that this happens when the css is not in the filesystem, so your test case with include2.css is not valid.

This also happens to me with the following statement in one of my theme's css files.

/* https://www.google.com/fonts#UsePlace:use/Collection:Ubuntu:500,700|Open+Sans:400,700 */
@import url(//fonts.googleapis.com/css?family=Ubuntu:500,700|Open+Sans:400,700&subset=latin,greek);

It seems that the aggregate version ignores the above statement where the url protocol is ommited.

kpaxman’s picture

kpaxman commented

I'm having the same issue with protocol-independent URLs. (I want to use protocol-independent URLs so that the site works over both HTTP and HTTPS without complaining about mixed content.)

This works when aggregated:
@import url(https://fonts.googleapis.com/css?family=Open+Sans:400,700,400italic,700italic,300,300italic,600,600italic);

This gets removed when aggregated:
@import url(//fonts.googleapis.com/css?family=Open+Sans:400,700,400italic,700italic,300,300italic,600,600italic);

rjbautista’s picture

rjbautista commented

Same issue with Wtower and kpaxman, subscribing...

Xenopathic’s picture

Xenopathic commented
StatusFileSize
new protoindependent.patch604 bytes

A quick fix for protocol-independent URLs, enhancing the regex to not match '//...' as well as all URLs of the format 'abc://...'

tjerkdekuijper’s picture

tjerkdekuijper commented

I confirm #14, this worked for me on a drupal 7.23 install, using with or without css aggregetion.

nvaken’s picture

nvaken
he/him
Primary language Dutch
commented

Same here, #14 is a valid fix for us.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
StatusFileSize
new css-aggregation-imports-646862-19-TEST-ONLY.patch2.46 KB
new css-aggregation-imports-646862-19.patch3.21 KB

I think this is the proper issue. This happens when query strings are added to the @import as well. For instance:

@import url('my-file.css?a-query-string');

Attached is a patch along with a test that illustrates the failure.

The last submitted patch, 19: css-aggregation-imports-646862-19-TEST-ONLY.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Component: base system » asset library system
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue summary: View changes
gaele’s picture

gaele commented
Related issues: +#2014851: Drupal CSS preprocessing breaks protocol-relative paths
mgifford’s picture

mgifford
he/him
Primary language English
commented
StatusFileSize
new css-aggregation-imports-646862-19.patch3.21 KB

re-uploading patch #19 for the bots.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: CSS aggregate removes @import rules when not properly resolved. » CSS aggregate removes @import rules when not properly resolved
Status: Needs review » Needs work 
+++ b/core/tests/Drupal/Tests/Core/Asset/css_test_files/css_input_with_import.css
@@ -2,6 +2,7 @@
+@import "import3.css?because-sometimes-this-is-added";
 @import url("http://example.com/style.css");
 @import url("//example.com/style.css");

Can we then also add an external CSS file that has a querystring? We should verify that those don't get imported.

Once that test coverage exists, I think we have sufficient assurances to commit this.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Related issues: +#646246: Only move non-resolved @import rules to top of aggregate stylesheet when also at top in own stylesheet, +#646244: Only @import rules at top of aggregated stylesheets should be resolved., +#2936067: CSS aggregation fails on many variations of @import

Wow! There are a lot of issues related with @import management.
Cross linking a few.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

bradjones1’s picture

bradjones1
Primary language English
commented
Status: Needs work » Closed (duplicate)

I think this would be resolved by #2936067: CSS aggregation fails on many variations of @import.