Add menu name to breadcrumb on List Links page

This code will add the menu title to the breadcrumb and page title when placed in edit_menu_item(). I will roll a patch when I get the time

  // Get the human readable menu title from the given menu name
  $titles = menu_get_menus();
  $current_title = $titles[$item['menu_name']];
 
  // Set the breadcrumb to include the menu title of the menu item we are currently editing
  $breadcrumb = menu_get_active_breadcrumb();
  $breadcrumb[] = l($current_title, "admin/structure/menu/manage/".$item['menu_name']);
  drupal_set_breadcrumb($breadcrumb);

  // Set the title of the page to include the menu title of the menu item we are currently editing
  drupal_set_title(t("%name: Edit item link", array('%name' => $current_title)), PASS_THROUGH);

Heres the patch

Oops, when adding a menu, the correct breadcrumb is already displayed! Rerolled with the fix, also changed the title to 'Add menu item' when on the add page, rather the 'Edit menu item'.

Rolled patch with suggestions from #7

#8: 1009832-add-menu-name-3.patch queued for re-testing.

Took a quick look and the patch is failing due to the breadcrumb testing in modules\simpletest\tests\menu.test for these two:

This one needs an additional element (navigation) added to the simpletest:
Line 1113: $this->assertBreadcrumb('admin/structure/menu/manage/navigation', $trail);

This test assertion:
Line 1120: $this->assertBreadcrumb('admin/structure/menu/manage/navigation/add', $trail);
fails because the breadcrumb is displaying:
Home » Administration » Structure » Menus » Navigation » Navigation
Is the double navigation in the breadcrumb the desired behavior?

Passing it back for discussion - whether to update the simpletest or the code (esp for the latter).

This patch should do the trick. We can't seem to test for $this->assertBreadcrumb('admin/structure/menu/manage/navigation', $trail);

Realistically this should have a url like navigation/list similar to /edit and /add.

If we leave that line in, it tries to test a breadcrumb with two of the same crumbs in a row which will make the test fail.

#13: add_menu_name_to_breadcrumb_1009832_13.patch queued for re-testing.

Thanks for working on this! Here's a few things to fix:

