'load arguments' of parent path are not inherited

In this case, block_load() requires two arguments: $module and $delta. These are expected to be passed to the menu argument loader, and hence, block_load() can't be invoked with %block only.

For the path admin/structure/block/manage/%block/% - which defines load arguments - the load function arguments are properly stored in the database.

For the path admin/structure/block/manage/%block/%/configure - which does not define load arguments on its own - only %block is stored as argument in load_functions.

Since admin/structure/block/manage/%block/%/configure is a MENU_DEFAULT_LOCAL_TASK, I expected that

  $router_item = menu_get_item('admin/structure/block/manage/foor/bar/configure');

would take over the defined load arguments from the parent, because I equally don't need to specify any other arguments defined by the parent to make the MENU_DEFAULT_LOCAL_TASK work as it should:

  $items['admin/structure/block/manage/%block/%'] = array(
    'load arguments' => array(5),
  );
  $items['admin/structure/block/manage/%block/%/configure'] = array(
    'title' => 'Configure',
    'type' => MENU_DEFAULT_LOCAL_TASK,
  );

I expected this behavior, because I can't see how any path of

admin/structure/block/manage/%block/%/configure

can work at all when the "parent path" defines load arguments to allow the menu argument loader %block, i.e. block_load(), to load the right thing, as in:

admin/structure/block/manage/%block/%

(yes, you ranted there's no concept of parents, but you didn't explain why, so I'm not sure how I can grok that.)

I would expect this behavior as long as an item does not define load arguments on its own:

  $items['admin/structure/block/manage/%block/%'] = array(
    'load arguments' => array(5),
  );
  $items['admin/structure/block/manage/%block/%/send-to-mars/%'] = array(
    'load arguments' => array(5, 7),
  );

With the new usage of local tasks/contextual links, I'm pretty sure that I'll not be the only one who runs into this bug + has no clue at all why it's not working, so I recommend doing #3.

It took 3 experienced Drupal developers to find out that "load arguments" are not inherited at all (which sounds quite simple, but it's totally not what you expect).

Thanks! This seems to pass the bot and looks correct. However, we also want 1) a bit more docs and 2) tests. Not sure whether I'll get to do that.

oopsie

Broken tags.

Reworked those tests. Also had to rework that foreach loop in menu_router_build(), as it did not properly work and led to bogus load functions data.

Lastly, during debugging I recognized that we're needlessly unserializing a lot of menu item load functions, and serialize()/unserialize() is very slow, so this patch additionally fixes that.

Just to be precise: adding Performance tag. unserialize() is known to be slow. And the current usage is just useless.

Although my local box is totally not suitable for doing benchmarks, I've mass-applied the following range of current performance patches and am getting a nice boost:

#910190: Entire schema cache needlessly loaded on all pages
#910156: Remove 'description' keys from schema cache
#606966: 'load arguments' of parent path are not inherited
#910124: Clean up menu_contextual_links()

BEFORE patches:
Executed 36 queries in 25.18 ms.
Page execution time was 481.48 ms.
Memory used at: devel_boot()=2.5 MB, devel_shutdown()=16.23 MB, PHP peak=16.75 MB.

AFTER patches:
Executed 35 queries in 15.99 ms.
Page execution time was 471.18 ms.
Memory used at: devel_boot()=1.95 MB, devel_shutdown()=15.85 MB, PHP peak=16.5 MB.

Test page: admin/config/development/devel; Garland; enabled Navigation menu block containing 1 taxonomy term link and 1 node link.

#17: drupal.menu-load-arguments-inherit.17.patch queued for re-testing.

#17: drupal.menu-load-arguments-inherit.17.patch queued for re-testing.

Although badly needed, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).

#17: drupal.menu-load-arguments-inherit.17.patch queued for re-testing.