I've just made a clean installation of drupal using MySQL 5, PHP 5 and Apache 2.
Drupal 7.x-dev (2008-oct.-30)
Drupal Administration Menu 7.x-1.x-dev (2008-oct.-29)
htmLawed 7.x-2.x-dev (2008-oct.-07)
Printer, e-mail and PDF versions 7.x-1.x-dev (2008-oct.-28)
Moleskine 7.x-1.x-dev (2008-août-23).
I've added a single main menu item: home, since the I can't see the menu list nor delete or edit any menu. I always get the following error:
Recoverable fatal error: Argument 3 passed to l() must be an array, string given, called in /home2/chicospe/public_html/7/includes/menu.inc on line 1165 and defined in l() (line 1571 of /home2/chicospe/public_html/7/includes/common.inc).
Comments
Comment #1
David_Rothstein CreditAttribution: David_Rothstein commentedCan you reproduce this without those extra modules enabled (i.e., just with Drupal 7.x-dev installed and nothing else)?
The 7.x-dev versions of the contributed modules you listed are almost certainly not expected to work at this stage (given that the core API for Drupal 7 is not even frozen), so I would guess there's a good chance it's being caused by one of them.
Thanks!
Comment #2
nikemen CreditAttribution: nikemen commentedI've just done so, but nothing changes (emptying caches &c., of course).
Some additional info:
1. Till now I used a PostgreSQL database and everything worked fine.
2. I've imported strings of a french translation for drupal 6.
Thanks!
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis is probably a rogue module doing something silly with a menu_hook() implementation. I'm requalifying this as a support request, please try reinstalling a clean Drupal 7, and confirm that it works correctly.
Comment #4
nikemen CreditAttribution: nikemen commentedNow performed a fresh install without any module. Nor did I include any localization. I'm getting the same error.
Thanks for your help.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedlocalized_options is passed to theme_menu_item_link... serialized.
WTF?
Comment #6
marcingy CreditAttribution: marcingy commentedThe issue by the looks if it is being caused when the menu router table is being built.
admin/build/menu-customize/navigation - options = a:0:{}
admin/build/menu-customize/main-menu - options = s:6:"a:0:{}";
admin/build/menu-customize/secondary-menu - options = s:13:"s:6:"a:0:{}";";
So the data is being unserialised it just that the info being pulled back is in the wrong format to start with
Comment #7
marcingy CreditAttribution: marcingy commentedThe issue is being caused because &items is passed in by ref, and so on the second time through the serialised value is appended in addition to the array.
This patch resets the options index and removes the serialised data before calling menu_link_save.
To test this patch do a clean install and reanable the menu module.
Comment #8
swentel CreditAttribution: swentel commentedI can confirm this issue. Updated the patch so you can apply it from drupal root (don't credit me).
I think pwolanin should take a look at this before setting this to RTBC.
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis was caused by a change in #302638: No-op and slow queries during menu rebuild. There is an issue in menu_enable(), which doesn't realize that $item can be modified, but I think it would be better to fix the cause there.
Comment #10
Damien Tournoud CreditAttribution: Damien Tournoud commentedOk, this will fix the two bugs (menu_enable being silly and menu_link_save misbehaving).
Comment #11
webchickCommitted, thanks.
Comment #12
hswong3i CreditAttribution: hswong3i commentedThe patch seems break menu's simpletest.
I tested with latest CVS HEAD but coming with "275 passes, 108 fails, and 106 exceptions" for menu test.
May someone double check if this is truth, or just a fault alarm?
Comment #13
webchick...and this is *exactly* why I should not commit patches while I'm on the phone. :P
Rolled back. Thanks.
Comment #14
webchickMarking this as a blocking patch for UNSTABLE-3, which I'd love to get rolled sometime in the next 24-48 hours so that we can start getting in some of these crazy cool patches we have waiting in the wings. :)
Comment #15
swentel CreditAttribution: swentel commentedProblem was in these two lines
Attached patch fixes this by a simple if (!empty) check. All menu tests pass with this.
Comment #16
Damien Tournoud CreditAttribution: Damien Tournoud commentedYep, my bad. swentel patch in #15 is working as intended.
Comment #17
Dries CreditAttribution: Dries commentedWe should have a test for this, IMO.
Crazy idea: maybe we should have a test that "walks" all the registered pages in the menu system and that scans for exceptions, warnings, etc.
Comment #18
pwolanin CreditAttribution: pwolanin commented@Dries - I proposed such a test a while back, but haven't found the time to implement it...
Comment #19
Dries CreditAttribution: Dries commentedThat test has probably become extremely trivial now we have some more advanced error logging in SimpleTest. :-)
Comment #20
hswong3i CreditAttribution: hswong3i commentedJust give a hand, revamp with DBTNG syntax, so we need not to redo that later.
Patch via CVS HEAD, tested with MySQL + menu simpletest. All pass without error message.
P.S. I also integrate progress of this patch into http://drupal.org/node/320510#comment-1104257, so try not to redo once again. Is this possible?
Comment #21
catch#20 looks fine, passed simpletest and a manual visit to admin/build/menu
menu.test contains the line
$this->drupalGet('admin/build/menu');
- so we do have a test for this, but it didn't catch the regression, which is very odd.I'm 100% in favour of the click-around test, could really help to keep us E^ALL compliant.
Comment #22
Damien Tournoud CreditAttribution: Damien Tournoud commented@hswong3i: one step at a time, please open a new issue for the conversion of those queries to the new API. This isssue is only for fixing that particular bug.
I studied why the bug wasn't showing up in simpletest. It turns out that the issue is with menu links, not menu routers, so it only shows up when the menu is displayed on the page. So here is a reroll of patch in #15, along with a full test case for that bug (it's a on line patch to menu.test :p).
Comment #23
catchThe tests fails spectacularly if you just make that one change, nice find! Passes with the full patch, RTBC.
Comment #24
catchComment #25
pwolanin CreditAttribution: pwolanin commentedCheck code and tested - I see the bug and this fixes it.
A minor point: db_fetch_array() returns FALSE if the link is not found, so we don't actually need the
!empty()
and we don't use it elsewhere in the function when checking $existing_item. Could be simplified like the attached (or just by omitting the!empty
).Comment #26
hswong3i CreditAttribution: hswong3i commented+1 for #25, looks elegant. Also test with:
Comment #27
webchickExcellent. :)
Committed once more (and verified the dang tests this time :D). Thanks, folks. :)
Comment #28
pwolanin CreditAttribution: pwolanin commentedComment #29
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis is out of the D7 visibility now... so it simply got ignored... typical.
Comment #30
catchSee #221510: Update contributors block for 8.x (and minor changes) a much-ignored issue to have a D6 criticals link in the contributors block.
Comment #31
pwolanin CreditAttribution: pwolanin commentedhere's a manual backport - the D7 patch didn't fully apply, so needs testing.
Comment #32
Damien Tournoud CreditAttribution: Damien Tournoud commentedLooks great. Given the amount of testing of this in D7, I'm very confident for D6.
For those wondering, the hunk of menu_link_save:
Is not really needed for D6, now, because we don't make good use of $existing_item, but it will be when #302638: No-op and slow queries during menu rebuild will be considered again for D6.
Comment #33
Gábor HojtsyThen please keep this issue focused on the $link by reference stuff, and move the remaining code to the referenced issue. It does look absolutely unrelated.
Comment #34
David_Rothstein CreditAttribution: David_Rothstein commentedHere it is with the unneeded code removed. I did not test this patch at all, but from looking at the code it makes abundant sense.
Comment #35
Apollo610 CreditAttribution: Apollo610 commentedHi all, I've applied the D6 patch to my development site and everything seems to be a-ok.
Navigating around is fine (including to admin/modules/build) and adding/removing menus doesn't cause any problems. All log files are squeaky clean.
Let me know if there's anything else you need me to test out with the D6 patch.
Comment #36
marcingy CreditAttribution: marcingy commentedComment #37
robertDouglass CreditAttribution: robertDouglass commentedsubscribe.
Comment #38
Gábor HojtsyThanks, committed to Drupal 6.
Comment #40
cesar.brod@gmail.com CreditAttribution: cesar.brod@gmail.com commentedI am having the exact same problem. Just upgraded to Drupal 7.9 and noticed the patches here are already applied. If I try to edit a node in the main menu I get the following error:
Recoverable fatal error: Argument 3 passed to l() must be an array, boolean given, called in /var/mundolinux/includes/menu.inc on line 2538 and defined in l() (line 2307 of /var/mundolinux/includes/common.inc).
The data was imported from a D6 install and this seems to be the only problem I have. The node in the main menu causing me the problem is not important so, if I could, I would just delete it. However, I get the same error all the time I try to edit it or delete it.
All help is welcome!
Comment #41
cesar.brod@gmail.com CreditAttribution: cesar.brod@gmail.com commentedI was able to fix it by mannually removing all entries relative to main_menu on the table menu_links and rebuilding my Main Menu.