IE: Stylesheets ignored after 30 link/style tags

hass - March 1, 2008 - 18:06
Project:Drupal
Version:7.x-dev
Component:base system
Category:bug report
Priority:critical
Assigned:Unassigned
Status:patch (code needs work)
Description

There is an important issue with the style sheets we are adding. I have been run into this issue at RTL layout development and lost many hours on figuring out - why the IE layout was broken and finally found http://support.microsoft.com/kb/262161.

We should change the way how we add CSS files to the page to solve this issue. Please keep in mind - since we added RTL support, the RTL people will be the very first running into this issue, but others with many modules installed will run into the same issues. This bug makes development VERY difficult and impossible, while i cannot turn on css aggregation in development.

If we go the following way we can keep the single files, but we end up with "one" style tag only - what finally solves the issues!

<style type="text/css" media="all">
@import "/drupal6/sites/all/themes/mytheme/node.css";
@import "/drupal6/sites/all/themes/mytheme/default.css";
@import "/drupal6/sites/all/themes/mytheme/admin.css";
@import "/drupal6/sites/all/themes/mytheme/test1.css";
@import "/drupal6/sites/all/themes/mytheme/test2.css";
@import "/drupal6/sites/all/themes/mytheme/test3.css";
@import "/drupal6/sites/all/themes/mytheme/test4.css";
@import "/drupal6/sites/all/themes/mytheme/test5.css";
</style>

#1

Trendy - March 3, 2008 - 10:39

Hello,
The problem, I had just made. Imports of over 37 individual css files from different modules. The IE7 has not completely accepted the layout. If you build a big side wants to many modules, such problems arise here soon. I think there is a fundamental solution required, as well as module developers CSS files.

greetings Trendy

#2

dman - March 3, 2008 - 11:23

I guess the fundamental solution is turn on css aggregation. It'll make your pages load faster too.
It's a bitch for developing ... but some form of css aggregation is the answer to this bug one way or another anyway.

#3

hass - March 3, 2008 - 14:32

The above code example will solve this in fundamental manner... developing a site CSS is nearly impossible with aggregate and compress activated!

#4

dman - March 3, 2008 - 14:53

True 'nuff.
Reading the issue, I see that the problem is only with the number of tags in the doc, not the actual number of stylesheets, (which I thought may have been a more understandable limitation of the code)!

#5

hass - March 3, 2008 - 17:24

Well, it counts the number of <style></style> and <link type="text/css" ... /> not the number of files added with @import... so you can add 1000 @import files inside a <style></style> without any problems. 1000 is untested, but it solved the limitation troubles at 30 for me...

#6

brahms - April 25, 2008 - 12:36

I totally agree with this request to use only one style tag per media type to solve the issue with the IE limitation. Using css aggregation is definititely no solution for me, because I am using private file download method and for security reasons I do have to use this. Looking at the function drupal_get_css() in common.inc, it does not seem to be much work to implement the request of hass (by the way: thank you very much hass for prividing the excellent yaml-for-drupal theme to the drupal community!). Maybe it would make sense to use a new variable to control whether to use one style tag per media type or to use one style tag per css file import as it is done now. I hope drupal's core team will accept this request, I don't like core patches at all and this would be needed otherwise.

#7

brahms - April 29, 2008 - 15:49

Here is a workaround for this issue for Drupal 5.7 (I know this may not be the right place because this issue is written for Drupal 6.1 but maybe someone has enough time to port my modification of 5.7 to 6.1...)

First I defined a new system variable "combine_css" which can be enabled under admin/settings/performance. The code for this is at the end of function system_performance_settings() (right after the form element for "preprocess_css") in the file system.module:

<?php
$form
['bandwidth_optimizations']['combine_css'] = array(
   
'#type' => 'radios',
   
'#title' => t('Combine CSS files per media type'),
   
'#default_value' => intval(variable_get('combine_css', FALSE)),
   
'#options' => array(t('Disabled'), t('Enabled')),
   
'#description' => t("Internet Explorer ignores css stylesheets after 30 link/style tags (see this <a href=\"http://support.microsoft.com/kb/262161\" title=\"IE: Stylesheets ignored after 30 link/style tags\" rel=\"nofollow\"> issue</a> on drupal.org. By enabling this option all import links for css files are combined into one &lt;style&gt; tag per media type to reduce the number of tags."),
  );
?>

Finally I modified the function drupal_get_css() in the file common.inc. If the system variable combine_css is enabled, drupal_get_css() combines the import commands for css stylesheets per media type into one <style> tag. No preprocessing of module and theme stylesheets is respected. If combine_css is disabled, everything works like it did without my modifications.

