Recoverable fatal error: Argument 3 passed to l() must be an array
nikemen - October 30, 2008 - 13:33
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | menu system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
| Issue tags: | drupal.org upgrade, Performance |
Description
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).
#1
Can 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!
#2
I'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!
#3
This 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.
#4
Now performed a fresh install without any module. Nor did I include any localization. I'm getting the same error.
Thanks for your help.
#5
localized_options is passed to theme_menu_item_link... serialized.
WTF?
#6
The 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
#7
The 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.
#8
I 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.
#9
This 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.
#10
Ok, this will fix the two bugs (menu_enable being silly and menu_link_save misbehaving).
#11
Committed, thanks.
#12
The 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?
#13
...and this is *exactly* why I should not commit patches while I'm on the phone. :P
Rolled back. Thanks.
#14
Marking 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. :)
#15
Problem was in these two lines
<?php$existing_item = db_fetch_array(db_query("SELECT * FROM {menu_links} WHERE mlid = %d", $item['mlid']));
$existing_item['options'] = unserialize($existing_item['options']);
?>
Attached patch fixes this by a simple if (!empty) check. All menu tests pass with this.
#16
Yep, my bad. swentel patch in #15 is working as intended.
#17
We 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.
#18
@Dries - I proposed such a test a while back, but haven't found the time to implement it...
#19
That test has probably become extremely trivial now we have some more advanced error logging in SimpleTest. :-)
#20
Just 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?
#21
#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.
#22
@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).
#23
The tests fails spectacularly if you just make that one change, nice find! Passes with the full patch, RTBC.
#24
#25
Check 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).#26
+1 for #25, looks elegant. Also test with:
#27
Excellent. :)
Committed once more (and verified the dang tests this time :D). Thanks, folks. :)
#28
#29
This is out of the D7 visibility now... so it simply got ignored... typical.
#30
See #221510: Show Drupal 6 critical issues in contributors block a much-ignored issue to have a D6 criticals link in the contributors block.
#31
here's a manual backport - the D7 patch didn't fully apply, so needs testing.
#32
Looks 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:
<?php- $existing_item = db_fetch_array(db_query("SELECT * FROM {menu_links} WHERE mlid = %d", $item['mlid']));
+ if ($existing_item = db_fetch_array(db_query("SELECT * FROM {menu_links} WHERE mlid = %d", $item['mlid']))) {
+ $existing_item['options'] = unserialize($existing_item['options']);
+ }
}
?>
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.
#33
Then 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.
#34
Here 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.
#35
Hi 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.
#36
#37
subscribe.
#38
Thanks, committed to Drupal 6.
#39
Automatically closed -- issue fixed for 2 weeks with no activity.