Remove workaround for IE's "Flash of Unstyled Content" bug

Woo-hoo! I'll make sure to do that before the next release. :-)

Fixed!

Hmm… Looks like I grabbed that bit of code from core’s page.tpl.php. And, after doing some research, it appears that line was in the original patch for modules/system/page.tpl.php that was committed with the new D6 theme layer #137236: theme preprocess documentation does not exist. I can't seem to find who/where it was first introduced.

Perhaps, the "fix" went into page.tpl.php before Drupal made the switch away from @import for CSS files?

Kicking the testbot for good measure.

Committed to CVS HEAD. Thanks.

Moving back to DRUPAL-6 so goba can commit if he wants. The patch at #5 still applies cleanly.

Is it not a common practice to use @import in the theme CSS files? Then this would break those themes in IE, right?

Is it not a common practice to use @import in the theme CSS files?

If a themer uses @import in their CSS files, then they wouldn't be able to use the CSS aggregator effectively, since those @imported files wouldn't be included in the aggregated CSS file. Common practice? I hope not. In the non-Drupal world, @import is common, but Drupal should be educating people why adding CSS files to the .info file is the best route.

Then this would break those themes in IE, right?

Technically, it wouldn't be “broken" because IE would just show a temporary flash of unstyled content before displaying things correctly. And this "flash problem" might prompt a themer to discover the proper way to include a CSS file in Drupal. So, personally, I think this is safe to put into D6.

JohnAlbin: to me, it looks like both drupal_load_stylesheet() and drupal_build_css_cache() resolve and include @import-ed CSS files, so it is far from an improper or unsupported way to include a stylesheet in Drupal. They are supported the same way as CSS files added through .info files as far as aggregation goes. Please correct me if this is not true.

Also, since IE has a limitation of not loading CSS files above a certain number of <link> tags, I can see people resorting to @import for at least workable development when they need their CSS unaggregated. In fact the "canonical" issue (for which others were marked duplicate) suggests using @import, but is a vibrant discussion for sure: #228818: IE: Stylesheets ignored after 31 link/style tags

Gábor, thanks for the links. I agree we should review those 2 functions and issue #228818 before committing this to D6.

/modules/system/maintenance-page.tpl.php in Drupal 7 still have this fix. It is unclear to me whether this fix will be removed in Drupal 7 and 6.

Marking as a bug since /modules/system/maintenance-page.tpl.php in Drupal 7 still have this fix at line 24.

Taking a stab at contributing patch, something I've never done before, so please be kind. This is for /modules/system/maintenance-page.tpl.php, only. Does the previous patch need to included as well? Let me know...

#16 looks good to me. The cleanup already went in to html.tpl.php and Garland's maintenance-page.tpl.php, so this is just making modules/system/maintenance-page.tpl.php have the same cleanup.

Yay for less cruft. :) Committed to HEAD.

Great job on your first patch, thund3rbox! :)

Moving back to a D6 issue as per #11 for John and goba to decide on, but marking postponed until resolution of #228818: IE: Stylesheets ignored after 31 link/style tags, which is RTBC for D7, but if/when that lands, will need a re-roll for D6, because the D7 patch connects with hook_css_alter() which isn't available in D6.

Doesn't need to be postponed any more. #228818: IE: Stylesheets ignored after 31 link/style tags landed. #727072: Warn admin when IE 31 CSS file limit exceeds on D6 is "won't fix".

My recommendation is to mark this "won't fix", unless there is benefit. If the only thing it accomplishes is making a couple tpl.php files slightly cleaner, I don't think it's worth introducing a possible, even if unlikely, regression. There may well be some site out there that overrides the $styles variable with HTML code that uses STYLE tags instead of LINK tags and that also removes all SCRIPT tags (even drupal.js and jquery.js). Yes, that's very unlikely, and if there was any real benefit to removing the FOUC workaround in D6, then it would make sense to do so, but without any identified benefit, we should err on the side of caution with respect to changes to D6.

Yeah, you're right. There's no harm in leaving it in. And removing it only makes things slightly cleaner.