<?php
function drupal_get_css($css = NULL) {
 
$output = '';
  if (!isset(
$css)) {
   
$css = drupal_add_css();
  }

 
$preprocess_css = variable_get('preprocess_css', FALSE);
 
$combine_css = variable_get('combine_css', FALSE);

 
$directory = file_directory_path();
 
$is_writable = is_dir($directory) && is_writable($directory) && (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC) == FILE_DOWNLOADS_PUBLIC);

  foreach (
$css as $media => $types) {
   
// If CSS preprocessing is off, we still need to output the styles.
    // Additionally, go through any remaining styles if CSS preprocessing is on and output the non-cached ones.
   
if ($combine_css) {
     
$no_module_preprocess_media = '';
     
$no_theme_preprocess_media = '';
     
$output_media = '';
    }
    foreach (
$types as $type => $files) {
      foreach (
$types[$type] as $file => $preprocess) {
        if (!
$preprocess || !($is_writable && $preprocess_css)) {
         
// If a CSS file is not to be preprocessed and it's a module CSS file, it needs to *always* appear at the *top*,
          // regardless of whether preprocessing is on or off.
         
if (!$preprocess && $type == 'module') {
            if (
$combine_css) {
             
$no_module_preprocess_media .= '@import "'. base_path() . $file .'";' ."\n";
            } else {
             
$no_module_preprocess .= '<style type="text/css" media="'. $media .'">@import "'. base_path() . $file .'";</style>' ."\n";
            }
          }
         
// If a CSS file is not to be preprocessed and it's a theme CSS file, it needs to *always* appear at the *bottom*,
          // regardless of whether preprocessing is on or off.
         
else if (!$preprocess && $type == 'theme') {
            if (
$combine_css) {
             
$no_theme_preprocess_media .= '@import "'. base_path() . $file .'";' ."\n";
            } else {
             
$no_theme_preprocess .= '<style type="text/css" media="'. $media .'">@import "'. base_path() . $file .'";</style>' ."\n";
            }
          }
          else {
            if (
$combine_css) {
             
$output_media .= '@import "'. base_path() . $file .'";' ."\n";
            } else {
             
$output .= '<style type="text/css" media="'. $media .'">@import "'. base_path() . $file .'";</style>' ."\n";
            }
          }
        }
      }
    }

    if (
$is_writable && $preprocess_css) {
     
$filename = md5(serialize($types)) .'.css';
     
$preprocess_file = drupal_build_css_cache($types, $filename);
      if (
$combine_css) {
       
$output .= '@import "'. base_path() . $preprocess_file .'";'. "\n";
      } else {
       
$output .= '<style type="text/css" media="'. $media .'">@import "'. base_path() . $preprocess_file .'";</style>'. "\n";
      }
    }

    if (
$combine_css && $no_module_preprocess_media) {
     
$no_module_preprocess_media = '<style type="text/css" media="'. $media .'">'. $no_module_preprocess_media .'</style>'. "\n";
     
$no_module_preprocess .= $no_module_preprocess_media;
    }
    if (
$combine_css && $no_theme_preprocess_media) {
     
$no_theme_preprocess_media = '<style type="text/css" media="'. $media .'">'. $no_theme_preprocess_media .'</style>'. "\n";
     
$no_theme_preprocess .= $no_theme_preprocess_media;
    }
    if (
$combine_css && $output_media) {
     
$output_media = '<style type="text/css" media="'. $media .'">'. $output_media .'</style>'. "\n";
     
$output .= $output_media;
    }
  }

  return
$no_module_preprocess . $output . $no_theme_preprocess;
}
?>

I tested this code without CSS aggregation and a yaml-for-drupal theme. It works in IE 7 and Firefox 2.0.0.14. Now IE 7 processes all CSS stylesheets!

#8

te-brian - May 5, 2008 - 19:59

Hey All,

For those of you weary about messing with core includes, we were able to find a workaround for this problem that doesn't solve the problem at a core level, but does work for the time being.

The lead up to this revelation came from our initial instinct to write a custom module that would replace drupal_get_css with our own get_css function (which we would then have to call in our template.php). This custom module would combine like media types into one style tag and hopefully solve the 30+ sheets problem. Well, looking at the code for drupal_get_css made us actually look at how drupal_add_css works and this is when we first noticed the fact that drupal_add_css already has a parameter that allows you to mark a css to not be preprocessed (read: aggregated).

So... we simply make sure that all drupal_add_css calls in our template.php are told not to preprocess (note: we are using the zen theme as our starting point):

drupal_add_css(path_to_subtheme() .'/layout.css', 'theme', 'all', $preprocess_theme);

$preprocess_theme is set to FALSE at the top of our template.php so that it will only need to be changed once when it is time to aggregate all css.

Then, just make sure that css aggregation is turned ON in the system settings. Now, all the core and module css style sheets will be aggregated and you can control which template style sheets are not for development purposes. This strategy will solve the 30+ style sheet IE problem up until the point that your theme has more than 30 style sheets (and you could preprocess theme style sheets that are "stable" to free up more space). The added benefit is that development is a bit quicker because the page loads much quicker with all the module and core style sheets aggregated.

Hope this helps someone,
Brian

#9

hass - May 5, 2008 - 20:27

@Brian: i don't like to duplicate nor support hacked core functions swapped in a custom module... it would be much better to get core fixed. If someone would write a well working and core worthy clean patch I'm sure that the core maintainers will not refuse this request. We might have a good chance to get this into D6.3...

I know i maintained the @charset core bugfix in D5 times and this one have been fixed somewhat quickly, too. People affected by this issue could replace the common.inc or patch themself if we have a working common.inc. I'm reluctant about adding an additional module and changing all core API function to mymodule_add_css().

I will not answer on any support requests about such hacks.

#10

hass - May 5, 2008 - 21:54
Status:active» patch (code needs review)

Here is a 5 line changing patch that seem to fix the issue.

Try it out and get the core maintainers eyes on this case :-)

AttachmentSize
common_style_limit_HEAD_2008050501.patch2.35 KB
common_style_limit_D6_2008050501.patch2.32 KB
common_style_limit_D5_2008050501.patch2.19 KB

#11

hass - May 6, 2008 - 20:11
Status:patch (code needs review)» patch (code needs work)

Damn, the media declaration breaks IE6 and below. Selfhtml.org is no trustworthy resource...

#12

Gábor Hojtsy - May 8, 2008 - 09:20

Well, hass, as suggested above, we could still do grouped @imports' with media="$media" specified in the <style> tag. So one <style> tag per one media type. That should avoid IE6 issues?

#13

Paratio - May 12, 2008 - 18:18

