[development] Using array_merge_recursive() in _system_theme_data()

[Steve Edwards]

While working with a couple jQuery files in a recent D6 project by adding a second script to the theme.info file, I came across
the issue where when a script is added in theme.info, it wipes out the default reference to script.js, so you also have to add
script.js in theme.info so it is available.  I was discussing this with Crell in #drupal, and it was suggested to me that I write
a patch to fix this.  So I figured I'd take a shot at my first core patch and tackle it.

After spending a few hours going through _system_theme_data() and drupal_parse_info_file() with my handy dandy Komodo debugger,
I've finally figured out the problem.  It lies in this line in _system_theme_data():

$themes[$key]->info = drupal_parse_info_file($theme->filename) + $defaults;

In particular, it's how the array returned from drupal_parse_info_file() is merged with $defaults.  Because it's using the +
operator, any duplicate keys are not overwritten (as specified at http://de3.php.net/manual/en/language.operators.array.php).  Let
me illustrate by showing the two arrays that are to be merged (this was created by adding scripts[] = script2.js to pushbutton.info).

//array returned from drupal_parse_info_file()
$info = array(
       'name' => 'Pushbutton',
       'description' => 'Tabled, multi-column theme in blue and orange tones',
       'version' => '7.x-dev',
       'core' => '7.x',
       'engine' => 'phptemplate',
       'project' => 'drupal',
       'datestamp' => '1230077295',
       'scripts' => array('script2.js'),
     );

//defaults as defined in system_theme_default()
$defaults = array(
     'regions' => array(
       'left' => 'Left sidebar',
       'right' => 'Right sidebar',
       'content' => 'Content',
       'header' => 'Header',
       'footer' => 'Footer',
     ),
     'description' => '',
     'features' => array(
       'comment_user_picture',
       'favicon',
       'mission',
       'logo',
       'name',
       'node_user_picture',
       'search',
       'slogan',
       'main_menu',
       'secondary_menu',
     ),
     'stylesheets' => array(
       'all' => array('style.css')
     ),
     'scripts' => array('script.js'),
     'screenshot' => 'screenshot.png',
     'php' => DRUPAL_MINIMUM_PHP,
   );

The problem is that both have a [scripts] key, so since duplicates aren't overwritten, only one script array remains.  Which one
remains depends on the order of the arrays in the statement (i.e. drupal_parse_info_file($theme->filename) + $defaults vs.
$defaults + drupal_parse_info_file($theme->filename)).

The way to fix this as best I could tell is to simply use array_merge_recursive() to merge the arrays:

$themes[$key]->info = array_merge_recursive(drupal_parse_info_file($theme->filename), $defaults);

What this does is merge the [scripts] arrays together so that instead of only getting one as the result, both are added:

[scripts] => Array
     (
         [1] => script2.js
         [2] => script.js
     )

This is also true for the regions, features, and stylesheets arrays; you have to replace everything in theme.info if you add more
items there, and using array_merge_recursive() fixes that problem.  However, problems do arise for the non-array default items, 
such as description and screenshot.  Since there is one in each array, the value at [description] and [screenshot] is an array 
instead of a string, so t() throws a "Warning: Illegal offset type in isset or empty in t() (line 1057 of C:\Program Files\Apache 
Software Foundation\Apache2.2\htdocs\drupal7\includes\common.inc)." error, and no screenshot is displayed on the themes page.

So, in my opinion, it seems the best thing to do is:
1) Use array_merge_recursive() instead of + to merge the arrays;
2) Remove 'description' and 'screenshot' from the default settings in system_theme_default()

I suppose code could be added to handle the description and screenshot values so only one is used, but what I've suggested seems 
neater.

Sorry for the book length email, but I've never been accused of leaving out details.:-)  Since this is my first core patch 
attempt, I just wanted to check with other more experienced core developers to see if there's something I'm not aware of that I 
need to be aware of.

Thanks.

Steve