You are making an incorrect assumption in your code. The key of the hook_css_alter array is NOT neccesarily the filename. The filename resides in $info['data'] in each array element as long as $info['type'] == 'file'.

My patch fixes that behavior by using $info['data'] instead. Also I am not quite sure why you were using array_keys and array_values and then (consequently) array_splice. I removed that part aswell in favor of a simple loop on the $css array.

CommentFileSizeAuthor
the-array-key-is-not-the-filename.patch2.8 KBfubhy

Comments

corey.aufang’s picture

corey.aufang commented

The assumption was made based on the way this module worked in D6.

In D6 the key was the actual file name, so you had to use splicing on the keys and then recombining with the values to make sure that a css file show up in the proper order.

Now if you're correct that in D7 the key is not as important, but is instead a reference, then yes a change to how it is done could be made.

This also spurs making a change to the weight of the module to make sure that it processes after other modules so they have a chance to change the LESS files before they are processed.

fubhy’s picture

fubhy commented

Okay, I see... Well in 99% of the cases the key will still be the filename, however, you CAN add css files like this:

drupal_add_css('some_random_string', array('data' => 'real_filename.css'));

Which also works and causes the array key in HOOK_css_alter to be "some_random_string" when you would expect the filename. The problem with this is that this means that (in your case) you are currently POTENTIALLY creating direectory structures with special characters like (in my case "ie::filename.css") which causes errors on Windows servers (I was using drupal_add_css('ie::filename.css', ...) in order to prevent my IE files from being overriden when I add the same file for non-IE again).

A high weight might be a good choice, yes. OR you could completely skip the HOOK_css_alter and instead use HOOK_element_info_alter to extend the "styles" array with another pre_render hook (#aggregate_callback). In this case we are not ALTERING the css, instead we are filtering it in a way. I implemented my CSS filtering with prepending a #aggregate_callback aswell. This gets invoked AFTER hook_css_alter (during rendering) and therefore might be a perfect place for your code.

http://api.drupal.org/api/drupal/includes--common.inc/function/drupal_ge...
http://api.drupal.org/api/drupal/includes--common.inc/function/drupal_pr...
http://api.drupal.org/api/drupal/modules--system--system.module/function...

corey.aufang’s picture

corey.aufang commented

I'm looking through the data from HOOK_element_info_alter and it appears that #pre_render and #aggregate_callback back are two different things.

Also while #pre_render is an array which can be easily extended, #aggregate_callback is a string and I worry about usurping drupal_aggregate_css.

If we use #pre_render I see all the information, tho I array_unshift so that my changes can piggyback onto drupal_pre_render_styles.

This does seem to be a more elegant solution than altering the weight of the module.

fubhy’s picture

fubhy commented

Yes, you are right... I mixed that up! I did not use another #aggregate_callback, instead I prepended another item to the pre_render array (so it gets executed BEFORE the core pre_render (aggregation and grouping) stuff). In my case this works like a charm and sure would be a elegant solution for LESS.

bocaj’s picture

bocaj commented

I have tested this patch and it works fine for me! Would like to see this committed!

corey.aufang’s picture

corey.aufang commented

I've committed the latest change which should finally fix both the race condition and it preserves the key in the styles array.

Should be rolled in the dev tomorrow morning.

corey.aufang’s picture

corey.aufang commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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