- Patch #765860 by effulgentsia, dww, dereine, mikey_p, xjm, sun, sven.lauer: drupal_alter() fails to order modules correctly in some cases.

4 files changed, 61 insertions, 2 deletions

diff --git a/includes/module.inc b/includes/module.inc
index 66c77f577..3a019f26d 100644
--- a/includes/module.inc
+++ b/includes/module.inc
@@ -952,10 +952,24 @@ function drupal_alter($type, &$data, &$context1 = NULL, &$context2 = NULL) {
       }
       // If any modules implement one of the extra hooks that do not implement
       // the primary hook, we need to add them to the $modules array in their
-      // appropriate order.
+      // appropriate order. module_implements() can only return ordered
+      // implementations of a single hook. To get the ordered implementations
+      // of multiple hooks, we mimic the module_implements() logic of first
+      // ordering by module_list(), and then calling
+      // drupal_alter('module_implements').
       if (array_diff($extra_modules, $modules)) {
-        // Order the modules by the order returned by module_list().
+        // Merge the arrays and order by module_list().
         $modules = array_intersect(module_list(), array_merge($modules, $extra_modules));
+        // Since module_implements() already took care of loading the necessary
+        // include files, we can safely pass FALSE for the array values.
+        $implementations = array_fill_keys($modules, FALSE);
+        // Let modules adjust the order solely based on the primary hook. This
+        // ensures the same module order regardless of whether this if block
+        // runs. Calling drupal_alter() recursively in this way does not result
+        // in an infinite loop, because this call is for a single $type, so we
+        // won't end up in this code block again.
+        drupal_alter('module_implements', $implementations, $hook);
+        $modules = array_keys($implementations);
       }
       foreach ($modules as $module) {
         // Since $modules is a merged array, for any given module, we do not
diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test
index c266dc3b4..a643ff94f 100644
--- a/modules/simpletest/tests/common.test
+++ b/modules/simpletest/tests/common.test
@@ -56,6 +56,14 @@ class DrupalAlterTestCase extends DrupalWebTestCase {
     $this->assertEqual($array_copy, $array_expected, t('First argument to drupal_alter() was altered.'));
     $this->assertEqual($entity_copy, $entity_expected, t('Second argument to drupal_alter() was altered.'));
     $this->assertEqual($array2_copy, $array2_expected, t('Third argument to drupal_alter() was altered.'));
+
+    // Verify alteration order when passing an array of types to drupal_alter().
+    // common_test_module_implements_alter() places 'block' implementation after
+    // other modules.
+    $array_copy = $array;
+    $array_expected = array('foo' => 'Drupal block theme');
+    drupal_alter(array('drupal_alter', 'drupal_alter_foo'), $array_copy);
+    $this->assertEqual($array_copy, $array_expected, t('hook_TYPE_alter() implementations ran in correct order.'));
   }
 }
 
diff --git a/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module
index c400eaed1..e75b45237 100644
--- a/modules/simpletest/tests/common_test.module
+++ b/modules/simpletest/tests/common_test.module
@@ -166,6 +166,34 @@ function bartik_drupal_alter_alter(&$data, &$arg2 = NULL, &$arg3 = NULL) {
 }
 
 /**
+ * Implements hook_TYPE_alter() on behalf of block module.
+ *
+ * This is for verifying that drupal_alter(array(TYPE1, TYPE2), ...) allows
+ * hook_module_implements_alter() to affect the order in which module
+ * implementations are executed.
+ */
+function block_drupal_alter_foo_alter(&$data, &$arg2 = NULL, &$arg3 = NULL) {
+  $data['foo'] .= ' block';
+}
+
+/**
+ * Implements hook_module_implements_alter().
+ *
+ * @see block_drupal_alter_foo_alter()
+ */
+function common_test_module_implements_alter(&$implementations, $hook) {
+  // For drupal_alter(array('drupal_alter', 'drupal_alter_foo'), ...), make the
+  // block module implementations run after all the other modules. Note that
+  // when drupal_alter() is called with an array of types, the first type is
+  // considered primary and controls the module order.
+  if ($hook == 'drupal_alter_alter' && isset($implementations['block'])) {
+    $group = $implementations['block'];
+    unset($implementations['block']);
+    $implementations['block'] = $group;
+  }
+}
+
+/**
  * Implements hook_theme().
  */
 function common_test_theme() {
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index 1e8d35a78..7ffdae4ea 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -1877,6 +1877,15 @@ function hook_mail_alter(&$message) {
  * hook in order to reorder the implementing modules, which are otherwise
  * ordered by the module's system weight.
  *
+ * Note that hooks invoked using drupal_alter() can have multiple variations
+ * (such as hook_form_alter() and hook_form_FORM_ID_alter()). drupal_alter()
+ * will call all such variants defined by a single module in turn. For the
+ * purposes of hook_module_implements_alter(), these variants are treated as
+ * a single hook. Thus, to ensure that your implementation of
+ * hook_form_FORM_ID_alter() is called at the right time, you will have to
+ * have to change the order of hook_form_alter() implementation in
+ * hook_module_implements_alter().
+ *
  * @param $implementations
  *   An array keyed by the module's name. The value of each item corresponds
  *   to a $group, which is usually FALSE, unless the implementation is in a