#576290: fix the breadcrumb and active-trail logic. From: Damien Tournoud --- common.inc | 4 + menu.inc | 199 +++++++++++++++++++++++++++++++++--------------------- menu/menu.module | 23 ++---- menu/menu.test | 4 + 4 files changed, 133 insertions(+), 97 deletions(-) diff --git includes/common.inc includes/common.inc index 8ecd247..56c868b 100644 --- includes/common.inc +++ includes/common.inc @@ -245,7 +245,7 @@ function drupal_get_profile() { function drupal_set_breadcrumb($breadcrumb = NULL) { $stored_breadcrumb = &drupal_static(__FUNCTION__); - if (!is_null($breadcrumb)) { + if (isset($breadcrumb)) { $stored_breadcrumb = $breadcrumb; } return $stored_breadcrumb; @@ -257,7 +257,7 @@ function drupal_set_breadcrumb($breadcrumb = NULL) { function drupal_get_breadcrumb() { $breadcrumb = drupal_set_breadcrumb(); - if (is_null($breadcrumb)) { + if (!isset($breadcrumb)) { $breadcrumb = menu_get_active_breadcrumb(); } diff --git includes/menu.inc includes/menu.inc index 2951129..d7cad1f 100644 --- includes/menu.inc +++ includes/menu.inc @@ -1084,49 +1084,27 @@ function menu_tree_page_data($menu_name, $max_depth = NULL) { if (!isset($data)) { // Build and run the query, and build the tree. if ($item['access']) { - // Check whether a menu link exists that corresponds to the current path. - $args[] = $item['href']; - if (drupal_is_front_page()) { - $args[] = ''; - } - $parents = db_select('menu_links') - ->fields('menu_links', array( - 'p1', - 'p2', - 'p3', - 'p4', - 'p5', - 'p6', - 'p7', - 'p8', - )) - ->condition('menu_name', $menu_name) - ->condition('link_path', $args, 'IN') - ->execute()->fetchAssoc(); - - if (empty($parents)) { - // If no link exists, we may be on a local task that's not in the links. - // TODO: Handle the case like a local task on a specific node in the menu. - $parents = db_select('menu_links') - ->fields('menu_links', array( - 'p1', - 'p2', - 'p3', - 'p4', - 'p5', - 'p6', - 'p7', - 'p8', - )) - ->condition('menu_name', $menu_name) - ->condition('link_path', $item['tab_root']) - ->execute()->fetchAssoc(); + // Parent mlids. + $parents = array(); + + // Find a menu link corresponding to the current path. + $top_link = menu_link_get_prefered(); + + if ($top_link) { + // Use all the coordinates, except the last one because there + // can be no child beyond the last column. + for ($i = 1; $i < MENU_MAX_DEPTH; $i++) { + if ($top_link['p' . $i]) { + $parents[] = $top_link['p' . $i]; + } + } } + // We always want all the top-level links with plid == 0. $parents[] = '0'; // Use array_values() so that the indices are numeric for array_merge(). - $args = $parents = array_unique(array_values($parents)); + $args = $parents = array_values(array_unique($parents)); $expanded = variable_get('menu_expanded', array()); // Check whether the current menu has any links set to be expanded. if (in_array($menu_name, $expanded)) { @@ -1994,52 +1972,24 @@ function menu_set_active_trail($new_trail = NULL) { elseif (!isset($trail)) { $trail = array(); $trail[] = array('title' => t('Home'), 'href' => '', 'localized_options' => array(), 'type' => 0); - $item = menu_get_item(); - // Check whether the current item is a local task (displayed as a tab). - if ($item['tab_parent']) { - // The title of a local task is used for the tab, never the page title. - // Thus, replace it with the item corresponding to the root path to get - // the relevant href and title. For example, the menu item corresponding - // to 'admin' is used when on the 'By module' tab at 'admin/by-module'. - $parts = explode('/', $item['tab_root']); - $args = arg(); - // Replace wildcards in the root path using the current path. - foreach ($parts as $index => $part) { - if ($part == '%') { - $parts[$index] = $args[$index]; - } - } - // Retrieve the menu item using the root path after wildcard replacement. - $root_item = menu_get_item(implode('/', $parts)); - if ($root_item && $root_item['access']) { - $item = $root_item; + $prefered_link = menu_link_get_prefered(); + if ($prefered_link) { + $tree = menu_tree_page_data($prefered_link['menu_name']); + if ($router = menu_get_item(_menu_fill_wildcards($prefered_link['link_path'], arg()))) { + $prefered_link += $router; } + list($key, $curr) = each($tree); } - $menu_names = menu_get_active_menu_names(); - $curr = FALSE; - // Determine if the current page is a link in any of the active menus. - if ($menu_names) { - $query = db_select('menu_links', 'ml'); - $query->fields('ml', array('menu_name')); - $query->condition('ml.link_path', $item['href']); - $query->condition('ml.menu_name', $menu_names, 'IN'); - $result = $query->execute(); - $found = array(); - foreach ($result as $menu) { - $found[] = $menu->menu_name; - } - // The $menu_names array is ordered, so take the first one that matches. - $name = current(array_intersect($menu_names, $found)); - if ($name !== FALSE) { - $tree = menu_tree_page_data($name); - list($key, $curr) = each($tree); - } + else { + $prefered_link = menu_get_item(); + $key = NULL; + $curr = NULL; } while ($curr) { // Terminate the loop when we find the current path in the active trail. - if ($curr['link']['href'] == $item['href']) { + if ($curr['link']['href'] == $prefered_link['href']) { $trail[] = $curr['link']; $curr = FALSE; } @@ -2055,14 +2005,105 @@ function menu_set_active_trail($new_trail = NULL) { // Make sure the current page is in the trail (needed for the page title), // but exclude tabs and the front page. $last = count($trail) - 1; - if ($trail[$last]['href'] != $item['href'] && !(bool)($item['type'] & MENU_IS_LOCAL_TASK) && !drupal_is_front_page()) { - $trail[] = $item; + if ($trail[$last]['href'] != $prefered_link['href'] && !(bool)($prefered_link['type'] & MENU_IS_LOCAL_TASK) && !drupal_is_front_page()) { + $trail[] = $prefered_link; } } return $trail; } /** + * Load the prefered menu link for a given path. + * + * @param $path + * The path, for example node/5. The function will find the corresponding + * menu link (node/5 if it exists, or fallback to node/%). + * @return + * A fully translated menu link, or NULL if not matching menu link was + * found. The most specific menu link (node/5 prefered over node/%) in the + * most prefered menu (as defined by menu_get_active_menu_names()) is returned. + */ +function menu_link_get_prefered($href = NULL, $check_access = TRUE) { + $prefered_links = &drupal_static(__FUNCTION__); + + if (!isset($href)) { + $href = $_GET['q']; + } + + if (!isset($prefered_links[$href])) { + $item = menu_get_item($href); + + // We look for the correct menu link by building a list of candidate + // paths. We pick the most relevant path in the prefered menu. + $path_candidates = array(); + // 1. The current item href. + $path_candidates[] = $item['href']; + // 2. The current item path (with wildcards). + $path_candidates[] = $item['path']; + if ($item['tab_parent']) { + // 3. The href of the current item tab root (if its exists). + $path_candidates[] = _menu_fill_wildcards($item['tab_root'], $item['original_map']); + // 4. The path of the current item tab root (if its exists). + $path_candidates[] = $item['tab_root']; + } + + // The list of prefered menu names, in preference order. + $menu_names = menu_get_active_menu_names(); + + // Build the query. + $query = db_select('menu_links', 'ml', array('fetch' => PDO::FETCH_ASSOC)); + $query->addTag('translatable'); + $query->fields('ml'); + $query->condition('ml.link_path', $path_candidates, 'IN'); + $query->condition('ml.menu_name', $menu_names, 'IN'); + + // Sort candidates by link path and menu name. + $candidates = array(); + foreach ($query->execute() as $candidate) { + $candidates[$candidate['link_path']][$candidate['menu_name']] = $candidate; + } + + // Pick the most specific link, in the prefered menu. + $prefered_links[$href] = NULL; + foreach ($path_candidates as $path) { + if (!isset($candidates[$path])) { + continue; + } + foreach ($menu_names as $menu_name) { + if (!isset($candidates[$path][$menu_name])) { + continue; + } + + $prefered_links[$href] = $candidates[$path][$menu_name]; + break 2; + } + } + } + + return $prefered_links[$href]; +} + +/** + * Replace a path wildcard with elements from a map. + * + * @param $path + * A menu path (ie. 'node/%/edit'). + * @param $map + * A menu map (ie. array('node', '12')). + * @return + * A completed menu path (ie. 'node/12/edit'). + */ +function _menu_fill_wildcards($path, array $map) { + $parts = arg(NULL, $path); + foreach ($parts as $index => $part) { + if ($part == '%' && isset($map[$index])) { + $parts[$index] = $map[$index]; + } + } + return implode('/', $parts); +} + +/** * Gets the active trail (path to root menu root) of the current page. * * See menu_set_active_trail() for details of return value. diff --git modules/menu/menu.module modules/menu/menu.module index d8ddffc..c3157eb 100644 --- modules/menu/menu.module +++ modules/menu/menu.module @@ -222,8 +222,6 @@ function menu_load($menu_name) { * - menu_name: The unique name of the custom menu. * - title: The human readable menu title. * - description: The custom menu description. - * - old_name: For existing menus, the current 'menu_name', otherwise empty. - * Decides whether hook_menu_insert() or hook_menu_update() will be invoked. * * Modules should always pass a fully populated $menu when saving a custom * menu, so other modules are able to output proper status or watchdog messages. @@ -231,7 +229,7 @@ function menu_load($menu_name) { * @see menu_load() */ function menu_save($menu) { - db_merge('menu_custom') + $status = db_merge('menu_custom') ->key(array('menu_name' => $menu['menu_name'])) ->fields(array( 'title' => $menu['title'], @@ -239,17 +237,14 @@ function menu_save($menu) { )) ->execute(); - // Since custom menus are keyed by name and their machine-name cannot be - // changed, there is no real differentiation between inserting and updating a - // menu. To overcome this, we define the existing menu_name as 'old_name' in - // menu_edit_menu(). - // @todo Replace this condition when db_merge() returns the proper query - // result (insert/update). - if (!empty($menu['old_name'])) { - module_invoke_all('menu_update', $menu); - } - else { - module_invoke_all('menu_insert', $menu); + switch ($status) { + case SAVED_NEW: + module_invoke_all('menu_insert', $menu); + break; + + case SAVED_UPDATED: + module_invoke_all('menu_update', $menu); + break; } } diff --git modules/menu/menu.test modules/menu/menu.test index 3c6f63b..f5e4fd1 100644 --- modules/menu/menu.test +++ modules/menu/menu.test @@ -112,14 +112,14 @@ class MenuTestCase extends DrupalWebTestCase { // Assert the new menu. $this->drupalGet('admin/structure/menu/manage/' . $menu_name . '/edit'); - $this->assertText($title, t('Custom menu was added.')); + $this->assertRaw($title, t('Custom menu was added.')); // Edit the menu. $new_title = $this->randomName(16); $menu['title'] = $new_title; menu_save($menu); $this->drupalGet('admin/structure/menu/manage/' . $menu_name . '/edit'); - $this->assertText($new_title, t('Custom menu was edited.')); + $this->assertRaw($new_title, t('Custom menu was edited.')); } /**