Hello Everyone,
I think a fundamental solution to managing CSS in drupal core would be fine.
There should be a way to use "css aggregation" when private filesystem is turned on because of performance reasons. My programming skills aren't good enough to make this possible. I'm working on my language improvement. On PHP and my english skills too :-)

I like the suggested solution of a grouped "style@import" because I found another big problem with IE5 with the "link-import" of D6. I solved this problem by changing the $styles var in page.tpl.php. Now I added the grouping function.

My workaround is placed in the template by changing the $styles in the page.tpl.php calling an own function ( <?php print _mychange_css($styles) ?> instead of <?php print $styles ?> ).

The following snippet (Drupal 6-Version) have to be placed in the used template.php:

<?php
function _mychange_css($oldstyles) {

global
$mycatch_media_all; // globals are needed to handle the preg-results outside the replacement-routine
global $mycatch_media_rest;
$mystyles = $mycatch_media_all = $mycatch_media_rest = ""; //reset of used vars

function _mycatch_css_f ($match) {  // the callback-function for the preg-replacement-routine
global $mycatch_not_rest;
global
$mycatch_media_all;
    if (
$match[1] == "all") { $mycatch_media_all .= '@import "'."$match[2]".'.css";'. "\n"; }
    else {
$mycatch_media_rest .= '<style type="text/css" media="'."$match[1]".'">@import "'."$match[2]".'.css";</style>' . "\n"; }
return
''; // return nothing for replacement
}

$oldstyles = preg_replace_callback('#<link type="text/css" rel="stylesheet" media="(.*?)" href="(.*?)\.css(.*?)/>\n#','_mycatch_css_f', $oldstyles ); // the preg-replacement-routine. "#" seems to be needed for cutting "<" an ">" (found on php.net).
$mystyles .= '<style type="text/css" media="all">' . "\n".$mycatch_media_all.'</style>'."\n".$mycatch_media_rest . $oldstyles;
return
$mystyles;
}
?>

Edit: The global-definition of $mycatch_not_rest was wrong (forgot to change after renaming) and I added . "\n" to "rest".

A Drupal 5 -Version needs a regex-pattern like: '#<style type="text/css" media="(.*?)"\>\@import "(.*?).css";\</style>#' (NOT tested).

At this time there is only one group for the most used media type "all". The "rest" gets a "style@import"-form too (not needed in D5). With elseif it's easy to add more groups. By using an array to collect the search results it would be easy to make a group for each media type. There could be other even unknown media types. Think of tools like CSS-Injector (http://drupal.org/project/css_injector) and additional css-files in .info-file.

I found no information about $query_string which is added to the css-import links between line 1717 an 1739 in the common.inc of d6. Can somebody of you explain this? In my solution I don't use this information. You can use this with "$match[3]".

Edit: In a german forum someone told me that $query_string is for cache-rebuilding in browser.

Yours sincerely
Carsten Logemann

#14

Paratio - May 12, 2008 - 21:58

Here the function (Drupal 6) with grouping all media-types in an array and adding the $query_string:

<?php
function _mychange_css($oldstyles) { // get the content of $styles-var from page.tpl.php

 
global $mycatch_media// globals are needed to handle the search results ...
 
global $myquery_string; // ... outside the preg_replace -routine
 
$mycatch_media = $mystyles = $myquery_string = ""; //reset of used vars

 
function _mycatch_css_f ($mymatch) {  // function for the preg-replacement
   
global $mycatch_media;
    global
$myquery_string;
   
$mycatch_media ["$mymatch[1]"] [] = $mymatch[2];  //add result with media-type as key
   
$myquery_string = $mymatch[3];  // get $query_string
   
return ''; // return nothing for replacement = deleting the <link />-tag in $oldstyles
 
}

 
$oldstyles = preg_replace_callback( // search regular expression in $oldstyles
   
'#<link type="text/css" rel="stylesheet" media="(.*?)" href="(.*?)\.css(.*?)" />\n#',
   
'_mycatch_css_f', // call of function with each preg-result with an array
   
$oldstyles ); // "#" seems to be needed for cutting "<" an ">" found on php.net

 
foreach ($mycatch_media as $mymedia => $mycss_links) // making a grouped <style>@import
 
{
 
$mystyles .= '<style type="text/css" media="'."$mymedia".'">' ."\n"; //group by media-type
   
foreach ($mycss_links as $mycss_link) // get each css-link
   
{
     
$mystyles .= '@import "'."$mycss_link".'.css'."$myquery_string" .'";'."\n";
    }
   
$mystyles .= '</style>' ."\n";
  }
 
$mystyles .= $oldstyles; // add the rest of $styles-var
 
return $mystyles; // return the changed content of $styles to page.tpl.php
}
?>

Edit: inline comment improvement

Successfull tested on my IE 5.

#15

dvessel - June 5, 2008 - 20:33

As Gábor mentions, if there is no issue with style tags grouped by media type, then it's the only solution.

From 5 to 6, the "style" tag was replaced with "link". Anyone recall why it was changed?

#16

hass - June 5, 2008 - 20:42

Well... as i remember - someone said @import was used in past to keep Netscape 4.7.x users outside of a Drupal website and that this is no more the case now ~15 years after Netscape 4.7.x :-). Group by media must keep the order intact or things will break in themes... this could cause more style tags as we expect now and might bring us back to the 30 style tag limit...

#17

dvessel - June 5, 2008 - 20:57

Right, I just found it. I didn't see that as a compelling reason for the change.

The only thing to consider when ordering is what added the style. Core should go in first, then contrib modules, possibly theme engines then themes and maybe sub-themes. The ordering within each group shouldn't be a big deal so at most, we would have 4-5 groups for the same media type.

