Menu links never actually link to the full path of a MENU_DEFAULT_LOCAL_TASK, only its parent.

/admin/structure/views           <-- Parent
/admin/structure/views/basic     <-- Default local task
/admin/structure/views/advanced  <-- Local task

As a part of #1995620: [policy, no patch] Document how to handle routes for MENU_DEFAULT_LOCAL_TASK, we're considering making the actual default local task path 404.

I'm not 100% sure if #914382: Contextual links incompatible with render cache changed this, but it certainly makes it harder to track down.
I believe that the db_select() in menu_contextual_links() is actually the issue.

CommentFileSizeAuthor
#5 menu-contextual-2006636-5.patch3.71 KBtim.plunkett
#4 menu-2006636-4.patch677 bytestim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

The query in question is this:

db_select('menu_router', 'm')
  ->fields('m')
  ->condition('tab_parent', $router_item['tab_root'])
  ->condition('context', MENU_CONTEXT_NONE, '<>')
  ->condition('context', MENU_CONTEXT_PAGE, '<>')
  ->orderBy('weight')
  ->orderBy('title')
  ->execute()
  ->fetchAllAssoc('path', PDO::FETCH_ASSOC);
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

What about something like

$or = db_or();
$and_one = db_and()->condition('m.tab_parent', $router_item['tab_root'])
  ->condition('m.context', MENU_CONTEXT_NONE, '<>')
  ->condition('m.context', MENU_CONTEXT_PAGE, '<>');
$and_two = db_and()
  ->condition('m_parent.context', MENU_CONTEXT_NONE, '<>')
  ->condition('m_parent.context', MENU_CONTEXT_PAGE, '<>');

$or->condition($and_one);
$or->condition($and_two);


$select = db_select('menu_router', 'm')
  ->fields('m');

$select->innerJoin('menu_router', 'm_parent', 'm_parent.path = m.tab_root');
$select->condition($or);

$select->orderBy('weight')
  ->orderBy('title')
  ->execute()
  ->fetchAllAssoc('path', PDO::FETCH_ASSOC);
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

#914382: Contextual links incompatible with render cache didn't touch menu_contextual_links() :)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Active » Needs review
FileSize
menu-2006636-4.patch677 bytes

I spent a good deal of time with this, and I'm not sure if it's possible with a single query.

What we want are all local tasks, and the parent task only when it has a default local task, and only when only none of the local tasks have MENU_CONTEXT_NONE or MENU_CONTEXT_PAGE.
But all parents have MENU_CONTEXT_NONE, and it's hard to know if they have a child MENU_DEFAULT_LOCAL_TASK.

So, this is easier.

However, this is going to break a lot of tests, but all of the ones that break are violating #1995620: [policy, no patch] Document how to handle routes for MENU_DEFAULT_LOCAL_TASK anyway, so we can just fix them.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
menu-contextual-2006636-5.patch3.71 KB

Actually, this shouldn't be a big problem. We don't need to change all the asserts, just the contextual link ones.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community

The default local task path is never used AFAIK so if it is used then it's a bug. I wonder whether this is backportable too.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed ace3459 and pushed to 8.x. Thanks!

Let the backport discussion begin :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Reviewed & tested by the community

Nice!

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

I'm to slow.

Crell’s picture

Crell CreditAttribution: Crell commented
Issue tags: -WSCCI

No WSCCI tag needed for D7.

bhide.nishad’s picture

bhide.nishad CreditAttribution: bhide.nishad at Clarion Technologies commented
Assigned: Unassigned » bhide.nishad
Issue summary: View changes

  • alexpott committed ace3459 on 8.3.x 
    Issue #2006636 by tim.plunkett: Fixed menu_contextual_links() will...

  • alexpott committed ace3459 on 8.3.x 
    Issue #2006636 by tim.plunkett: Fixed menu_contextual_links() will...
bhide.nishad’s picture

bhide.nishad CreditAttribution: bhide.nishad at Clarion Technologies commented
Assigned: bhide.nishad » Unassigned

  • alexpott committed ace3459 on 8.4.x 
    Issue #2006636 by tim.plunkett: Fixed menu_contextual_links() will...

  • alexpott committed ace3459 on 8.4.x 
    Issue #2006636 by tim.plunkett: Fixed menu_contextual_links() will...