@import rules shouldn't be resolved in inline css.

CommentFileSizeAuthor
#13 cssinlineresolve.patch3.52 KBcasey
#9 cssinlineflatten.patch3.62 KBcasey
#8 cssinlineflatten.patch2.99 KBcasey
#6 cssinlineflatten.patch2.62 KBcasey
#3 cssinlineflatten.patch3.83 KBcasey
cssinlineflatten.patch2.72 KBcasey

Comments

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Please add more information so that everybody will understand the whole context. Give an example of the problem and explain what you expect, what happens, and why what happens is wrong.

Subscribing, thanks.

casey’s picture

casey commented 
$css = <<<EOS
@import url(stylesheet-that-should-not-be-resolved.css);
EOS;

drupal_add_css($css, 'inline');

I don't think it's proper behavior to resolve @import rules and replace it with the actual contents in inline css. This wasn't done in previous drupal versions either.

Currently above example will resolve the stylesheet with its actual contents.

casey’s picture

casey commented
StatusFileSize
new cssinlineflatten.patch3.83 KB

Patch with test

scroogie’s picture

scroogie commented

But why isn't it proper behavior?

casey’s picture

casey commented
Priority: Normal » Critical

Drupal 7 works different than previous versions; @import rules where never resolved in inline css before. I really think this should be discussed.

casey’s picture

casey commented
StatusFileSize
new cssinlineflatten.patch2.62 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, cssinlineflatten.patch, failed testing.

casey’s picture

casey commented
Status: Needs work » Needs review
StatusFileSize
new cssinlineflatten.patch2.99 KB
casey’s picture

casey commented
StatusFileSize
new cssinlineflatten.patch3.62 KB

Uhh I lost part of the patch... this one is complete.

Status: Needs review » Needs work

The last submitted patch, cssinlineflatten.patch, failed testing.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Priority: Critical » Normal
Issue tags: +CSS

As per Priority levels of Issues, this is not critical.

+++ modules/simpletest/tests/common.test	8 Feb 2010 11:04:34 -0000
@@ -563,9 +563,12 @@
     $css_preprocessed = '<style type="text/css">' . drupal_load_stylesheet_content($css, TRUE) . '</style>';

Shouldn't this include $flatten = FALSE?

Drupal 7 works different than previous versions; @import rules where never resolved in inline css before. I really think this should be discussed.

I agree that this should be discussed, however I don't really see why this behavior is unwanted ... The @import rules are valid in inline stylesheets, and if we resolve those in stylesheet files why shouldn't we do the same in inline styles? Can you explain what the disadvantages are to resolving these @imports and why we should not be doing it?

Powered by Dreditor.

casey’s picture

casey commented

caching for one

casey’s picture

casey commented
StatusFileSize
new cssinlineresolve.patch3.52 KB
casey’s picture

casey commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, cssinlineresolve.patch, failed testing.

StevenWill’s picture

StevenWill commented

I also have concerns about using @import. I am guessing the change in Drupal 7 to using @import is because of IE's limitation on reading more than 31 style sheets. After stumbling upon this issue, posted by casey, I looked more closely at how Drupal 7 is handling style sheets. When CSS aggregation is not enabled Drupal separates style sheets into sections of @import, allowing Drupal to include more than 31 style sheets. Once CSS aggregation is turned on the style sheets are imported using link! I my opinion the modifications to Drupal 7 is providing the best option for the configuration and I am guessing this was the rational for making the change.

I would imagine most sites would be using the @import during development and once development is complete the site would switch to aggregating CSS and therefore switch to the link method.

This seems like a closed issue.

StevenWill’s picture

StevenWill commented
Status: Needs work » Closed (works as designed)

Updating status.