#18

dvessel - June 5, 2008 - 21:06

btw hass, why did you roll a patch with the media type pushed into the @import? IE no likey. Your original post should do the trick.

#19

hass - June 5, 2008 - 21:14

Not that would break themes. you might end up with X medias per core, modules and themes. And if you must keep the original order intact you will easily exceed the 10 media switches per core, modules, themes. I know some core developers don't care about the order, but CSS order IS VERY important.

#20

dvessel - June 5, 2008 - 21:28

Who uses more than a handful of media types? The most I've encountered is 3-4.

CSS order is important but we shouldn't take it too far. Does Views need to worry about the order set by cck? I don't believe so, each module should only be concerned with the styles it adds itself. Set it's defaults and let the theme worry about the details.

There might be some cases where modules that depend on each other try and alter its style, in that case what's wrong with them working based on specificity instead of cascades? But most times, it shouldn't need to worry about anything but itself or it can get messy.

If there's an alternate method, then what is it? Please, lets keep it simple. :) I found this queue because I ran into this bs 30 style limit. It really needs to be fixed.

#21

hass - June 5, 2008 - 21:54

Well, between modules this overwrites should never happen... and I'm happy that YAML use media=all everywhere and inside the CSS files @media {} what would hopefully not cause anything to break in the order, but there seems only a workaround that reduces the problem, but is no final solution.

Good to have you on this issue... a better chance to get something in :-)

#22

dvessel - June 6, 2008 - 18:00

This is really tricky. Grouping by media types would also mean splitting aggregated files for modules and themes. Something I'd rather not see.

I've been out of it for a while now. Not sure I'll have much weight in this issue but I'm definitely thinking about a fix.

#23

brahms - July 2, 2008 - 16:24

Sorry to say but today I found out that grouped @imports' with media="$media" inside one style tag does not work in IE7 if there are more than 30 grouped CSS files. In my Drupal Site I have now 34 CSS files (after adding another module) inside they style tag for media="all". The first 30 CSS Stylesheets are loaded by IE7 the stylesheets after them are skipped.

So I tried another solution. I replaced the "style" tag with "link" like it is done in Drupal 6.x and this seems to work. I attach a patch file for the official common.inc in Drupal 5.7, please apply this patch from Drupals base directory.

AttachmentSize
common.inc-5.7-css_link.patch2.01 KB

#24

hass - July 2, 2008 - 16:41

This will not work in IE6 as i know. Cannot remember for sure, but I think I've tested this some time ago. IE doesn't make any difference between link as style and inline styles.

#25

brahms - July 3, 2008 - 14:05

If this does not work in IE6 I see primary one solution that may work: I will try to group the stylesheets per media into style tags again but this time I use a maximum of 30 stylesheets per style tag. Maybe this will work, I will post a patch if it succeeds.

Another solution could be this one: we make it possible to aggregate CSS stylesheets even if private download method is set. I have to use private download method becausee I am working on an intranet solution and there is a need for role based restrictions on content and uploaded documents.

By the way: if replacing style tags with link tags does not work in IE6 it means that many people will have troubles using Drupal 6.x and IE6 ?

#26

brahms - July 3, 2008 - 17:31

Here is now a patch for Drupal 5.7 (modifying include/common.inc and modules/system/system.module) for grouping a maximum number of CSS stylesheets inside media specific style tags. I call this feature "Combine CSS" and you find the settings for it at the bottom of the "performance" settings form. There it is possible to disable or enable the "combine CSS" feature and to specify the limit of CSS stylesheets that get grouped into one media specific style tag. It is best to set this limit to 30 to minimize the number of style tags. I have just made short tests with IE7 and disabled CSS aggregation, here it seems to work. I will do more testing with a higher number of stylesheet files in the next weeks.

AttachmentSize
common.inc-5.7-css_styles.patch9.47 KB

#27

brahms - July 3, 2008 - 17:33

Here is an example of the grouped style tags for my site, with the conditional comment for IE7 I get 41 CSS stylesheet files grouped inside 8 style tags and this works in IE7 (and FireFox 2). I am using yaml-for-drupal 3.x and an additional yaml layout:

