#979958 by das-peter, Damien Tournoud: Fixed regression in load_functions detection for menu items

3 files changed, 107 insertions, 40 deletions

diff --git a/includes/menu.inc b/includes/menu.inc
index 382f02cd4..0b8f59436 100644
--- a/includes/menu.inc
+++ b/includes/menu.inc
@@ -3518,9 +3518,20 @@ function _menu_router_build($callbacks) {
             $item['theme arguments'] = $parent['theme arguments'];
           }
         }
-        // Same for load arguments, if the parent path defines any.
-        if (!isset($item['load arguments']) && !empty($parent['load arguments'])) {
-          $item['_load_functions'] = $parent['_load_functions'];
+        // Same for load arguments: if a loader doesn't have any explict
+        // arguments, try to find arguments in the parent.
+        if (!isset($item['load arguments'])) {
+          foreach ($item['_load_functions'] as $k => $function) {
+            // This loader doesn't have any explict arguments...
+            if (!is_array($function)) {
+              // ... check the parent for a loader at the same position
+              // using the same function name and defining arguments...
+              if (isset($parent['_load_functions'][$k]) && is_array($parent['_load_functions'][$k]) && key($parent['_load_functions'][$k]) == $function) {
+                // ... and inherit the arguments on the child.
+                $item['_load_functions'][$k] = array($function => reset($parent['_load_functions'][$k]));
+              }
+            }
+          }
         }
       }
     }
diff --git a/modules/simpletest/tests/menu.test b/modules/simpletest/tests/menu.test
index f46b81df4..c25240e99 100644
--- a/modules/simpletest/tests/menu.test
+++ b/modules/simpletest/tests/menu.test
@@ -475,19 +475,61 @@ class MenuRouterTestCase extends DrupalWebTestCase {
   }
 
   /**
+   * Load the router for a given path.
+   */
+  protected function menuLoadRouter($router_path) {
+    return db_query('SELECT * FROM {menu_router} WHERE path = :path', array(':path' => $router_path))->fetchAssoc();
+  }
+
+  /**
    * Tests inheritance of 'load arguments'.
    */
   function testMenuLoadArgumentsInheritance() {
-    $arg1 = $this->randomName();
-    $arg2 = $this->randomName();
-    $expected = print_r(array('%menu_test_argument' => $arg1, '%' => $arg2), TRUE);
-
-    $this->drupalGet("menu-test/arguments/$arg1/$arg2");
-    $this->assertRaw($expected, t('Inherited load arguments found.'));
-    $this->drupalGet("menu-test/arguments/$arg1/$arg2/default");
-    $this->assertRaw($expected, t('Inherited load arguments found.'));
-    $this->drupalGet("menu-test/arguments/$arg1/$arg2/task");
-    $this->assertRaw($expected, t('Inherited load arguments found.'));
+    $expected = array(
+      'menu-test/arguments/%/%' => array(
+        2 => array('menu_test_argument_load' => array(3)),
+        3 => NULL,
+      ),
+      // Arguments are inherited to normal children.
+      'menu-test/arguments/%/%/default' => array(
+        2 => array('menu_test_argument_load' => array(3)),
+        3 => NULL,
+      ),
+      // Arguments are inherited to tab children.
+      'menu-test/arguments/%/%/task' => array(
+        2 => array('menu_test_argument_load' => array(3)),
+        3 => NULL,
+      ),
+      // Arguments are only inherited to the same loader functions.
+      'menu-test/arguments/%/%/common-loader' => array(
+        2 => array('menu_test_argument_load' => array(3)),
+        3 => 'menu_test_other_argument_load',
+      ),
+      // Arguments are not inherited to children not using the same loader
+      // function.
+      'menu-test/arguments/%/%/different-loaders-1' => array(
+        2 => NULL,
+        3 => 'menu_test_argument_load',
+      ),
+      'menu-test/arguments/%/%/different-loaders-2' => array(
+        2 => 'menu_test_other_argument_load',
+        3 => NULL,
+      ),
+      'menu-test/arguments/%/%/different-loaders-3' => array(
+        2 => NULL,
+        3 => NULL,
+      ),
+      // Explicit loader arguments should not be overriden by parent.
+      'menu-test/arguments/%/%/explicit-arguments' => array(
+        2 => array('menu_test_argument_load' => array()),
+        3 => NULL,
+      ),
+    );
+
+    foreach ($expected as $router_path => $load_functions) {
+      $router_item = $this->menuLoadRouter($router_path);
+      $this->assertIdentical(unserialize($router_item['load_functions']), $load_functions, t('Expected load functions for router %router_path' , array('%router_path' => $router_path)));
+    }
   }
 }
 
