Incorrect use of hook_js_alter() [#1966832]

I was using the Advanced Aggregation module, and found a small bug in the implementation of hook_add_js(): it does not use the $javascript as a reference, so the JavaScript is never added when the Advanced Aggregation is enabled.

The correct implementation should be:

/**
 * Implements hook_js_alter().
 */
function clientside_validation_js_alter(&$javascript) {
  $clientside_validation_settings = &drupal_static('clientside_validation_settings', array());
  if (isset($clientside_validation_settings['clientsideValidation'])) {
    foreach ($clientside_validation_settings['clientsideValidation']['forms'] as $form_id => $settings) {
      $stripped_form_id = $form_id;
      $matches = array();
      preg_match(' /.*?(--(\d)*)/', $form_id, $matches);
      if (isset($matches[1])) {
        $stripped_form_id = str_replace($matches[1], '', $form_id);
      }
      if ($stripped_form_id != $form_id) {
        $clientside_validation_settings['clientsideValidation']['forms'][$form_id] = $clientside_validation_settings['clientsideValidation']['forms'][$stripped_form_id];
        $clientside_validation_settings['clientsideValidation']['groups'][$form_id] = $clientside_validation_settings['clientsideValidation']['groups'][$stripped_form_id];
      }
    }
  }
  $javascript = drupal_add_js($clientside_validation_settings, 'setting');

}

Comments

Comment #1

attiks CreditAttribution: attiks commented 16 April 2013 at 17:58

Can you provide a patch?

Comment #2

adam_bear

English

Tahoe

CreditAttribution: adam_bear commented 13 June 2013 at 03:46

Yep... this function is buggy. In addition to the missing $js param, it needs to be assigned to the module js group.

$clientside_validation_settings['clientsideValidation']['group'] = JS_DEFAULT

Comment #3

amberau79 CreditAttribution: amberau79 commented 28 June 2013 at 01:06

Note that implementing this way also fixes a bug where the settings were stripped if the scope of the js settings was set to footer. Thank you!!

Comment #4

Jelle_S

Dutch

Antwerp, Belgium

CreditAttribution: Jelle_S commented 4 July 2013 at 08:23

Version:	7.x-1.37	» 7.x-1.x-dev
Status:	Active	» Fixed

Fixed in the latest dev version. Thanks for the report

Comment #5

18 July 2013 at 08:31

Status:

Fixed

» Closed (fixed)

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

Comment #6

vflirt CreditAttribution: vflirt commented 6 August 2013 at 11:23

Status:

Closed (fixed)

» Active

Hi,

i have to reopen this because this is actually breaking the locale js. If you look at the locale_js_alter you will see that it adds the translation javascript file but because you overwrite the javascript array you are actually removing this. You should only add your things and/or edit current records in the array. Changing all the javascript array is just breaking th site js and therefor you are not getting any javascript translations so Drupal.t() will not work at all.

What I think should be done is :

$javascript['settings']['data'][] = $clientside_validation_settings;
instead of
$javascript = drupal_add_js($clientside_validation_settings, 'setting');

This way only the needed settings are added to the javsacript array that will be compressed later on.
Hope someone else could trow a thought on this.

Kind Regards