<style type="text/css" media="all">@import "/ak/intratest/sites/all/modules/admin_menu/admin_menu.css";
</style>
<style type="text/css" media="all">@import "/ak/intratest/sites/all/modules/devel/devel.css";
@import "/ak/intratest/modules/aggregator/aggregator.css";
@import "/ak/intratest/modules/book/book.css";
@import "/ak/intratest/modules/node/node.css";
@import "/ak/intratest/modules/poll/poll.css";
@import "/ak/intratest/modules/system/defaults.css";
@import "/ak/intratest/modules/system/system.css";
@import "/ak/intratest/modules/user/user.css";
@import "/ak/intratest/sites/all/modules/cck/content.css";
@import "/ak/intratest/sites/all/modules/date/date.css";
@import "/ak/intratest/sites/all/modules/date/date_popup/themes/white.calendar.css";
@import "/ak/intratest/sites/all/modules/date/date_popup/themes/timeentry.css";
@import "/ak/intratest/sites/all/modules/img_assist/img_assist.css";
@import "/ak/intratest/sites/all/modules/lightbox2/css/lightbox.css";
@import "/ak/intratest/sites/all/modules/tagadelic/tagadelic.css";
@import "/ak/intratest/sites/all/modules/cck/fieldgroup.css";
@import "/ak/intratest/sites/all/themes/yaml/layouts/yaml_akstmk5v3/advforum/advanced_forum-structure.css";
@import "/ak/intratest/sites/all/themes/yaml/layouts/yaml_akstmk5v3/advforum/advanced_forum.css";
@import "/ak/intratest/sites/all/modules/panels/css/panels.css";
@import "/ak/intratest/modules/forum/forum.css";
@import "/ak/intratest/sites/all/modules/nodequeue/nodequeue.css";
@import "/ak/intratest/modules/comment/comment.css";
@import "/ak/intratest/sites/all/themes/yaml/yaml/core/base.css";
@import "/ak/intratest/sites/all/themes/yaml/css/screen/basemod.css";
@import "/ak/intratest/sites/all/themes/yaml/css/screen/basemod_2col_13.css";
@import "/ak/intratest/sites/all/themes/yaml/css/screen/basemod_drupal.css";
@import "/ak/intratest/sites/all/themes/yaml/layouts/yaml_akstmk5v3/css/screen/basemod_akstmk5v3.css";
@import "/ak/intratest/sites/all/themes/yaml/layouts/yaml_akstmk5v3/css/screen/basemod_gfxborder_akstmk5v3.css";
@import "/ak/intratest/sites/all/themes/yaml/css/navigation/nav_shinybuttons.css";
@import "/ak/intratest/sites/all/themes/yaml/layouts/yaml_akstmk5v3/css/navigation/nav_shinybuttonsmod_akstmk5v3.css";
</style>
<style type="text/css" media="all">@import "/ak/intratest/sites/all/themes/yaml/css/navigation/nav_vlist_drupal.css";
@import "/ak/intratest/sites/all/themes/yaml/css/screen/content.css";
@import "/ak/intratest/sites/all/themes/yaml/layouts/yaml_akstmk5v3/css/screen/contentmod_akstmk5v3.css";
@import "/ak/intratest/sites/all/themes/yaml/yaml/core/print_base.css";
@import "/ak/intratest/sites/all/themes/yaml/css/print/print_003.css";
@import "/ak/intratest/sites/all/themes/yaml/css/print/print.css";
</style>
<style type="text/css">#page_margins { width: auto; min-width: 740px; max-width: 95em }</style>
<!--[if lte IE 7]>
<style type="text/css" media="all">@import "/ak/intratest/sites/all/themes/yaml/yaml/core/iehacks.css";</style>
<style type="text/css" media="all">@import "/ak/intratest/sites/all/themes/yaml/css/patches/patch_nav_vlist_drupal.css";</style>
<style type="text/css" media="all">@import "/ak/intratest/sites/all/themes/yaml/css/patches/patch_2col_13.css";</style>
<style type="text/css" media="all">@import "/ak/intratest/sites/all/themes/yaml/css/patches/patch_drupal.css";</style>
<![endif]-->

#28

hass - July 3, 2008 - 18:12

I totally don't understand the sense behind "Max. number of CSS files inside one style tag".

#29

brahms - July 4, 2008 - 00:32

I try to explain in detail:

After I modified the function drupal_get_css() in common.inc like I described in my comment#7 above I thought that everything worked fine. I got one style tag per media type and in the style tag for media="all" I had more than 30 imported stylesheet files and they all seemed to be processed by IE7. What I did not realize at this time was the fact that IE7 still did only process the first 30 imported files, because my site looked correctly formatted. After I added 2 modules with some additional CSS stylesheets I noticed that IE7 did not process all of those imported stylesheets inside the style tag for media="all". The font styles (color and font-family) of the navigation menu suddenly had changed in IE7. By comparing the html source of the web page to the html source of the same page before adding the 2 modules I found out that the affected stylesheet was now number 31 in the import sequence inside the style tag for media="all". Before I added the 2 additional modules the same stylesheet was number 29 in the import sequence inside the style tag for media="all". (As you are the developer of YAML for Drupal: it is a stylesheet I called contentmod_my-yaml-layout.css which overrides styles in yamls content.css). So I think you are mistaken in your comment#5 above where you say "Well, it counts the number of <style></style> and <link type="text/css" ... /> not the number of files added with @import"

So I tried what I describe in my comment#25 above: I still group stylesheets per media type with @import inside <style&gt tags but I limited the number of imports to 30 stylesheets. For the following stylesheet files 31, 32, and so on I use a second <style> tag for the same media type="all". Again I would limit the number of @import sequences to 30 stylesheets, if I had more than 60 stylesheets to import for thia media type (but I haven't, I have exactly 36 CSS stylesheets for this media type). You can see the result in my comment#27 above.

The sense behind "Max. number of CSS files inside one style tag" now is that you can specify this limit for testing purposes. If in my example from comment#27 I would set it to 5, I would get 6 <style> tags for media type="all" each with 5 @import lines inside and 1 more <style> tag for the last of the 36 CSS stylesheets for the media types.

If I set the limit to 30 I reach the optimum setting, because I minimize the number of generated <style> tags using IE7s (obvious) limit for @import lines inside one <style> tag.

If I set the limit to 31 oder any higher number, I will get layout errors in IE again because @import number 31 and above will not get processed.

I considered if I should hard code the setting for "Max. number of CSS files inside one style tag" to the optimum value of 30. I did not because the possibility to specify "Max. number of CSS files inside one style tag" in performance settings page makes it easier to test IEs errors without changing the PHP source.

#30

hass - July 4, 2008 - 07:41

IE does not have issues with loading only a maximum of 30 files! It have issues with more then 30 <style> or <link> tags - it doesn't matter if you add 100 or 1000 files between one style tag <style type="css/text" media="all">... [here you can add for e.g. 1000 @imports without any problems] ...</style>. 1000 @imports are nevertheless untested :-).

We need to fix this without adding new options to the UI what is not required at all.

#31

brahms - July 4, 2008 - 10:26

