Rebuilding phptemplate variable $scripts after already build may not add drupal.js and jquery.js

My first pass at a patch for this. Only building the $javascript array if there is data to populate it.

My first pass at a patch for this. Only building the $javascript array if there is data to populate it.

Another version with a little less logic.

@Gábor - I think this is critical because without a change drupal_add_js won't add jquery.js or drupal.js after template_preprocess_page [1]. This can cause problems for themes wanting to add javascript. Currently, in functions in template.php you would need to either manually add the javascript html to the $scripts (which won't go through javascript preprocessing) or you need to do a drupal_add_js for jquery.js and drupal.js.

This is because drupal_get_js is called in template_preprocess_page. drupal_get_js calls drupal_add_js with a scope but no data causing

  if (isset($scope) && !isset($javascript[$scope])) {
    $javascript[$scope] = array('core' => array(), 'module' => array(), 'theme' => array(), 'setting' => array(), 'inline' => array());

to happen. This populates the $javascript array so that when drupal_add_js is called again

 if (isset($data) && empty($javascript)) {
    $javascript['header'] = array(
      'core' => array(
        'misc/jquery.js' => array('cache' => TRUE, 'defer' => FALSE, 'preprocess' => TRUE),
        'misc/drupal.js' => array('cache' => TRUE, 'defer' => FALSE, 'preprocess' => TRUE),
      ),
      'module' => array(), 'theme' => array(), 'setting' => array(), 'inline' => array(),
    );
  }

doesn't happen.

The attached patch matches the functionality of D5 to add drupal.js and jquery.js any time drupal_add_js is called with data. It, also, checks if a $javascript[$scope] is set and only returns data if it is. This removes some 'Undefined index' messages. $javascript[$scope] is only populated if there is data now where before it was always populated.

Responding to #4

Why isn't the header scope properly initialized? Your first call to drupal_add_js() should trigger the if (isset($data) && empty($javascript)) { line in drupal_add_js(). This is not changed with your patch either. You now do if (isset($data)) { if (empty($javascript)) { which should do the same. I don't understand this.

You are right. What I did ended up removing one isset($data) call. It went from:

if (isset($data) && empty($javascript)) {
...
}
if (isset($data)) {
  ...
}

to

if (isset($data)) {
  if (empty($javascript)) {
    ...
  }
  ...
}

This part could probably be reworked to use the isset($scope) wrapper, and then do if (isset($javascript[$scope])) inside. But then this function is documented to return an array in all cases, so why do you return an empty string and not an empty array?

Included both of these in this patch.

[1] http://drupal.org/files/theme_flow_6_0.pdf