If we have a site menu like Parent(mlid=1150)->Child(mlid=1147)->Node(mlid=2200) breadcrumb generated with token for Node(mlid=2200) would be Child(mlid=1147)->Parent(mlid=1150), instead of Parent(mlid=1150)->Child(mlid=1147)

Child mlid

Suggestion:

Replace LINE 131

// Parent ids of menu item.
        $pids = array(
          $menu_item->plid
          $menu_item->p1, $menu_item->p2, $menu_item->p3,
          $menu_item->p4, $menu_item->p5, $menu_item->p6,
          $menu_item->p7, $menu_item->p8, $menu_item->p9,
       
        );

with

// Parent ids of menu item.
        $pids = array(
          $menu_item->p1, $menu_item->p2, $menu_item->p3,
          $menu_item->p4, $menu_item->p5, $menu_item->p6,
          $menu_item->p7, $menu_item->p8, $menu_item->p9,
          $menu_item->plid
        );

and insert
ksort($trail);

before function return.

Don't know for sure if this fix is correct.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Ryotsuke’s picture

Issue was cut by editor. I'll continue in comment:

Child mlid is less than Parent mlid

Suggestion is to replace code on LINE 131 of custom_breadcrumbs_identifiers.module with

$pids = array(
          $menu_item->p1, $menu_item->p2, $menu_item->p3,
          $menu_item->p4, $menu_item->p5, $menu_item->p6,
          $menu_item->p7, $menu_item->p8, $menu_item->p9,
          $menu_item->plid
        );

and to add ksort($trail);

before return $trail;

Don't know if this is a correct fix

jefflogan’s picture

I can confirm the above fix works for me.... the issue has been bugging me for ages, so I am glad you shared how you solved this.

I haven't seen any other 'undesired' consequences, could someone else test this, and then perhaps get this committed to dev?

marktheshark’s picture

Problem also noticed here.

Will try recommended fix.

marktheshark’s picture

Tried it, did not fix my issue.

Has the maintainer acknowledged this problem?

afreeorange’s picture

Confirming the problem and that Ryotsuke's fix works for me. Thank you!

jureczkyb’s picture

Confirming the problem and also that the above fix works for me.

dejoost’s picture

thx, fix worked for me

zhangtaihao’s picture

I have reviewed this specific area of the code and I can confirm that #1 is basically correct. I've rolled a patch.

There are two other issues in the code, namely the addition of $menu_item->plid and the mlid search to remove the current item. My response to the first would be simply to remove it, since one of p1-p9 would definitely contain plid if it existed at all (as documented in system_schema()). The mlid would always be the last one, so only an array_pop() would suffice. This would work given Drupal core expectations, unless someone had gone and seriously screwed up the {menu_links} table.

zhangtaihao’s picture

To illustrate what I'm talking about in #8, here's another patch. It seems to work the same way as #8 on my setup.

redndahead’s picture

FileSize
1.42 KB

This one is a patch against 7.x-2.x. It's the patch in #8.

lamp5’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)