Are you sure that IE7 does not have issues importing and processing more than 30 files inside one single <style> tag? I had read your comment#5 and I know you have written there and in your new comment#30 that it doesn't matter if there are 100 or 1000 imported files inside one singel <style> tag.

But as I notized in my Development Site, in IE7 the file number 31 (contentmod_myyaml.css) did not get processed because the styles for the navigation menu elements in this file did not override the default yaml styles from a previous processed stylesheet (content.css) anymore. If I deactivated one drupal modul with 2 own CSS files, contentmod_myyaml.css got number 29 inside the <style> tag and got processed again, as I saw in the now correct styling of the navigation menu.

I can only reproduce this, I haven't found an microsft issue describing this effect.

I will test it again and maybe I can install a demonstration site to show you what I mean.

Nevertheless I'd like to thank you for your strong engagement in this issue. I think it is a very important issue which may lead to many side effects. If someone doesn't know about this IE bug he could hardly find the reason why sometimes after adding a module something suddenly does not work anymore or the freshly installed module does not work correct.

#32

brahms - July 4, 2008 - 13:34

Demo Site as answer to comment#30:

I provide a demo site under http://78.47.120.6/demo/intratest/
(maybe hass will be so kind to try the demonstartion, I prepared this site as an answer to his post in comment#30)

On this site anyone can reproduce the bug in Internet Explorer 7 as I descibed it in my comment#23 above.

Here are some instructions to reproduce the bug and test the work-around patch from my comment#26> above. (If someone wants to apply the patch on a site: please don't use the patch from comment#23, because this patch does not work. Use the patch in file common.inc-5.7-css_styles.patch from comment#26> instead!) I included those instructions in the front page of the demo site too.

I would be glad if someone tries to reproduce this bug in Internet Explorer 6 and post the results here.

Please login as user demo with password demo

You see the bug in different font colors and styles of the navigation menu left under following circumstances:

Navigate to Performance, go to the bottom of the page, enable Combine CSS files per media type and set Max. number of CSS files inside one style tag to 0. Then follow this link forum-issue/how-add-images-story to go to the forum issue How to add images to a story. You will notice that the colors of the title of the navigation menu and the color of the forum topic title are blue. They should be green like defined in the CSS stylesheet contentmod_akstmk5v3.css, which you can see in the html source but did not get processed in IE.

Now try a work-around for this bug

Navigate to Performance again, go to the bottom of the page, enable Combine CSS files per media type and set Max. number of CSS files inside one style tag to 30. Then follow this link forum-issue/how-add-images-story to go to the forum issue How to add images to a story again. You will notice that the color of both the title of the navigation menu and the color of the forum topic title are now dark green because now the stylesheet contentmod_akstmk5v3.css got processed!

#33

hass - July 4, 2008 - 15:50

Hm... this is really new to me and makes thinks more worse... :-(. I haven't tested IE7 much regarding this issue, but thought it should work the same way wrong as IE6 and as MS documented this :-(.

#34

brahms - July 4, 2008 - 17:31

hass, thank you for your quick reply. By the way, in the meantime I could test the demo site with IE6 and IE6 shows the same bug as IE7! So I think with the patch we may achieve a usable work around. We can have a maximum of 30 <style> tags over all and inside each of them a maximum of 30 imported CSS files. So we can use a limit of 900 CSS files... This is not really correct because Drupal's drupal_get_css (and my modified version) groups some no_preprocess CSS files in their own <style> tag and there are some more conditional CSS files from YAML. So we may end with a 880 file limit or so, but this should work for everyone.

#35

Damien Tournoud - July 4, 2008 - 19:03
Version:6.1» 7.x-dev

#245268: Notify about, or avoid, IE 31 stylesheet limit was a duplicate.

Bugs should be fixed first in the development version (Drupal 7.x currently), than backported.

#36

brahms - July 5, 2008 - 00:06

Damien, I agree to your comment#35. Here is my patch for the head revision of Drupal, which is Drupal 7. Please apply this patch from Drupal's base directory. This patch modifies the files include/common.inc and modules/system/system.admin.inc. It will add a fieldgroup named "Group CSS files" at the bottom of the performance settings page.

This fieldgroup provides 2 new fields:

Group CSS files per media type enables or disables the grouping of multiple CSS stylesheets (using @import) into one <style> tag per media type.

Limit for the number of CSS files inside each style tag gives the additional possibility to specify the maximum number of CSS files inside each <style> tag.

Best settings are:

Group CSS files per media type: Enabled
Limit for the number of CSS files inside each style tag: 30

AttachmentSize
drupal-head_css-styles.patch9.57 KB

#37

brahms - July 5, 2008 - 20:03

Here are the backported patches for:

Drupal 6 Branch (with cvs tag DRUPAL-6): drupal-6_css-styles.patch
Drupal 6.2 (with cvs tag DRUPAL-6-2): drupal-6-2_css-styles.patch
Drupal 5 Branch (with cvs tag DRUPAL-5): drupal-5_css-styles.patch
Drupal 5.7 (with cvs tag DRUPAL-5-7): drupal-5-7_css-styles.patch

AttachmentSize
drupal-5-7_css-styles.patch9.45 KB
drupal-5_css-styles.patch9.45 KB
drupal-6-2_css-styles.patch9.67 KB
drupal-6_css-styles.patch9.67 KB

#38

Crell - July 5, 2008 - 20:52

No no no! The use of @import was removed in Drupal 6 very deliberately, because it causes various problems. Let's not bring it back. (See #145218: Use href instead of @import for CSS)

#39

hass - July 5, 2008 - 21:33

@Crell: If we are not able to move back to @import we cannot solve this issue...

#40

Crell - July 6, 2008 - 00:35

@hass: You'll forgive me for not being in favor of making our code lower quality to cater to a dying browser that has already resulted in the waste of hundreds of thousands of hours. If you use @import, you cannot effectively Save-As a page because the imported stylesheets don't come with it. Frankly I've yet to hit the 30 stylesheet mark myself so I've not encountered this problem.

What about instead some sort of setting that forced the compressed CSS to be regenerated for every page request? That way if you get as far as 30 stylesheets AND you're doing your primary development in IE 6 (in which case you're doing it wrong anyway), you can enable the compressor to get down to one file and then get a new stylesheet built each time. It's development anyway so you just eat the recompilation cost.

#41

brahms - July 6, 2008 - 02:32

@Crell: from an idealistic point of view you may be right if you prefer other browsers than IE, I also do prefer Firefox over IE. But if you are developing web sites for the people out there and especially if you try to develop web sites for companies you have to accept that many of them will use IE as browser. For me that is fact number one.

Drupal offers a public and a private download method: with public download method everyone has the possibility to download every file from Drupal's file system path. If you want or need to control access to downloads you have to use the private download method. Therefore you don't have the possibility to compress the stylesheets. For me this is fact number two.

A limit of 30 stylesheets seems enough at first glance. But in Drupal core alone you will find 20 stylesheets (without a theme). If you build a feature rich site you will need to integrate contributed modules and many of them bring additional stylesheets into your pages. It won't take long to recognize that 30 stylesheets are not enough. This is fact number three for me.

And if you use a modular theme like Yaml for Drupal with a lot of stylesheets you soon will get about 50 stylesheets or more that need to be processed.

So the big and serious dilemma for me is: I do need private download method, I do have a customer who uses IE7, I am using a pretty bunch of modules (cck, views, panels, tinymce to name some of them) and I am using this awesome layout framework Yaml for Drupal is.

At the end I have to choose between 2 problems:

  • I apply the patch I provided to make my site work in IE and loose the possibilty to "save as a page"
  • I deliver a site with a broken layout to my customer for the sake of code quality. Maybe he will be pleased by the perfect code quality which gives him the possibility to save the broken layouts as a page. Or he won't get much impressed by the excellent code which sends such ugly pages to his IE and might think that the developer or the system offers only poor quality

Which of these 2 options would you select?

#42

Michelle - July 6, 2008 - 03:18

@Crell - I just stumbled on this issue, but have dealt with this problem on CRO for a while. I can't turn off aggregation on the live site, even if I just want to check something quick in firebug, because then the site is unusable for any IE users that happen by. I don't know enough about this to know what the best solution is, but I wanted to offer another voice saying yes this is a real world problem even for people who don't normally use IE.

Michelle

#43

pegleglax - July 6, 2008 - 04:50

This. Patch. Saved. My. Life! Brahms you're the shiznit!

Will this patch be in 6.3?

#44

hass - July 6, 2008 - 09:03

You'll forgive me for not being in favor of making our code lower quality to cater to a dying browser that has already resulted in the waste of hundreds of thousands of hours.

I don't know what your site is doing but this is more then unrealistic today and for the next ~2 years. I'm not a fan of IE and i hope it rest in peace, but it will *not* for the next 5 years and therefore we must also take a look to IE6 and IE7 as long as this versions going under 2% usage what may never be happen.

If you use @import, you cannot effectively Save-As a page because the imported stylesheets don't come with it.

I truly understand save-as this issue... but it is *much more* important to get a browser working that has a market share of ~45% (IE6) and ~35% IE7. However we all know how buggy this damn browsers are or not. I don't think 80% usage are a dying browser we don't need to support...

What about instead some sort of setting that forced the compressed CSS to be regenerated for every page request? That way if you get as far as 30 stylesheets AND you're doing your primary development in IE 6 (in which case you're doing it wrong anyway), you can enable the compressor to get down to one file and then get a new stylesheet built each time. It's development anyway so you just eat the recompilation cost.

