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).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein’s picture

Status: Active » Postponed (maintainer needs more info)

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!

nikemen’s picture

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!

Damien Tournoud’s picture

Category: bug » support

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.

nikemen’s picture

Category: support » bug
Status: Postponed (maintainer needs more info) » Active

Now performed a fresh install without any module. Nor did I include any localization. I'm getting the same error.
Thanks for your help.

Damien Tournoud’s picture

localized_options is passed to theme_menu_item_link... serialized.

WTF?

marcingy’s picture

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

marcingy’s picture

Status: Active » Needs review
FileSize
676 bytes

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.

swentel’s picture

FileSize
820 bytes

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.

Damien Tournoud’s picture

Status: Needs review » Needs work

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.

Damien Tournoud’s picture

Status: Needs work » Needs review
FileSize
2.75 KB

Ok, this will fix the two bugs (menu_enable being silly and menu_link_save misbehaving).

webchick’s picture

Status: Needs review » Fixed

Committed, thanks.

hswong3i’s picture

Status: Fixed » Active

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?

webchick’s picture

Status: Active » Needs work

...and this is *exactly* why I should not commit patches while I'm on the phone. :P

Rolled back. Thanks.

webchick’s picture

Title: Recoverable fatal error: Argument 3 passed to l() must be an array » UNSTABLE-3 blocker: Recoverable fatal error: Argument 3 passed to l() must be an array

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. :)

swentel’s picture

Status: Needs work » Needs review
FileSize
3.09 KB

Problem was in these two lines

     $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.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

Yep, my bad. swentel patch in #15 is working as intended.

Dries’s picture

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.

pwolanin’s picture

Title: UNSTABLE-3 blocker: Recoverable fatal error: Argument 3 passed to l() must be an array » Recoverable fatal error: Argument 3 passed to l() must be an array
Status: Reviewed & tested by the community » Needs review

@Dries - I proposed such a test a while back, but haven't found the time to implement it...

Dries’s picture

That test has probably become extremely trivial now we have some more advanced error logging in SimpleTest. :-)

hswong3i’s picture

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?

catch’s picture

#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.

Damien Tournoud’s picture

@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).

catch’s picture

Status: Needs review » Reviewed & tested by the community

The tests fails spectacularly if you just make that one change, nice find! Passes with the full patch, RTBC.

catch’s picture

Title: Recoverable fatal error: Argument 3 passed to l() must be an array » UNSTABLE-3 blocker: Recoverable fatal error: Argument 3 passed to l() must be an array
pwolanin’s picture

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).

hswong3i’s picture

+1 for #25, looks elegant. Also test with:

  1. Update test only: error triggered with "259 passes, 151 fails, and 37 exceptions".
  2. Complete patch: without error.
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Excellent. :)

Committed once more (and verified the dang tests this time :D). Thanks, folks. :)

pwolanin’s picture

Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)
Damien Tournoud’s picture

Title: UNSTABLE-3 blocker: Recoverable fatal error: Argument 3 passed to l() must be an array » Recoverable fatal error: Argument 3 passed to l() must be an array

This is out of the D7 visibility now... so it simply got ignored... typical.

catch’s picture

See #221510: Update contributors block for 8.x (and minor changes) a much-ignored issue to have a D6 criticals link in the contributors block.

pwolanin’s picture

Status: Patch (to be ported) » Needs review
FileSize
2.01 KB

here's a manual backport - the D7 patch didn't fully apply, so needs testing.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

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:

-    $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.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

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.

David_Rothstein’s picture

Status: Needs work » Needs review
FileSize
1.25 KB

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.

Apollo610’s picture

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.

marcingy’s picture

Status: Needs review » Reviewed & tested by the community
robertDouglass’s picture

subscribe.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed to Drupal 6.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

cesar.brod@gmail.com’s picture

I 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!

cesar.brod@gmail.com’s picture

I was able to fix it by mannually removing all entries relative to main_menu on the table menu_links and rebuilding my Main Menu.