This are the facts:
1. Menu paths (router item paths to be exact) can have a maximum depth of MENU_MAX_PARTS in Drupal 6.
2. You can specify load arguments in hook_menu that are stored with the router items as 'load_functions' to the database.
For the case that every part is dynamic we can have MENU_MAX_PARTS load arguments for a single item. The most complex case of load arguments is 'load arguments' => array('%map', '%index'). All the parameters are serialized and stored in the 'load_functions' field in table 'menu_router'. The schema definition for that field look like this:
'load_functions' => array(
'description' => t('A serialized array of function names (like node_load) to be called to load an object corresponding to a part of the current path.'),
'type' => 'varchar',
'length' => 255,
'not null' => TRUE,
'default' => ''),
Here is the problem:
The db field of length 255 can NOT store more than a few (3-4) load arguments (of that kind) at a time. If you specify more of them in hook_menu the string resulting from the serialized array of arguments exceeds the 255 chars limit and is truncated.
As the name of the load function is stored in the array as well and the name can be very long (like 'this_is_my_new_module_load'), you can probably reach that limit with e.g. 2 load arguments already.
Solution is simple:
Extend the field to a length that can store the serialized array without truncation. 768 chars should do (better 1024 ?).
P.S. I'm currently away from my coding environment, but I will roll a patch later today.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | system_update_7010.patch | 1.49 KB | catch |
| #11 | 227486.patch | 1.93 KB | gdd |
| #6 | loadarg.patch | 1.6 KB | profix898 |
| #1 | loadarg_field.patch | 1.7 KB | profix898 |
Comments
Comment #1
profix898 commentedHere is a patch to increase the field size to 1024. I also changed the size of to_arg_functions for the same reason.
Comment #2
gpk commentedHmmm "Drupal 6 supports MySQL 4.1 or higher" (http://drupal.org/requirements).
"Values in VARCHAR columns are variable-length strings. The length can be specified as a value from 1 to 255 before MySQL 4.0.2 and 0 to 255 as of MySQL 4.0.2." (http://dev.mysql.com/doc/refman/4.1/en/char.html)
VARCHAR of 256 or more characters only came in with MySQL 5.0.3.
Comment #3
Susurrus commentedWhat about just changing these fields to TEXT?
Comment #4
profix898 commentedThis is still an issue! Should I roll a patch that updates the column to TEXT? Or are there any other options?
Comment #5
gpk commentedThis would be fixed first in HEAD, for which VARCHAR(1024) should be fine. However the minor change to the minimum version of MySQL (from 5.0 to 5.0.3) also needs to be included in the patch. See http://api.drupal.org/api/constant/DRUPAL_MINIMUM_MYSQL/7. You might also need to include a/some test case(s).
For 6.x, TEXT would be the only general solution AFAICS. Whether such a change to the schema would make it into a future 6.x release I don't know - I don't know all the ins and outs, but am sure a core committer could shed some light on this. No harm in rolling a patch for this as well :)
Comment #6
profix898 commentedIf you take a look at other fields designed to take serialized arguments (e.g. 'access_arguments' in the same table or 'options' in table menu_links) their are all using TEXT. So here is a patch that changes 'load_functions' and 'to_arg_functions' fields to TEXT for Drupal HEAD.
Comment #7
profix898 commentedPatch still applies ... and the issue is still present and really annoying.
Comment #8
Crell commentedWe just ran into this issue as well using Views under D6. The more Views arguments you have, the longer load_functions() gets. This bug basically kills any respectfully complex Views or Panels that rely on lots of args. I am therefore bumping this to critical, and it will need to be backported to D6.
Comment #9
gddPatch applies, does what its supposed to, all tests pass against HEAD. I think this is good to go.
Comment #10
dries commentedI've committed this against CVS HEAD. Still needs to go into DRUPAL-6 though. Lowering the version.
Comment #11
gddPatch attached for 6.x-dev. Please credit profix898.
Comment #12
pwolanin commentedmarked http://drupal.org/node/295800 as duplicate
Comment #13
pwolanin commentedyes, needs to go into 6.x
Comment #14
pwolanin commentedpatch still applies cleanly. As a follow-up, probably 'title_arguments' should be converted to text too for consistency.
Comment #15
catchPatch applies, I tested this on my personal 6.x site and it didn't cause any problems.
Comment #16
gábor hojtsyCommitted to 6.x, will go out with the next release, thanks!
Comment #17
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #18
catchThe update should be removed from 7.x - if you upgrade to the latest Drupal 6 you'll get it. If you upgrade from 6.3 to 7.3, you'll also get it due to schema version in system table.
Comment #19
webchickCorrection. In 7.x, 7010 needs to be renamed to 6048 and located with the 6.x updates to match what's in the 6.x branch.
Comment #20
catchComment #21
webchickSmall update to comment (made it "Moved to system_update_6048().") and committed. Thanks a lot!