This will not work for people with private files folder and speed will be awful...

Aside, I'm not developing "only for" or "primary with" IE... but the yaml theme provides compatibility to *all* major browsers we cannot ignore and solves uncountable issues that you do not need to care about as webdeveloper if you use the YAML CSS Framework. See http://www.yaml.de/en/documentation/introduction/browser-support.html about the supported browsers. There is no other theme out there that works reliable with so many and all this browsers. You can do this yourself different and develop only for standard compliant browsers what we are doing too, but we will not ignore IE and have a *fallback* for this browser we really need to test.

Have you tested IE8 with RTL? *argl* I don't know what this guys in Redmond are working on, but this is more broken then IE6 ever was.

#45

hass - July 6, 2008 - 09:23

Will this patch be in 6.3?

@pegleglax: I expect this is not going to get committed as is - today and we cannot say when 6.3 goes out.

@brahms: your patch cannot get in as is, while you are changing the UI that is additional not required. This is something we don't need as we know the hard technical limits (30 files). The settings are fine for someone how like to reproduce and test this IE bugs themself, but we cannot stress people to configure this as they do not understand the issue behind (newbies, non developers, etc).

#46

hass - July 6, 2008 - 09:37

@brahms: your patch produces a WSOD. See your site on performance link.

#47

brahms - July 6, 2008 - 11:44

your patch cannot get in as is, while you are changing the UI that is additional not required. This is something we don't need as we know the hard technical limits (30 files).

