We recently added back to _menu_navigation_links_rebuild() code to delete menu link where their router path is missing, as long as they are not customized:

http://api.drupal.org/api/function/_menu_navigation_links_rebuild/6

However, this still causes too much deletion. Consider this structure for links derived from a modules paths where * indicated customized:

admin
    \
    L1
      \
      L2*

If the module is disabled, L1 will be deleted and L2 re-parented. When the module is re-enabled the links will look like:

  admin
  /   \
 L2*   L1

The only way to prevent this (if we do indeed want to delete links) is that we basically need to load information about the links that are excluded from deletion due to having customized = 1, and then exclude from delete any link on the path to root of those links.

Actually - when I tried, this the entire menu had something bad happened - so there may be a more critical error here.

tried this - moved /admin to the primary links

moved admin/content/taxonomy to under admin/content/comment

disable both comment and taxonomy and then re-enable.

the link is not even properly re-parented. I show admin/content/taxonomy with a plid that refers to a deleted link, and the menu overview is broken.

CommentFileSizeAuthor
#5 orphan-220814-5.patch1.17 KBpwolanin

Comments

cburschka’s picture

cburschka
they
commented

I get the point, but the scenario you're describing to reproduce this sounds kind of far-fetched... are you sure this error is common enough to block the release?

we basically need to load information about the links that are excluded from deletion due to having customized = 1, and then exclude from delete any link on the path to root of those links.

That would be undesirable behavior - if the links are broken, they should be removed. If necessary, I would rather store the menu parent of the customized menu item, and re-parent it under this same item when the module is re-enabled.

pwolanin’s picture

pwolanin commented

The first part of the report is non-critical (the diagrams)

The fact that we get orphaned links is what makes this critical.

nescius’s picture

nescius commented
Status: Active » Closed (duplicate)

sorry, my mistake

cburschka’s picture

cburschka
they
commented
Status: Closed (duplicate) » Active

Utterly unrelated. Typo?

And here I thought you were going for a menu router pun with the "route" =P.

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new orphan-220814-5.patch1.17 KB

attached patch *may* alleviate the critical part of the issue, but if so it points to some other problem in the menu system.

Note that this patch should be totally safe, but could add one extra query per link deleted.

pwolanin’s picture

pwolanin commented
Priority: Critical » Normal
Status: Needs review » Active

moving the cirticial bug part to: http://drupal.org/node/220953

catch’s picture

catch
he/him
Primary language English
commented
Priority: Normal » Critical
Status: Active » Needs review

pwolanin: not related to this is it? http://drupal.org/node/217803

catch’s picture

catch
he/him
Primary language English
commented
Priority: Critical » Normal

whoops.

pwolanin’s picture

pwolanin commented

@catch no, not related. That issue was for when a module was uninstalled rather than simply disabled.

pwolanin’s picture

pwolanin commented
Status: Needs review » Active

this is related: http://drupal.org/node/215127

Since that patch we sometimes have children of links from disabled modules floating up, and sometimes not, depending on whether the parent has been customized

dpearcefl’s picture

dpearcefl commented

Is this still an issue in current D6?

Status: Active » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.