As a followup to #913972: [Regression] Add an option for CSS/JS aggregation into a big file each

-> The atheme css and js files will not get included in the "big" aggregated version of the js or css
-> Schemes are not working, when css aggregation is turned on

I'm using the latest dev version of atheme with a fresh subtheme ( which has schemes working correctly when aggregation is turned off ).

Comments

Jeff Burnz’s picture

Jeff Burnz commented
Assigned: Unassigned » Jeff Burnz

I will test this tonight, cheers.

Jeff Burnz’s picture

Jeff Burnz commented
Status: Active » Fixed

I had not updated drupal_add_css for style schemes, I have fixed this and committed to head, the change is in template.php (the subthemes template.php), around line 21 it will now look like this:

if (theme_get_setting('style_enable_schemes') == 'on') {
  $style = get_at_styles();
  drupal_add_css(path_to_theme() . '/css/schemes/' . $style, array('group' => CSS_THEME));
}

This should be fixed.

Its kind of odd about the js, I too cannot find them (the js files loaded via drupal_add_js) in the aggregated files, but, they keep on working so they must be loaded somewhere? The one js file loaded via the info file is there (at.js).

XiaN Vizjereij’s picture

XiaN Vizjereij commented
Status: Fixed » Active

That makes the schemes working again, but still all the css files are seperate

<link type="text/css" rel="stylesheet" href="http://example.com/sites/default/files/css/css_cleuHFojkzTyTanalmqhTbYgaYI8S0j6iuAiZoZvOAM.css" media="all" /> 
<link type="text/css" rel="stylesheet" href="http://example.com/sites/default/files/css/css_HSgv8_OK8ywkq5425pUurW9g5-rUqX4crhdZrp5Ndkg.css" media="all" /> 
<link type="text/css" rel="stylesheet" href="http://example.com/sites/default/files/css/css_lhSY-hMKe3Et_jdbGonL3WFbdxmUZPd_HioblarUKc8.css" media="all" /> 
<link type="text/css" rel="stylesheet" href="http://example.com/sites/default/files/css/css_VeLvNj0ldPkmf8IEiVTZQgHFpMbHIcxXoOyrV2P89Eg.css" media="all" /> 
<link type="text/css" rel="stylesheet" href="http://example.com/sites/default/files/css/css_rH6denrwELMY0yK2WtGPDx_aaxYl_i9AOCL7WN-qU6A.css" media="all" /> 
<link type="text/css" rel="stylesheet" href="http://example.com/sites/default/files/css/css_03QohP9S1J6PwkHlmWGy03eikSWB4AX-TVP3GWu2efw.css" media="all" /> 

<script type="text/javascript" src="http://example.com/sites/default/files/js/js_VF53tELsw3kPql1FJD633rc2wNa0fCZV5Foz0aUfhvo.js"></script> 
<script type="text/javascript" src="http://example.com/sites/default/files/js/js_eIAhklcOBGlZqIiVf8Fgy8nrQwFLvB1DHZfTV4C9Yek.js"></script> 
<script type="text/javascript" src="http://example.com/sites/default/files/js/js_rzr0KDa9byCJYFYrPGuJyUfdTrXu8fFOmFNeHpdqQa0.js"></script> 
<script type="text/javascript" src="http://example.com/sites/default/files/js/js_Io9BZ0Vb29p9ueRqdiYDNYGQapo5_UIQeObDdksHa6A.js"></script>

Thats probably, because drupal_add_js/css has a new "preprocess" option that needs to be set in order to get into the "big" file.
See http://drupal.org/update/modules/6/7#drupal_add_js_options

Jeff Burnz’s picture

Jeff Burnz commented

I will test some more, the way I read it cache and preprocess default to TRUE http://api.drupal.org/api/drupal/includes--common.inc/function/drupal_ad..., need to lookup drupal_add_css again to see what the story us there, I would think its the same, with aggregation being the default setting.

XiaN Vizjereij’s picture

XiaN Vizjereij commented

Doesn't $options['preprocess'] = $options['cache'] ? $options['preprocess'] : FALSE; defaults preprocess to false? The values deeper into the function are the defaults for the drupal.js as far as i understand it. Also

If JavaScript aggregation is enabled, all JavaScript files added with $options['preprocess'] set to TRUE will be merged into one aggregate file.

And

$options['preprocess'] should be only set to TRUE when a file is required for all typical visitors and most pages of a site.

implies that the default is false. The same applies for the drupal_add_css function.

Jeff Burnz’s picture

Jeff Burnz commented

Did you test it out, what were your results? A patch would be pretty good :)

Jeff Burnz’s picture

Jeff Burnz commented

I tested with the various drupal_add_js options such as preprocess, cache and group and none made any difference to aggregation (with regards to the number of aggreated files).

I also compared the number of aggregated files to Garland and in comparison Adaptivetheme adds just one more aggregated JS file. None of the themes I tested with have one big JS or CSS file, there always seems to be 6 CSS files and 3 or 4 JS files (only tested a couple of themes).

XiaN Vizjereij’s picture

XiaN Vizjereij commented

I can confirm that. Tested some cases also and non are working. I'll reopen the bug report on drupal core.

int’s picture

int commented
Status: Active » Closed (duplicate)

please use this:
#913972: [Regression] Add an option for CSS/JS aggregation into a big file each

XiaN Vizjereij’s picture

XiaN Vizjereij commented
Title: Aggregation not working correctly and schemes not possible when aggregation is turned on » Make sure drupal_add_* are in the correct group and have preprocess enabled
Category: bug » feature
Priority: Major » Normal
Status: Closed (duplicate) » Active

Ok apparently that ... is ... working as intended .. kind of. Another change in D7 i will probably never understand. That might make sense from a core developers point of view, but for me as end user and administrator this simply adds massive I/O on my cache folder, 7 new HTTP requests per page view and more objects ( the parallel download problem ) >.<

As for at_theme ... Jeff, could you please have a look over the theme and make sure that all drupal_add_* have the correct group and preprocess enabled where it makes sense? Because its still FALSE by default.

Jeff Burnz’s picture

Jeff Burnz commented

@10 Yep, will make sure this is done.

Jeff Burnz’s picture

Jeff Burnz commented
Status: Active » Fixed

The options have been added + committed to head, so marking as fixed.

Status: Fixed » Closed (fixed)

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