Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
When upgrading a site from previous version, there may be some odd links created that are remnants of prior versions of Drupal. For example links to /admin/path and /admin/contact may appear after upgrading a site from 4.7.x. There may also be menu items remaining from uninstalled modules such as forum or aggregator.
There is currently no way to delete these broken links from the menu.
Comment | File | Size | Author |
---|---|---|---|
#38 | broken_delete.patch | 3.63 KB | chx |
#37 | broken_delete.patch | 3.59 KB | chx |
#36 | broken_delete.patch | 2.91 KB | chx |
#2 | broken_delete.patch | 3.31 KB | webernet |
#1 | broken_delete.patch | 2.73 KB | chx |
Comments
Comment #1
chx CreditAttribution: chx commentedWe make a good effort in _menu_navigation_links_rebuild (thanks to webernet for catching a bug in here) to keep the updated mark only in items that are likely to be broken and then let the delete go through.
Comment #2
webernet CreditAttribution: webernet commentedAttached patch fixes a missing ml.updated in a query.
Code needs work since for some odd reason, this causes all menu items without a valid router path and those for disabled modules to be deleted automatically.
Comment #3
hass CreditAttribution: hass commentedI think this is more my issue here then the one i opened at http://drupal.org/node/194585... with a wrong idea about what garbaged my menu tree.
Someone able to provide / explain how the value menu_links.hidden is calculated? I cannot find anything about this in D5 - so i have no idea how D5 knows to make "admin/block" (D4.7.x link) hidden and show "admin/build/block" only...
Comment #4
hass CreditAttribution: hass commentedComment #5
chx CreditAttribution: chx commentedThose items are going to appear because... well, the reason I have already explained it twice in the other issue, but just for you I will explain at 2:45am for the third time but no more -- I am going to delete comments that parrot "old links appear", OK? Drupal can not discern between menu links to paths belonging to modules disabled/uninstalled/upgraded a year ago or just disabled right now because of the upgrade. Those items will appear no matter how many times you try to bash my poor head and you won't die from deleting them by hand after the upgrade. The issue here is that those items currently can not be deleted. The patch intends to fix that.
Comment #6
hass CreditAttribution: hass commentedI re-read all comments here and in the other thread and one major thing i'm totally missing is - nobody explained - why there is no way to know what is bogus and what not.
Comment #7
Gábor HojtsyHass, on an installed site, Drupal can check through all links and see what paths are valid and which ones are not. In the update process, users are asked to disable their modules, so it is not possible to do these checks there. Later, once you enabled all the modules you surely need, and you are fine with loosing everything else, which might be dated back later or not, a menu cleanup can be done, as then the system knows again about the valid links. Update is an intermediate stage where we cannot tell.
Hass, you see your D4.7 -> D5 upgrade also left those links in the table, that's why those are still there, so it did not remove them. In D6 however, those are displayed, while in D5, they were not.
Comment #8
JirkaRybka CreditAttribution: JirkaRybka commented#6 3.2. - Where is a data loss here? IMO the lost data is any customizations done. Changed titles, weights, whatever.
Comment #9
chx CreditAttribution: chx commentedAbout how D5 knew bogus menu links from valid links. As Gabor says once you have all your modules enabled, D5 ran hook_menu(TRUE) and then checked customizations for the returned paths. We do not have those paths at update time. Not to mention these paths can change with when the D6 upgrade finishes. The D6 menu update does something no Drupal did before: it iterates the menu table record by record simply because there is nothing better it can do.
We are talking of manually cleaning up a few links. That surely can't be a big problem. A message maybe about on the menu overview screen... possible, I will add later. But currently the problem is they can't be deleted, so fix that first.
Comment #10
hass CreditAttribution: hass commentedI'm not deep enough inside this menu code part so i cannot say much about this...
The only thing i complain about are the 30+ broken (i counted at minimum 25 (~20 core / 5 old modules) on my box in "admin" and "admin/settings") links from D47 - i need to manually remove today. This is far far away from a "few" what could be theoretical max 9 links.
It would be a very good idea to delete all outdated links we know sure. Something like "admin/blocks" that exists in "admin/build/blocks" could be deleted without loosing anything or am i wrong? From usability side i feel stressed and heavily confused if i see all this dead links. I'm sure many people will open many cases in d.o about this and write about an unclean, difficult and problematic update process and we should not ignore this. My personal first idea was - the site install got broken.
My 2 cents.
Comment #11
Gábor Hojtsyhass: As explained, cleaning up is only possible in runtime, so a "menu cleanup" module could be developed which you could run once all the modules you updated are done. (I am not volunteering, just pointing out a possibility).
Comment #12
chx CreditAttribution: chx commentedYes, a module which offers checkboxes for each item where the updated flag is 1 and a delete button is a certain possibility. Still you will need to manually clean up but said cleanup would be easier that way. I am unsure whether I want to develop said module.
Comment #13
JirkaRybka CreditAttribution: JirkaRybka commentedI'm not sure how many users will *find* that module in the current pile of contribs, and then *download* it just for the sake of single post-update cleanup. As for myself, I'll rather go to PhpMyAdmin, if unable to delete from UI.
Comment #14
vjordan CreditAttribution: vjordan commented@chx #9: I think this is something which might be more problematic than first appears - #10 could be a typical reaction to the upgrade process and broken menus are something the vast majority of upgrades will encounter.
In my case I end up with a broken 'Logs' menu and a new 'Reports' menu because I run d5 module "Update Status" which has become d6 core. I suspect many admins are also running this module. Now after the d5-6 upgrade my routine task of looking at the "watchdog" is broken and I'm starting to panic. No problem on a beta site in a sandbox. But an admin upgrading a production site might be reaching for the back-up tapes (and cursing) if it's not easy to understand why this is happening and apply the appropriate fix.
This issue is something that takes time to get one's head around, as evidenced by the repeated rationale here and in http://drupal.org/node/194585. Despite that rationale it could be perceived by many that the menu is being broken by a "faulty upgrade process".
It might need something substantially more visible than "A message...on the menu overview screen". It's the kind of thing we'd want all admins to be watching out for and sorting out as part of the upgrade sequence. Perhaps the information to fix this should be included in UPGRADE.TXT or maybe include a pointer in UPGRADE.txt to a handbook page? I'd say somewhere between items 12 & 14 in UPGRADE.TXT (// $Id: UPGRADE.txt,v 1.10 2007/09/14 20:00:42 goba Exp $) could be a good place.
Comment #15
JirkaRybka CreditAttribution: JirkaRybka commentedOr drupal_set_message() in the upgrade process?
Comment #16
hass CreditAttribution: hass commented@vjordan: Well this should be documented in UPGRADE.txt, but it must be an obtrusive error message in admin section that, do not move away until all menu points have a router path (?)... like the "requirement" hook for modules.
Comment #17
Gábor Hojtsychx: how complicated would it be to provide a "clean up your menu" post update on the menu admin screen, with some help text: "Once you updated all your modules, it is advised to run a cleanup of your menu table, so menu items remained from older versions, but impossible to remove in the standard update process are removed."? This seems to be a problem with which a huge percentage of our user base will be faced.
Comment #18
Gábor HojtsyComment #19
chx CreditAttribution: chx commentedI have no idea what problems webernet had. Review and commit this one and I will write a cleanup tab, OK?
Comment #20
Gábor Hojtsychx: patch looks workable IMHO, and I don't see what might have caused problem for webernet.
hass, webernet: can you please re-test with #2? ("Unfortunately" I always stayed away from using menu module in most projects, so I don't have a useful data set to test with).
Comment #21
vjordan CreditAttribution: vjordan commentedPatch works fine for me.
Comment #22
webernet CreditAttribution: webernet commentedUnfortunately I'm currently unable to test this due to another issue, but if I recall correctly, the problem was that the menu items without valid router entries were deleted automatically while running the 5.x to 6.x updates. This includes entries for views etc.
This is likely due to the changes to _menu_delete_item() which is called from system_update_6021().
Comment #23
Gábor Hojtsywebernet: we got the other issue fixed in the meantime :) Please test again, if you can.
Comment #24
webernet CreditAttribution: webernet commentedMy comment in #22 is accurate - menu items are automatically removed including those for uninstalled/disabled modules (forum, aggregator, search, views), old 4.7 paths (/admin/contact, /admin/path) etc.
Views created menu items don't appear with or without the patch, but I assume that they will be automatically recreated when 6.x views is available and enabled.
Comment #25
Gábor HojtsyThat's not the intention as far as I see.
Comment #33
chx CreditAttribution: chx commentedAfter playing with webernet's table a lot, I am sure the patch does what the patch should. Example
This is after I changed node/6 to something that does not have a router_path for sure. So no, the patch does not delete what not should be deleted. That you do not see it? Well, you will when you switch on a module which gives it a router_path until then it fails access check.
Comment #34
chx CreditAttribution: chx commentedAh well, let it CNR. I hope webernet will second my findings.
Comment #35
chx CreditAttribution: chx commentedOK he pointed me out some missing links.
Comment #36
chx CreditAttribution: chx commentedI forgot about the cleanup at the end of menu and there was the menu_delete_item , of course. I was keeping an eye on customized items but the uncustomized items were failing.
Comment #37
chx CreditAttribution: chx commentedWe can do better though if we move the module checking to the cleanup query.
Comment #38
chx CreditAttribution: chx commentedThat query was there to kill the items put in by the first menu_rebuild which is now gone. Because what is module if it's not system? Menu. But then customized is 1. But then this query wont' affect anything. Gone. This damned update is so hard!
Comment #39
webernet CreditAttribution: webernet commentedTested patch in #38 - It seems to be working fine - too bad there isn't a safe way to automatically delete the invalid links. Being able to do it manually through the UI is better than nothing though.
I'll let someone else test as well before marking RTBC.
Comment #40
Gábor HojtsyI think this patch was pretty heavily investigated and reviewed. Although I don't have a good data set to test myself, I trust webernet's and chx's judgement on this that it is ready.
It would be great to get that cleanup tab in a follow up, so the deletion will not be a manual process.
Comment #42
spiffyd CreditAttribution: spiffyd commentedThis patch does not work with Drupal 6.4. Can anyone confirm this?