Example: when on the edit tab for a content type, the breadcrumb trail ends with "Content types" as it should. The title of the page is the name of the content type and then the tabs are available as sub pages.

However, if "Manage fields" is selected, then the title of the page is appended to the breadcrumb trail. This is really inconsistent and confusing. The trail should not be affected in any way simply by selecting a different tab.

CommentFileSizeAuthor
#2 fix_breadcrumbs-983294-2.patch10.65 KBidflood
#1 fix_breadcrumbs-983294-1.patch10.65 KBidflood
fix-breadcrumbs.patch10.72 KBjbrown
Non-default tab breadcrumbs - fixed.png52.28 KBjbrown
Non-default tab breadcrumbs.png52.41 KBjbrown
Default tab breadcrumbs.png48.38 KBjbrown

Comments

idflood’s picture

idflood commented
StatusFileSize
new fix_breadcrumbs-983294-1.patch10.65 KB

I tested this and seems to be ok. I've checked this on many default admin pages and this doesn't break anything.

Here is the same patch for d8 with some trailing space removed.

idflood’s picture

idflood commented
Version: 7.x-dev » 8.x-dev
StatusFileSize
new fix_breadcrumbs-983294-2.patch10.65 KB

forgot to change the version. Attaching the same patch to have it tested against 8.x

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Closed (won't fix)

This is the intended and expected behavior, and the only way to provide a breadcrumb that allows to navigate within multiple levels of local tasks.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ b/modules/simpletest/tests/menu.test
@@ -911,15 +911,9 @@ class MenuBreadcrumbTestCase extends DrupalWebTestCase {
     $this->assertBreadcrumb('menu-test/breadcrumb/tasks', $trail, $title, $tree);
     $this->assertBreadcrumb('menu-test/breadcrumb/tasks/first', $trail, $title, $tree);
     $this->assertBreadcrumb('menu-test/breadcrumb/tasks/first/first', $trail, $title, $tree);
-    $trail += array(
-      'menu-test/breadcrumb/tasks' => t('Breadcrumbs test: Local tasks'),
-    );
     $this->assertBreadcrumb('menu-test/breadcrumb/tasks/first/second', $trail, $title, $tree);
     $this->assertBreadcrumb('menu-test/breadcrumb/tasks/second', $trail, $title, $tree);
     $this->assertBreadcrumb('menu-test/breadcrumb/tasks/second/first', $trail, $title, $tree);
-    $trail += array(
-      'menu-test/breadcrumb/tasks/second' => t('Second'),
-    );
     $this->assertBreadcrumb('menu-test/breadcrumb/tasks/second/second', $trail, $title, $tree);

It's exactly this expectation that this patch breaks. As you can probably figure, there was a major reason for explicitly registering a dedicated local tasks structure in menu_test.module and verifying that our expectations are met.

+++ b/modules/simpletest/tests/menu.test
@@ -949,40 +940,37 @@ class MenuBreadcrumbTestCase extends DrupalWebTestCase {
     // Verify Node administration breadcrumbs.
     $trail = $admin + array(
       'admin/structure' => t('Structure'),
-      'admin/structure/types' => t('Content types'),
     );
     $this->assertBreadcrumb('admin/structure/types/add', $trail);

As obvious from this change, the user will have no way to get back to the content types page when she's on the "Add" local action page. That's the second fundamental regression that this patch would introduce.

Powered by Dreditor.