diff --git a/modules/simpletest/tests/menu_test.module b/modules/simpletest/tests/menu_test.module
index 4d673e8af..65d5fdaf5 100644
--- a/modules/simpletest/tests/menu_test.module
+++ b/modules/simpletest/tests/menu_test.module
@@ -264,8 +264,7 @@ function menu_test_menu() {
   $items['menu-test/arguments/%menu_test_argument/%'] = array(
     'title' => 'Load arguments inheritance test',
     'load arguments' => array(3),
-    'page callback' => 'menu_test_arguments',
-    'page arguments' => array(2),
+    'page callback' => 'menu_test_callback',
     'access callback' => TRUE,
   );
   $items['menu-test/arguments/%menu_test_argument/%/default'] = array(
@@ -274,44 +273,59 @@ function menu_test_menu() {
   );
   $items['menu-test/arguments/%menu_test_argument/%/task'] = array(
     'title' => 'Local task',
-    'page callback' => 'menu_test_arguments',
-    'page arguments' => array(2),
+    'page callback' => 'menu_test_callback',
+    'access callback' => TRUE,
+    'type' => MENU_LOCAL_TASK,
+  );
+  // For this path, load arguments should be inherited for the first loader only.
+  $items['menu-test/arguments/%menu_test_argument/%menu_test_other_argument/common-loader'] = array(
+    'title' => 'Local task',
+    'page callback' => 'menu_test_callback',
     'access callback' => TRUE,
     'type' => MENU_LOCAL_TASK,
   );
+  // For these paths, no load arguments should be inherited.
+  // Not on the same position.
+  $items['menu-test/arguments/%/%menu_test_argument/different-loaders-1'] = array(
+    'title' => 'An item not sharing the same loader',
+    'page callback' => 'menu_test_callback',
+    'access callback' => TRUE,
+  );
+  // Not the same loader.
+  $items['menu-test/arguments/%menu_test_other_argument/%/different-loaders-2'] = array(
+    'title' => 'An item not sharing the same loader',
+    'page callback' => 'menu_test_callback',
+    'access callback' => TRUE,
+  );
+  // Not the same loader.
+  $items['menu-test/arguments/%/%/different-loaders-3'] = array(
+    'title' => 'An item not sharing the same loader',
+    'page callback' => 'menu_test_callback',
+    'access callback' => TRUE,
+  );
+  // Explict load arguments should not be overriden (even if empty).
+  $items['menu-test/arguments/%menu_test_argument/%/explicit-arguments'] = array(
+    'title' => 'An item defining explicit load arguments',
+    'load arguments' => array(),
+    'page callback' => 'menu_test_callback',
+    'access callback' => TRUE,
+  );
 
   return $items;
 }
 
 /**
- * Menu argument loader for the 'load arguments' inheritance test.
- *
- * @param $arg1
- *   The path argument defined as %menu_test_argument.
- * @param $arg2
- *   The additional path argument that should be passed due to 'load arguments'.
- *
- * @return
- *   An array containing $arg1 and $arg2.
+ * Dummy argument loader for hook_menu() to point to.
  */
-function menu_test_argument_load($arg1, $arg2) {
-  return array('%menu_test_argument' => $arg1, '%' => $arg2);
+function menu_test_argument_load($arg1) {
+  return FALSE;
 }
 
 /**
- * Page callback for the 'load arguments' inheritance test.
- *
- * @param $joined_args
- *   An associative array containing the values for both path arguments, as
- *   intercepted and processed by menu_test_argument_load():
- *   - %menu_test_argument: The primary path argument string.
- *   - %: The additional argument string.
- *
- * @return
- *   A dump of the loaded %menu_test_argument argument.
+ * Dummy argument loader for hook_menu() to point to.
  */
-function menu_test_arguments($joined_args) {
-  return print_r($joined_args, TRUE);
+function menu_test_other_argument_load($arg1) {
+  return FALSE;
 }
 
 /**