@hass: I agree with you. Before I'm going to modify the patch please tell me your opinion to following suggestions:

  • I remove the parameter Max. number of CSS files inside one style tag from the performance settings page. Instead of the UI setting I hard code the limit to 30 files. As it is now this limit is only relevant, if the other remaining UI parameter Combine CSS files per media type has been set to Enabled
  • I keep the other parameter Combine CSS files per media type in the performance setting page but rename it to Group CSS files for simplicity. If the parameter Group CSS files has been left or set to it's default state Disabled I'll change the code so that it will use <link> tags just like it is doing now in Drupal head revision and Drupal 6.x. I hope this way the patch is going to be accepted as it leaves the behaviour of the function drupal_get_css untouched if grouping of CSS files is disabled.

#48

brahms - July 6, 2008 - 11:48

your patch produces a WSOD. See your site on performance link

@hass: hm, I could not reproduce the WSOD with IE7 or Firefox. I also did not find any hint for the problem in apache logfiles or Drupal's watchdog logs. What browser did you use? Do you still get the WSOD?

#49

hass - July 6, 2008 - 14:28

IE7. "Max. number of CSS files inside one style tag" = 0 and the "Combine CSS files per media type" have been changed from enabled to disabled. Afterwards I've got a WSOD. After some clicks, wait and another click the performance page showed again.

Don't change the UI please. I'm not yet sure what your patch does in detail as I haven't found any time to test. If the combine CSS files change the order of CSS files you must remove this stuff, too. I'm not sure what this radio box change without taking a look to the code.

#50

brahms - July 6, 2008 - 16:36

IE7. "Max. number of CSS files inside one style tag" = 0 and the "Combine CSS files per media type" have been changed from enabled to disabled. Afterwards I've got a WSOD. After some clicks, wait and another click the performance page showed again.

@hass: I am not sure but this WSOD may be a side effect of the 30 stylesheet problem. With these settings some of the stylesheet are not processed and I get some kind of javascript error (variable 'Drupal' is undefined' in IE7).

But nevertheless: I will prepare a fresh demo site for Drupal 7 head revision. I won't install any contributed module there. To get the needed number of CSS files I will slightly modify Garland theme so that it loads some dummy stylesheets. In stylesheet number 31 I will place a significant style change (I think changing the text color of all headings to red) so it would be easy to see, if this stylesheet gets processed or not.

Don't change the UI please.

@hass: I'm not shure what you mean here? I intend to add only one new parameter Group CSS files to performance setting page so that everyone has the possibility to switch it on or off.

If the combine CSS files change the order of CSS files you must remove this stuff, too.

My code is written to leave the order of CSS files untouched. If he does not he would be buggy! And if you disable parameter Group CSS files or if you use public download method and enable Optimize CSS files everything should work as it did before applying my patch.

But please wait for the Drupal 7 demo site and the new cleaned up patch I announced and descibed in my comment#47. I hope it will be ready in a few hours.

#51

brahms - July 6, 2008 - 19:07

New patch and Drupal 7 demo site:

I attach a new and hopefully final patch file for Drupal 7 (head revision from today). This patch modifies the files include/common.inc and modules/system/system.admin.inc. To apply the patch please gunzip this file in your Drupal 7 base directory and start the patch from there with:

patch -p0 < drupal-head_css-styles.patch1

It will add the new parameter Group CSS files in the existing fieldgroup Bandwith optimization in the performance settings page:

If this parameter is disabled or if the existing parameter Optimize CSS files is enabled everything keeps working like it did before applying the patch.

Only if Group CSS files is enabled and Optimize CSS files is disabled all CSS files are grouped using the CSS @import command up to a limit of 30 files per media type into single <style> tags. The order of file processing is itended to be unchanged. This will provide a work around for IE's 30 stylesheets bug.

The new demo site where everone can reproduce the bug and see the patch working is: http://78.47.120.6/demo/drupal7head/

I really hope that this patch will be accepted for core as long as it will prove to be free of bugs. It does not change Drupal 7 behaviour if the new option Group CSS files is disabled.

AttachmentSize
drupal-head_css-styles.patch1_.gz2.02 KB

#52

gh0st25 - July 6, 2008 - 19:12

Isn't this because IE6 doesn't recognize more than 30 CSS links? Turn on CSS compression.

But really, this seems to be a problem deeper in the Drupal core / community. Every module has its own CSS it seems like. There needs to be a more uniform approach to module layout to where its not so absurd with the CSS includes.

#53

brahms - July 6, 2008 - 19:27

Isn't this because IE6 doesn't recognize more than 30 CSS links? Turn on CSS compression.

@gh0st25: yes this is the reason (btw; IE7 shows the same bug). But you can't turn on CSS compression if you set Drupal's file download method to private...

#54

jmburnz - July 6, 2008 - 21:53

Subscribing

#55

hass - July 6, 2008 - 21:58

@brahms: only as side info... there is a private/public handing patch on the road... #166759: SoC 2007: Public/Private File Handling that was "planed" for D6 and hopefully go in for D7, but nevertheless will not solve our issue here.

#56

gh0st25 - July 6, 2008 - 22:55

Anything more than 5 CSS includes is absurd, there should be a more concerted effort to addressing that. Personally, I refuse to use @import. IE6 works fine with link rel=''.

People on slower internet connections are getting roadblocked by all those includes to download, not to mention images and site graphics.

Can't there be an algorithm where you put your CSS files into a directory, and in the admin say which files should be included in order, then, Drupal takes all of these files and creates one sitewide CSS file with all the definitions in it on demand or cron? If this is CSS Compression I apologize, but I am a stickler on the amount of includes for a website. This doesn't seem like a difficult issue, the issue is moreso on the side of the necessity (or perceived necessity) to having 25-50 CSS includes.

 
 

Drupal is a registered trademark of Dries Buytaert.