#566094 by Arancaytar and smk-ka: Fixed hierarchy generation in menu_tree_data().

2 files changed, 65 insertions, 8 deletions

diff --git a/includes/menu.inc b/includes/menu.inc
index 3a3ef9306..1e6f13445 100644
--- a/includes/menu.inc
+++ b/includes/menu.inc
@@ -1225,28 +1225,29 @@ function menu_tree_data(array $links, array $parents = array(), $depth = 1) {
  * the next menu link.
  */
 function _menu_tree_data(&$links, $parents, $depth) {
-  $done = FALSE;
   $tree = array();
-  while (!$done && $item = array_pop($links)) {
+  while ($item = array_pop($links)) {
     // We need to determine if we're on the path to root so we can later build
     // the correct active trail and breadcrumb.
     $item['in_active_trail'] = in_array($item['mlid'], $parents);
-    // Look ahead to the next link, but leave it on the array so it's available
-    // to other recursive function calls if we return or build a sub-tree.
-    $next = end($links);
     // Add the current link to the tree.
     $tree[$item['mlid']] = array(
       'link' => $item,
       'below' => array(),
     );
+    // Look ahead to the next link, but leave it on the array so it's available
+    // to other recursive function calls if we return or build a sub-tree.
+    $next = end($links);
     // Check whether the next link is the first in a new sub-tree.
     if ($next && $next['depth'] > $depth) {
       // Recursively call _menu_tree_data to build the sub-tree.
       $tree[$item['mlid']]['below'] = _menu_tree_data($links, $parents, $next['depth']);
+      // Fetch next link after filling the sub-tree.
+      $next = end($links);
     }
-    else {
-      // Determine if we should exit the loop and return.
-      $done = (!$next || $next['depth'] < $depth);
+    // Determine if we should exit the loop and return.
+    if (!$next || $next['depth'] < $depth) {
+      break;
     }
   }
   return $tree;
diff --git a/modules/simpletest/tests/menu.test b/modules/simpletest/tests/menu.test
index f373eb8f1..889d877d3 100644
--- a/modules/simpletest/tests/menu.test
+++ b/modules/simpletest/tests/menu.test
@@ -171,3 +171,59 @@ class MenuRebuildTestCase extends DrupalWebTestCase {
   }
 
 }
+
+/**
+ * Menu tree data related tests.
+ */
+class MenuTreeDataTestCase extends DrupalUnitTestCase {
+  /**
+   * Dummy link structure acceptable for menu_tree_data().
+   */
+  var $links = array(
+    1 => array('mlid' => 1, 'depth' => 1),
+    2 => array('mlid' => 2, 'depth' => 1),
+    3 => array('mlid' => 3, 'depth' => 2),
+    4 => array('mlid' => 4, 'depth' => 3),
+    5 => array('mlid' => 5, 'depth' => 1),
+  );
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Menu tree generation',
+      'description' => 'Tests recursive menu tree generation functions.',
+      'group' => 'Menu',
+    );
+  }
+
+  /**
+   * Validate the generation of a proper menu tree hierarchy.
+   */
+  function testMenuTreeData() {
+    $tree = menu_tree_data($this->links);
+
+    // Validate that parent items #1, #2, and #5 exist on the root level.
+    $this->assertSameLink($this->links[1], $tree[1]['link'], t('Parent item #1 exists.'));
+    $this->assertSameLink($this->links[2], $tree[2]['link'], t('Parent item #2 exists.'));
+    $this->assertSameLink($this->links[5], $tree[5]['link'], t('Parent item #5 exists.'));
+
+    // Validate that child item #4 exists at the correct location in the hierarchy.
+    $this->assertSameLink($this->links[4], $tree[2]['below'][3]['below'][4]['link'], t('Child item #4 exists in the hierarchy.'));
+  }
+
+  /**
+   * Check that two menu links are the same by comparing the mlid.
+   *
+   * @param $link1
+   *   A menu link item.
+   * @param $link2
+   *   A menu link item.
+   * @param $message
+   *   The message to display along with the assertion.
+   * @return
+   *   TRUE if the assertion succeeded, FALSE otherwise.
+   */
+  protected function assertSameLink($link1, $link2, $message = '') {
+    return $this->assert($link1['mlid'] == $link2['mlid'], $message ? $message : t('First link is identical to second link'));
+  }
+}
+