1. +++ b/core/modules/menu/menu.admin.incundefined
   @@ -348,6 +358,18 @@ function menu_edit_item($form, &$form_state, $type, $item, $menu) {
   +  // print_r($breadcrumb);
   

   Let's please remove the commented debug code. :)

2. +++ b/core/modules/simpletest/tests/menu.testundefined
   @@ -1110,7 +1110,6 @@ class MenuBreadcrumbTestCase extends MenuWebTestCase {
   -    $this->assertBreadcrumb('admin/structure/menu/manage/navigation', $trail);
   

   Rather than removing this assertion, shouldn't we replace it with an updated one?

3. +++ b/core/modules/menu/menu.admin.incundefined
   @@ -80,6 +80,16 @@ function menu_overview_form($form, &$form_state, $menu) {
   +  // Get the human readable menu title from the given menu name
   ...
   +  // Set the breadcrumb to include the menu title of the menu item we are currently editing
   
   @@ -348,6 +358,18 @@ function menu_edit_item($form, &$form_state, $type, $item, $menu) {
   +  // Get the human readable menu title from the given menu name
   ...
   +  // Set the breadcrumb to include the menu title of the menu item we are currently editing
   ...
   +  // Set the title of the page to include the menu title of the menu item we are currently editing
   

   Few points for cleanup here:

   1. Inline comments should always be less than 80 characters. If the comment is longer, wrap it to another line of inline comments.
   2. Inline comments should end in a period.
   3. Minor point of grammar, but "human-readable" should be hyphenated.

   Reference: http://drupal.org/node/1354#inline

4. +++ b/core/modules/menu/menu.admin.incundefined
   @@ -348,6 +358,18 @@ function menu_edit_item($form, &$form_state, $type, $item, $menu) {
        '#description' => t('Optional. In the menu, the heavier links will sink and the lighter links will be positioned nearer the top.'),
      );
      $form['actions']['submit'] = array('#type' => 'submit', '#value' => t('Save'));
   +  ¶
   

   There's a bit of trailing whitespace here.

Also, let's get a screenshot of the change. (Crop the screenshot to only the relevant portion of the screen.) Thanks!

While working on this I noticed quite a difference across different modules in core.
For example:

Taxonomy

Vocabulary "list" page does not have the vocabulary title in the breadcrumb. However, the vocabulary title is put into the breadcrumb on the "edit", "manage fields", and "manage display" pages. And the page title on all of the pages remains just the vocabulary title.

On the "add term" page the breadcrumb shows the vocabulary title in the breadcrumb, and the page title is the vocabulary title. But on the term edit page, the breadcrumb is just "Home -> 'term'" and the title is "term."

Fields

On the content type "edit" page, the breadcrumb does not show the content type title. However, on the rest of the tabs "manage fields," "manage display," etc. the content type does show in the breadcrumb. And like taxonomy, the title remains the same throughout.

Unlike the taxonomy and menu, there is no separate "add" page for fields. The "add" page and "list" page are combined into one interface. But then if we add a field, the first page we get the breadcrumb is:

"Home » Administration » Structure » Content types » Article » Manage fields » Field_Name"

And as soon as we save the field settings and are directed to the field "edit" page the breadcrumb becomes:

"Home » Administration » Structure » Content types » Article » Manage fields"

Notice the field name is not present. But if you go back to either the "field settings" or "widget type" pages, the field name will show back up in the breadcrumb. And across all of these pages, the title of the page just the field name.

Menu

On the Menu list links page, the menu name is not in the breadcrumb, but is in the breadcrumb on the "edit menu" page which links back to the list links page. Both pages have the menu name as the page title. On the add link page the menu title is added to the breadcrumb, and the page title remains the menu title. But on the "edit link" page, the menu title is NOT in the breadcrumb, and the title as changed to "Edit menu link."

SO...

I think the only thing that we can do that makes any sense is add the menu title to the breadcrumb on the "edit link" page. Attached is a patch which adds this. Though since the menu name is already added to the breadcrumb on the "add" page, we only need to add it to the "edit link" page. So since the first line of the function is an if statement checking whether this is an add, i just added an else statement to hold the code.

D7 Patch attached if desired.

#17 - The code corrects the menu on the edit menu link page.
Attached screenshot.

#18 corrects the same in Drupal 7.

Thanks for the screenshot. Can we get a screenshot of the behavior before it is fixed as well?

+++ b/modules/menu/menu.admin.incundefined
@@ -259,6 +259,16 @@ function menu_edit_item($form, &$form_state, $type, $item, $menu) {
+  }	

There's a bunch of trailing whitespace on this line, so let's remove that from the patch.

+++ b/modules/menu/menu.admin.incundefined
@@ -259,6 +259,16 @@ function menu_edit_item($form, &$form_state, $type, $item, $menu) {
+    // Get the human readable menu title from the given menu name

Also if someone wants to add a hyphen to "human-readable" that would be good. :)

Finally, I think we still need an automated test for this.

Here is a patch with a test, and a screenshot.

Thanks @chertzog! I requeued the test on QA.

...And the above is the test in a patch by itself, to expose the presumed failure without the fix.

Test fails alone and passes with the fix; patch looks clean now -- I think this is RTBC (#22). :)

Ok, I think I get it. So #22 is the "before" screenshot, #19 is the "after" screenshot, and the difference is that the name of the menu you're editing is in the breadcrumbs, for better orientation. That seems like a nice improvement to me, and it should even be safe for backport since it doesn't introduce any new strings.

Committed and pushed to 7.x and 8.x. Thanks!

Updating tags per http://drupal.org/node/1517250