#208611 by effulgentsia, ksenzee, p.brouwers, et al: Fixed drupal_add_js() includes settings twice and breaks certain JS files.

4 files changed, 130 insertions, 6 deletions

diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc
index 256121c3e..84fdd9833 100644
--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -1877,6 +1877,79 @@ function drupal_hash_base64($data) {
 }
 
 /**
+ * Merges multiple arrays, recursively, and returns the merged array.
+ *
+ * This function is similar to PHP's array_merge_recursive() function, but it
+ * handles non-array values differently. When merging values that are not both
+ * arrays, the latter value replaces the former rather than merging with it.
+ *
+ * Example:
+ * @code
+ * $link_options_1 = array('fragment' => 'x', 'attributes' => array('title' => t('X'), 'class' => array('a', 'b')));
+ * $link_options_2 = array('fragment' => 'y', 'attributes' => array('title' => t('Y'), 'class' => array('c', 'd')));
+ *
+ * // This results in array('fragment' => array('x', 'y'), 'attributes' => array('title' => array(t('X'), t('Y')), 'class' => array('a', 'b', 'c', 'd'))).
+ * $incorrect = array_merge_recursive($link_options_1, $link_options_2);
+ *
+ * // This results in array('fragment' => 'y', 'attributes' => array('title' => t('Y'), 'class' => array('a', 'b', 'c', 'd'))).
+ * $correct = drupal_array_merge_deep($link_options_1, $link_options_2);
+ * @endcode
+ *
+ * @param ...
+ *   Arrays to merge.
+ *
+ * @return
+ *   The merged array.
+ *
+ * @see drupal_array_merge_deep_array()
+ */
+function drupal_array_merge_deep() {
+  return drupal_array_merge_deep_array(func_get_args());
+}
+
+/**
+ * Merges multiple arrays, recursively, and returns the merged array.
+ *
+ * This function is equivalent to drupal_array_merge_deep(), except the
+ * input arrays are passed as a single array parameter rather than a variable
+ * parameter list.
+ *
+ * The following are equivalent:
+ * - drupal_array_merge_deep($a, $b);
+ * - drupal_array_merge_deep_array(array($a, $b));
+ *
+ * The following are also equivalent:
+ * - call_user_func_array('drupal_array_merge_deep', $arrays_to_merge);
+ * - drupal_array_merge_deep_array($arrays_to_merge);
+ *
+ * @see drupal_array_merge_deep()
+ */
+function drupal_array_merge_deep_array($arrays) {
+  $result = array();
+
+  foreach ($arrays as $array) {
+    foreach ($array as $key => $value) {
+      // Renumber integer keys as array_merge_recursive() does. Note that PHP
+      // automatically converts array keys that are integer strings (e.g., '1')
+      // to integers.
+      if (is_integer($key)) {
+        $result[] = $value;
+      }
+      // Recurse when both values are arrays.
+      elseif (isset($result[$key]) && is_array($result[$key]) && is_array($value)) {
+        $result[$key] = drupal_array_merge_deep_array(array($result[$key], $value));
+      }
+      // Otherwise, use the latter value, overriding any previous value.
+      else {
+        $result[$key] = $value;
+      }
+    }
+  }
+
+  return $result;
+}
+
+/**
  * Generates a default anonymous $user object.
  *
  * @return Object - the user object.
diff --git a/includes/common.inc b/includes/common.inc
index 7a9dff388..4ad8e08d8 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -3751,6 +3751,7 @@ function drupal_region_class($region) {
  *     array('type' => 'inline', 'scope' => 'footer', 'weight' => 5)
  *   );
  *   drupal_add_js('http://example.com/example.js', 'external');
+ *   drupal_add_js(array('myModule', array('key' => 'value')), 'setting');
  * @endcode
  *
  * Calling drupal_static_reset('drupal_add_js') will clear all JavaScript added
@@ -3783,9 +3784,11 @@ function drupal_region_class($region) {
  *     hosted on the local server. These files will not be aggregated if
  *     JavaScript aggregation is enabled.
  *   - 'setting': An associative array with configuration options. The array is
- *     directly placed in Drupal.settings. All modules should wrap their actual
- *     configuration settings in another variable to prevent conflicts in the
- *     Drupal.settings namespace.
+ *     merged directly into Drupal.settings. All modules should wrap their
+ *     actual configuration settings in another variable to prevent conflicts in
+ *     the Drupal.settings namespace. Items added with a string key will replace
+ *     existing settings with that key; items with numeric array keys will be
+ *     added to the existing settings array.
  * @param $options
  *   (optional) A string defining the type of JavaScript that is being added in
  *   the $data parameter ('file'/'setting'/'inline'/'external'), or an
@@ -4070,7 +4073,7 @@ function drupal_get_js($scope = 'header', $javascript = NULL, $skip_alter = FALS
       case 'setting':
         $js_element = $element;
         $js_element['#value_prefix'] = $embed_prefix;
-        $js_element['#value'] = 'jQuery.extend(Drupal.settings, ' . drupal_json_encode(call_user_func_array('array_merge_recursive', $item['data'])) . ");";
+        $js_element['#value'] = 'jQuery.extend(Drupal.settings, ' . drupal_json_encode(drupal_array_merge_deep_array($item['data'])) . ");";
         $js_element['#value_suffix'] = $embed_suffix;
         $output .= theme('html_tag', array('element' => $js_element));
         break;
diff --git a/modules/simpletest/tests/bootstrap.test b/modules/simpletest/tests/bootstrap.test
index d8a9d8c1b..99bdca774 100644
--- a/modules/simpletest/tests/bootstrap.test
+++ b/modules/simpletest/tests/bootstrap.test
@@ -426,3 +426,28 @@ class BootstrapResettableStaticTestCase extends DrupalUnitTestCase {
     $this->assertEqual($var, 'foo', t('Variable was reset after second invocation of global reset.'));
   }
 }
+
+/**
+ * Test miscellaneous functions in bootstrap.inc.
+ */
+class BootstrapMiscTestCase extends DrupalUnitTestCase {
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Miscellaneous bootstrap unit tests',
+      'description' => 'Test miscellaneous functions in bootstrap.inc.',
+      'group' => 'Bootstrap',
+    );
+  }
+
+  /**
+   * Test miscellaneous functions in bootstrap.inc.
+   */
+  function testMisc() {
+    // Test drupal_array_merge_deep().
+    $link_options_1 = array('fragment' => 'x', 'attributes' => array('title' => 'X', 'class' => array('a', 'b')), 'language' => 'en');
+    $link_options_2 = array('fragment' => 'y', 'attributes' => array('title' => 'Y', 'class' => array('c', 'd')), 'html' => TRUE);
+    $expected = array('fragment' => 'y', 'attributes' => array('title' => 'Y', 'class' => array('a', 'b', 'c', 'd')), 'language' => 'en', 'html' => TRUE);
+    $this->assertIdentical(drupal_array_merge_deep($link_options_1, $link_options_2), $expected, t('drupal_array_merge_deep() returned a properly merged array.'));
+  }
+}
diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test
index 83f2e9734..9dc2532ab 100644
--- a/modules/simpletest/tests/common.test
+++ b/modules/simpletest/tests/common.test
@@ -1195,11 +1195,34 @@ class JavaScriptTestCase extends DrupalWebTestCase {
    * Test drupal_get_js() for JavaScript settings.
    */
   function testHeaderSetting() {
-    drupal_add_js(array('testSetting' => 'testValue'), 'setting');
+    // Only the second of these two entries should appear in Drupal.settings.
+    drupal_add_js(array('commonTest' => 'commonTestShouldNotAppear'), 'setting');
+    drupal_add_js(array('commonTest' => 'commonTestShouldAppear'), 'setting');
+    // All three of these entries should appear in Drupal.settings.
+    drupal_add_js(array('commonTestArray' => array('commonTestValue0')), 'setting');
+    drupal_add_js(array('commonTestArray' => array('commonTestValue1')), 'setting');
+    drupal_add_js(array('commonTestArray' => array('commonTestValue2')), 'setting');
+    // Only the second of these two entries should appear in Drupal.settings.
+    drupal_add_js(array('commonTestArray' => array('key' => 'commonTestOldValue')), 'setting');
+    drupal_add_js(array('commonTestArray' => array('key' => 'commonTestNewValue')), 'setting');
+
     $javascript = drupal_get_js('header');
     $this->assertTrue(strpos($javascript, 'basePath') > 0, t('Rendered JavaScript header returns basePath setting.'));
-    $this->assertTrue(strpos($javascript, 'testSetting') > 0, t('Rendered JavaScript header returns custom setting.'));
     $this->assertTrue(strpos($javascript, 'misc/jquery.js') > 0, t('Rendered JavaScript header includes jQuery.'));
+
+    // Test whether drupal_add_js can be used to override a previous setting.
+    $this->assertTrue(strpos($javascript, 'commonTestShouldAppear') > 0, t('Rendered JavaScript header returns custom setting.'));
+    $this->assertTrue(strpos($javascript, 'commonTestShouldNotAppear') === FALSE, t('drupal_add_js() correctly overrides a custom setting.'));
+
+    // Test whether drupal_add_js can be used to add numerically indexed values
+    // to an array.
+    $array_values_appear = strpos($javascript, 'commonTestValue0') > 0 && strpos($javascript, 'commonTestValue1') > 0 && strpos($javascript, 'commonTestValue2') > 0;
+    $this->assertTrue($array_values_appear, t('drupal_add_js() correctly adds settings to the end of an indexed array.'));
+
+    // Test whether drupal_add_js can be used to override the entry for an
+    // existing key in an associative array.
+    $associative_array_override = strpos($javascript, 'commonTestNewValue') > 0 && strpos($javascript, 'commonTestOldValue') === FALSE;
+    $this->assertTrue($associative_array_override, t('drupal_add_js() correctly overrides settings within an associative array.'));
   }
 
   /**