Update Dec 2017

This issue appears to have gone away for most users around 2015, however if you are noticing any PHP warnings, please try updating your Drupal version to the latest version and to use the latest releases for all of the content moderation modules first before reopening this issue.

The latest patch is rtbtc by itself, but the patch may cause additional issues if you use a module that extends the menu structure that the Diff module defines.

Thanks

Original report

On almost every page call, an error like this is triggered:
Undefined offset: 4 in /var/www/drupal-hfopi/includes/menu.inc on line 576.
This is caused by using _menu_translate() in unintended context, namely filling in placeholders from paths different from the $router_item. _menu_translate() doesn't check if both path chains are of same size, so on any node/%/edit page, among others the placeholders node/%/revisions/view/%/% are attempted to be replaced from the - too short - current path.

DELETE FROM `watchdog` WHERE `type` LIKE 'php' AND `variables` REGEXP 'menu.*576' result: 8264 row(s) deleted..

How is it that with more than 25kilo installations of this module, this bug hasn't been sqashed yet? THESE types of bugs were my initial motivation to propose a different development concept for D7 core on the core mailing list... Ah well whatever x|

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

I've never heard of this bug before.

Additionally, that function is a straight clone of menu_local_tasks so it needs to be explained to me why this function needs to change when the core one works?

eMPee584’s picture

Project: Chaos Tool Suite (ctools) » Drupal core
Version: 6.x-1.x-dev » 6.x-dev
Component: Code » menu system

OMG admin_links has the very same prob (abusing internal function), wtf... In this case, maybe the assignment in core's menu.inc should be made conditional, as in the attached patch?

eMPee584’s picture

That alas is a good question i have zero answers prepared for. Huh? More wtf. 8D

merlinofchaos’s picture

Can you start by helping us reproduce this bug? It's not something I've ever seen in my development, so it may be related to some particular setup feature you have that triggers it?

eMPee584’s picture

Status: Needs review » Active
FileSize
923 bytes

aight merlin, please apply the attached patch to your core menu (assuming you have enabled the devel module because of call to dpm()) and go to a node edit page. If that doesn't trigger the described bug, something is truely foobared on my installation...

merlinofchaos’s picture

Ok, I did this; I added the code to my core menu.inc (it modified _menu_translate()) and I visited node/1/edit -- the new code produces a notice but it does not produce the dpm()s at all.

Running a menu_rebuild() does not produce the dpms either.

eMPee584’s picture

Mhh alright that debug code was a bit to specific maybe.. however i am getting the problem only for very few paths aswell. One is node/%/revisions/view/%/% (provided by the diff module) and the other is.. well i swear it was admin/build/modules/list/confirm or so but i can't reproduce it?
gross, i shouldn't have touched drupal the last two days, what a waste of time. Now it's deep at night and still haven't prepared tomorrow's nuclear fusion lecture *sigh*

jennifer.chang’s picture

Project: Drupal core » Diff
Version: 6.x-dev » 6.x-2.x-dev
Component: menu system » Code

I confirm the same error on a site with diff.module. Is it something that diff.module can fix itself?

grendzy’s picture

Project: Diff » Drupal core
Version: 6.x-2.x-dev » 6.x-dev
Component: Code » menu system

This seems more like a core bug to me. I can't find anything wrong with the way the diff module uses the menu system.

eMPee584’s picture

Status: Active » Needs review
FileSize
461 bytes

Ok the attached core patch fixes the undefined index problem when viewing diff revisions..

keinstein’s picture

FileSize
634 bytes

A different solution for the same problem: I reproduced, what PHP (5.2) does in my patch.

It sets $link_map[$i] to NULL if the right hand side is not defined.

I don't know which idea is better, setting it to '%' or to NULL.

zzolo’s picture

Title: undefined offset 4 in includes/menu.inc on line 576 » undefined offset in includes/menu.inc on line 576

Changing title. The offset number is dependent on how the menu item is defined.

wik’s picture

subscribing

eMPee584’s picture

Hey this is a trivial bug, who dares committing the simple fix? ^^

eMPee584’s picture

Version: 6.x-dev » 7.x-dev

Patch from #10 also applies to D7's menu.inc. Patch #11 imho does the wrong thing, changing a "%" placeholder string that cannot be expanded here with NULL.

This is so simple it doesn't need a test, ain't it?

sun’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

1) Only attachments ending in .patch are tested.

2) This entire issue sounds a bit odd. I did not find clear code or instructions to replicate the bug. If this notice would appear in stock core, then many more people would have followed up here. I suspect a wrong usage of core APIs in a contributed module instead. Without knowing what people are doing wrong, we cannot tell whether this patch is the right fix.

eMPee584’s picture

Status: Needs work » Closed (won't fix)

Well, obviously there are circumstances, apparently involving the diff module, when the _menu_translate() function is invoked on a URL trail it can not find enough placeholders for.. The exact circumstances are unknown to me at the moment, and i have zero ambitions to invest time in tracking them down, neither to implement a test. What on earth is debatable about path #10? It simply cannot break anything (!) - and it will make _menu_translate() not choke on obscure URL trails. I have posted an example scenario for this in #7, and it has been confirmed by someone else. If that is not enough, whatever. This debating (for months and often YEARS) about evidently simple bug fixes (respectively shielding against core API fuzzing/misuse) is a huge PITA.. fsck it.

prodosh’s picture

Just experienced this bug myself. Interestingly we didn't have it in the staging environment which ran PHP5.2.
In the production environment, we are running PHP5.3 and this bug seems to occur. Maybe this information will help somebody figure out what's going on.
In the meantime, is using the patch posted here advisable? If not, what other options are there?

yngens’s picture

Ditto. We have migrated one of Drupal sites to PHP Version 5.3.3 and encountered this error. Disabling Diff module helps, but we would like to use it too after the issue is solved.

EvanDonovan’s picture

Status: Closed (won't fix) » Needs work

Looks like people are still reporting this. Can someone repost the patch with an extension ending in .patch so testbot can try it?

yngens’s picture

This has definitely something with PHP version. I believe I have to revert to earlier version, because on every site I transfer to this PHP5.3.3 I start to get lots of errors like, for example:

notice: Undefined variable: output in /home/myusername/public_html/sites/mysite.com/themes/city/template.php on line 19.
notice: Undefined variable: node in /home/myusername/public_html/sites/mysite.com/themes/city/page.tpl.php on line 42.
notice: Trying to get property of non-object in /home/myusername/public_html/sites/mysite.com/themes/city/page.tpl.php on line 42.
notice: Undefined index: #value in /home/myusername/public_html/sites/mysite.com/themes/city/template.php on line 30.

ChrisBryant’s picture

I also just ran into this when migrating a 6.x site to a server running PHP 5.3. Disabling diff did remove the notices as well.

sun’s picture

Looking into the 7.x-2.x and 6.x-2.x branches of Diff (e.g., http://drupalcode.org/viewvc/drupal/contributions/modules/diff/diff.modu... - relevant parts are identical), there's some very complex re-shuffling of menu items going on. That looks highly suspicious to me. It is perfectly possible that Drupal core merely reveals a non-obvious mistake in that code.

chriscohen’s picture

We had this issue on:
* wampserver 2 on Windows 7
* Drupal 6.19 core
* PHP 5.2.11
* error_reporting = E_ALL

We switched the error_reporting over to E_ALL & ~E_NOTICE to get rid of it. I'm not sure what was causing it in the first place though, so I'm not discounting the possibility of a core bug, or asserting the presence of one, either.

febbraro’s picture

Actually get this same error using the nodequeue module for D7 on PHP 5.2.13

To reproduce:
* Install/ Enable Nodequeue
* Add a new nodequeue
* Go to admin/structure/nodequeue/1/view
* Refresh (to get the Notice posted into $messages)

The Notice I'm seeing is:
Notice: Undefined offset: 5 in _menu_translate() (line 739 .../includes/menu.inc)

The only difference is this is generating a notice in D7 and not in D6, same server, same PHP version, same operation.

  for ($i = 0; $i < $router_item['number_parts']; $i++) {
    if ($link_map[$i] == '%') {
      $link_map[$i] = $path_map[$i];  <=== CAUSES A NOTICE IN D7
    }
    [some extra stuff in D7 version]
  }
febbraro’s picture

Status: Needs work » Needs review
FileSize
836 bytes

Not that I know the real reason why it behaves differently, but here is a re-rolled patch that checks if the value exists before using it, otherwise setting it to NULL, thus removing the NOTICE.

febbraro’s picture

Ugh. Now has a .patch extension.

sun’s picture

Hm. This only adds evidence that this bug actually does not exist in Drupal core, is merely exposed by Drupal core, but caused by bogus code in contributed modules.

Thus, by hiding the error message, the bug is not actually fixed, you merely do not see it anymore.

Instead of doing that isset() tweak, it would be helpful to put the following line into that loop:

if (!isset($path_map[$i]) && !isset($debug)) ($debug = 1; function_exists('ddebug_backtrace') ? ddebug_backtrace() : debug_backtrace());

Based on that output, try to figure out from where and how the bogus menu router information is coming from. In the case of Diff module, that information indeed comes from the menu system, but originally derived from the definition in diff_menu(), which happens to contain a couple of internal menu system properties, which normally shouldn't be set manually. In the case of NodeQueue module, I wasn't able to find similar stuff, so something else must be going on.

Ultimately, we need to understand that the menu system itself sets $router_item['number_parts'] in order to process dynamic path argument placeholders in this very function. If number_parts is suddenly no longer what the menu system set on its own, then something is really blown up. It's like parking your car at the next corner, and you know it's there, but suddenly it's not. We don't want to hide that error.

febbraro’s picture

Well here is what is happening in Node Queue's scenario.

The menu is defined as such

  $items['admin/structure/nodequeue/%nodequeue'] = array(
    'access arguments' => array(3),
    'access callback' => 'nodequeue_queue_access',
    'page callback' => 'nodequeue_admin_view',
    'page arguments' => array(3),
    'type' => MENU_CALLBACK
  );
  $items['admin/structure/nodequeue/%nodequeue/view/%subqueue'] = array(
    'title' => 'View',
    'access arguments' => array(3, 5),
    'access callback' => 'nodequeue_queue_access',
    'page callback' => 'nodequeue_admin_view',
    'page arguments' => array(3, 5),
    'weight' => -10,
    'type' => MENU_DEFAULT_LOCAL_TASK
  );
  • When clicking on a View link for a particular nodequeue, the path does not contain a subqueue ID and is admin/structure/nodequeue/1/view
  • The router_item in _menu_translate is for admin/structure/nodequeue/%/view/% with number_parts = 6
  • The $map var comes in with a 5 value array (admin, structure, nodequeue, 1, view)
  • $map gets copied to $path_map, again a 5 valued array
  • After _menu_load_objects($router_item, $map) $map is now 6 valued, (admin, structure, nodequeue, stdClass[nodequeue], view, NULL)
  • The router_item['path'] is then exploded and looped, when it reaches index 5 (the 6th element) there is a % placeholder for subqueue, but nothing in $path_map (based on what $map was upon entry to the function)
  • The notice is generated and NULL is correctly put in $link_map. NOTE: This is the same exact behavior in D6/Nodequeue, there is no 6th element, and NULL is assigned but with no Notice.

So things appear to be functioning exactly the same way as in D6 so I guess the question to ask here is, Does D7 change at all how dynamic menu arguments are handled? and should that 6th element be prepopulated to something in $map before copied to $path_map?

servantleader’s picture

Having the exact same problem as febbraro. Don't know the answers to his questions. The API documentation does not mention any changes for Drupal 7, but it also not mention using paths in this way.

Damien Tournoud’s picture

#29 only means that you are missing a subqueue_to_arg() function. Drupal cannot read into the mind of the module developer here and decide how to resolve the missing placeholder when generating the link to the 'View' local task.

stoica.ionut’s picture

This menu hook produces the same kind of error, in fact the last 3 items do this.
Maybe this is an incorrect menu construction as the last menu items are in fact a submenu of a submenu.. maybe it helps


/**
 * Implements hook_menu().
 */
function examplemod_menu() {
  $items['admin/config/examplemod'] = array(
    'title' => t('Example - Module settings'),
    'description' => t('Configure Example Module parameters.'),
    'page callback' => 'drupal_get_form',
    'page arguments' => array('examplemod_admin'),
    'access arguments' => array('access administration pages'),
    'type' => MENU_NORMAL_ITEM,
  );
  
  // User control panel
  $items['user/%user/example/control-panel'] = array(
    'weight' => 3,
    'title' => t('Example - Control panel'),
    'page arguments' => array(1),
    'page callback' => '_examplemod_backend_control_panel',
    'access callback' => '_examplemod_backend_actions_access',
    'type' => MENU_LOCAL_TASK,
  );
  
  $items['user/%user/example/control-panel/overview'] = array(
    'title' => t('Overview'),
    'type' => MENU_DEFAULT_LOCAL_TASK,
    'weight' => 0,
  );
  
  $items['user/%user/example/control-panel/api'] = array(
    'title' => t('Developer API'),
    'page callback' => '_examplemod_backend_api',
    'page arguments' => array(1),
    'access callback' => '_examplemod_backend_actions_access',
    'type' => MENU_LOCAL_TASK,
    'weight' => 1,
  );
  
  // Rooms - backend
  $items['user/%user/example/control-panel/rooms'] = array(
    'title' => t('Rooms'),
    'page callback' => '_examplemod_domain_room_page_list',
    'page arguments' => array(1),
    'access callback' => '_examplemod_backend_actions_access',
    'type' => MENU_LOCAL_TASK,
    'weight' => 2,
    'file' => 'domain/room/pages.inc',
  );
  $items['user/%user/example/control-panel/rooms/list'] = array(
    'title' => t('Rooms list'),
    'type' => MENU_DEFAULT_LOCAL_TASK,
    'weight' => 0,
  );
  $items['user/%user/example/control-panel/rooms/%/read'] = array(
    'title' => t('Room read'),
    'page arguments' => array(1, 5),
    'page callback' => '_examplemod_domain_room_page_read',
    'access callback' => '_examplemod_backend_actions_access',
    'type' => MENU_LOCAL_TASK,
    'file' => 'domain/room/pages.inc',
  );
  $items['user/%user/example/control-panel/rooms/%/edit'] = array(
    'title' => t('Room edit'),
    'page arguments' => array(1, 5),
    'page callback' => '_examplemod_domain_room_page_edit',
    'access callback' => '_examplemod_backend_actions_access',
    'type' => MENU_LOCAL_TASK,
    'file' => 'domain/room/pages.inc',
  );
  $items['user/%user/example/control-panel/rooms/%/delete'] = array(
    'title' => t('Room delete'),
    'page arguments' => array(1, 5),
    'page callback' => '_examplemod_domain_room_page_delete',
    'access callback' => '_examplemod_backend_actions_access',
    'type' => MENU_LOCAL_TASK,
    'file' => 'domain/room/pages.inc',
  );
  return $items;
}

jonaswouters’s picture

subscribing

Sivaji_Ganesh_Jojodae’s picture

FileSize
233.37 KB

@sun, I have attached the debug_backtrace output in this comment. Please see if this helps.

@Damien, you are right "to_arg_functions" is empty. Can you point me to code where this is handled properly ?

aaron.r.carlton’s picture

subsribing. very possible i just got bit by this.

Damien Tournoud’s picture

Category: bug » support
Status: Needs review » Active

This looks like a support request to me. In both cases (the subqueue case and the case described in #32), you have a tab with incomplete placeholders. Typically, you have tabs like this:

  • mymenu/view
  • mymenu/edit
  • mymenu/cook/%/do

When you are on the View or Edit tab, Drupal needs to generate the link to the third tab. It cannot read your mind and magically fill in the missing placeholders.

So either you need to implement some *_to_arg() functions, or (more likely) your menu definition is broken and this is not what you actually wanted.

Damien Tournoud’s picture

Issue tags: -Needs tests

Removing the tag.

danielb’s picture

I have run into this problem as well when upgrading a module from 6 to 7.

mymenu/view
mymenu/edit
mymenu/cook/%/do
When you are on the View or Edit tab, Drupal needs to generate the link to the third tab. It cannot read your mind and magically fill in the missing placeholders.

So either you need to implement some *_to_arg() functions, or (more likely) your menu definition is broken and this is not what you actually wanted.

The behavior I expected was that when the placeholders are missing, the tab will not appear.
When I implemented _to_arg() function and made it return 0 or NULL or empty string, it caused the tab to permanently appear with the bogus empty value, so the link didn't actually go anywhere useful.

The reason for this is that there is another spot on the page (a form) that will cause the redirect to this tab (and the placeholder is a form value), and I want the user to see the navigation change reflected in the tabs.

In Drupal 6 I was able to work around this sort of thing using access callbacks that hide the link when the relevant arg(n) isn't present, but in D7 I get the undefined offset notice anyway.

edit: I actually did manage to get it working now using the _to_arg() WITH the access callback workaround. Had to make some compromises with my url paths and how I wanted tabs nested as well.

pillarsdotnet’s picture

Version: 7.x-dev » 8.x-dev
Category: support » bug
Issue tags: +Needs backport to D6, +Needs backport to D7

Bug also exists in a fresh install of d6, when visiting admin/build/themes. Here is how it happens:

  1. In function menu_local_tasks(), the $router_item fetched from menu_get_item() has a tab_root = 'admin/build/themes'
  2. The query
    $result = db_query("SELECT * FROM {menu_router} WHERE tab_root = '%s' ORDER BY weight, title", $router_item['tab_root']);
    

    fetches, among others, router items with the following paths:

    • admin/build/themes/settings/bluemarine/%
    • admin/build/themes/settings/chameleon/%
    • admin/build/themes/settings/garland/%
    • admin/build/themes/settings/marvin/%
    • admin/build/themes/settings/minnelli/%
    • admin/build/themes/settings/pushbutton/%
  3. The following code runs _menu_translate() on each item fetched:
        $map = arg();
    

    ...

        while ($item = db_fetch_array($result)) {
          _menu_translate($item, $map, TRUE);
    
  4. The _menu_translate() function includes the following code:
      $path_map = $map;
    

    ...

      $link_map = explode('/', $router_item['path']);
      for ($i = 0; $i < $router_item['number_parts']; $i++) {
        if ($link_map[$i] == '%') {
          $link_map[$i] = $path_map[$i];
    

    A runtime error is produced for each of the above paths, because $path_map = $map = arg() does not have an index corresponding to the placeholder position in $link_map = explode('/', $router_item['path']).

pillarsdotnet’s picture

Bug does not manifest in d7 or d8 because none of the standard themes have a placeholder in their menu paths. However, the code is still the same; only the data has changed.


MariaDB [d8]> select path from menu_router where tab_root = 'admin/appearance';
+-------------------------------------------------+
| path                                            |
+-------------------------------------------------+
| admin/appearance                                |
| admin/appearance/settings/global                |
| admin/appearance/list                           |
| admin/appearance/settings/bartik                |
| admin/appearance/settings/garland               |
| admin/appearance/settings/seven                 |
| admin/appearance/settings/stark                 |
| admin/appearance/settings/test_theme            |
| admin/appearance/settings/update_test_basetheme |
| admin/appearance/settings/update_test_subtheme  |
| admin/appearance/settings                       |
+-------------------------------------------------+
11 rows in set (0.00 sec)
drm’s picture

FYI, a client has this error and was unable to access Views admin pages (just got listing of other admin/structure pages as if admin/structure/views doesn't exist). Disabling Locale module made error go away and Views admin accessible.

foxfabi’s picture

had the same problem with a fresh d7 installation.
changing from node/%/view/diff/% to node/%node/view/diff/%hash solved the error messages.

Countzero’s picture

Just trying to summarize, as I stumbled on this developing a custom module.

I guess the question boils down to : What should be the behaviour of Drupal when a menu path doesn't have enough wildcards for its ['page arguments'] array ?

Two possible answers :

1 - This shouldn't happen, and it's the modules devs job to design their menu entries better.

2 - Drupal should have a way to handle this situation without letting PHP throw a notice.

Just for those (like me) who are not so cumfortable with all this, this fix should work in most cases, given 1 above is the right answer :

function your_wildcard_to_arg($arg) {
  ! $arg ? $r = 0 : $r = $arg; 
  return $r;
}

... which is basicaly what Damien talked about above.

Your mileage may vary, but the goal is to replace with something the void which disturbs the menu system. '0' here but anything could do as long as the code using the argument later is able to handle it.

Hope this may help.

pillarsdotnet’s picture

Version: 8.x-dev » 6.x-dev
Issue tags: -Needs backport to D6, -Needs backport to D7

So this is a d6-only bug as far as core is concerned?

Countzero’s picture

No, I was speaking about a D7 environment.

pillarsdotnet’s picture

You were speaking about a D7 environment with a custom module. I was speaking about a D6 environment with themes included by core.

Can you think of an existing module or theme shipped with D7 core which exhibits this bug? If not, then it is a D6-only bug.

Countzero’s picture

No, I can't, and have no problem with the classifications this issue ends in, but the fact remains that the same issue will arise when other devs will stumble on this, whatever the core version, and whoever is right about the underlying problem.

So it seems to me the question has to be answered. But well, I'm no core maintainer and will not argue about this further.

pillarsdotnet’s picture

Okay, then I'll try to roll patches against the above d6 themes when I get a chance.

The suggestion that core produce helpful error messages when contrib does something unexpectedly wrong has been rejected in plenty of other issues.

Countzero’s picture

Noted. The news didn't reach the lesser levels I use to dwell in.

sabeeln’s picture

Status: Active » Needs review

ctools-menu-translate-fix.patch queued for re-testing.

azovsky’s picture

Title: undefined offset in includes/menu.inc on line 576 » undefined offset in includes/menu.inc on line 576 function _menu_translate()
mikeytown2’s picture

Title: undefined offset in includes/menu.inc on line 576 function _menu_translate() » diff_menu() issue - undefined offset in includes/menu.inc on line 576 function _menu_translate()
Project: Drupal core » Diff
Version: 6.x-dev » 6.x-2.x-dev
Component: menu system » Code
FileSize
454 bytes

Here is the issue for nodequeue: #1005100: Notice: Undefined offset: 5 in _menu_translate() on line 576 of menu.inc

I'm moving this to the diff module.
Notice: Undefined offset: 4/5 in _menu_translate() (includes/menu.inc).

Backtrace from _menu_translate().

Array
(
    [0] => Array
        (
            [file] => sites/all/modules/ctools/includes/menu.inc
            [line] => 299
            [function] => _menu_translate
            [args] => Array
                (
                    [0] => Array
                        (
                            [path] => node/%/revisions/view/%/%
                            [load_functions] => Array
                                (
                                    [1] => node_load
                                    [4] =>
                                    [5] =>
                                )

                            [to_arg_functions] =>
                            [access_callback] => diff_node_revision_access
                            [access_arguments] => a:1:{i:0;i:1;}
                            [page_callback] => diff_diffs_show
                            [page_arguments] => a:3:{i:0;i:1;i:1;i:4;i:2;i:5;}
                            [fit] => 44
                            [number_parts] => 6
                            [tab_parent] => node/%/revisions/list
                            [tab_root] => node/%
                            [title] => Diff
                            [title_callback] => t
                            [title_arguments] =>
                            [type] => 128
                            [block_callback] =>
                            [description] =>
                            [position] =>
                            [weight] => 0
                            [file] => sites/all/modules/diff/diff.pages.inc
                        )

                    [1] => Array
                        (
                            [0] => node
                            [1] => 653083
                        )

                    [2] => 1
                )

        )

    [1] => Array
        (
            [file] => sites/all/modules/ctools/includes/menu.inc
            [line] => 474
            [function] => ctools_menu_local_tasks
        )

    [2] => Array
        (
            [file] => sites/all/modules/ctools/includes/menu.inc
            [line] => 445
            [function] => ctools_menu_tab_root_path
        )

    [3] => Array
        (
            [file] => sites/all/modules/ctools/includes/menu.inc
            [line] => 435
            [function] => ctools_menu_get_active_help
        )

    [4] => Array
        (
            [function] => ctools_menu_help
        )

    [5] => Array
        (
            [file] => includes/theme.inc
            [line] => 668
            [function] => call_user_func_array
        )

    [6] => Array
        (
            [file] => includes/theme.inc
            [line] => 1880
            [function] => theme
        )

    [7] => Array
        (
            [function] => template_preprocess_page
        )

    [8] => Array
        (
            [file] => includes/theme.inc
            [line] => 709
            [function] => call_user_func_array
        )

    [9] => Array
        (
            [file] => index.php
            [line] => 39
            [function] => theme
        )

)

In case someone else encounters this issue again; this is what my hacked _menu_translate() function looks like at the moment.

function _menu_translate(&$router_item, $map, $to_arg = FALSE) {
  if ($to_arg) {
    // Fill in missing path elements, such as the current uid.
    _menu_link_map_translate($map, $router_item['to_arg_functions']);
  }
  // The $path_map saves the pieces of the path as strings, while elements in
  // $map may be replaced with loaded objects.
  $path_map = $map;
  if (!_menu_load_objects($router_item, $map)) {
    // An error occurred loading an object.
    $router_item['access'] = FALSE;
    return FALSE;
  }

  // Generate the link path for the page request or local tasks.
  $link_map = explode('/', $router_item['path']);
  for ($i = 0; $i < $router_item['number_parts']; $i++) {
    if ($link_map[$i] == '%') {

      // Start Of Backtrace Code
      if (empty($path_map[$i])) {
        $bt = debug_backtrace();
        foreach ($bt as $key => $value) {
          if ($key == 0) {
            continue;
          }
          unset($bt[$key]['args']);
          unset($bt[$key]['object']);
        }
        drupal_set_message(str_replace('    ', '&nbsp;&nbsp;&nbsp;&nbsp;', nl2br(htmlentities(print_r($bt, TRUE)))));
      }
      // End Of Backtrace Code

      $link_map[$i] = $path_map[$i];
    }
  }
  $router_item['href'] = implode('/', $link_map);
  $router_item['options'] = array();
  _menu_check_access($router_item, $map);

  // For performance, don't localize an item the user can't access.
  if ($router_item['access']) {
    _menu_item_localize($router_item, $map);
  }

  return $map;
}

Here is the patch for the diff module. Need to flush caches after applying it.

adrianmak’s picture

I just came across this problem on d6.22 without diff module installed.

pillarsdotnet’s picture

I'm somewhat annoyed that mikeytown2 hijacked a legitimate bug report against Drupal core to report a bugfix to a contrib module, but as I don't use Drupal 6 anymore, and core developers don't seem interested in reviewing Drupal 6 bug reports, I can't be bothered to correct his error.

mikeytown2’s picture

@pillarsdotnet, @adrianmak
Follow the directions I gave in #52 on how to debug this. One of the modules you have installed is using hook_menu incorrectly. What your looking for is the page_callback; whatever module that is set to is the offending module; look at thats hook_menu callback in its .module file.

pillarsdotnet’s picture

@#55 -- See #39. The originally reported issue was the result of a bug in D6 core, and was reproduced without any contrib modules being installed at all. I don't doubt that many contrib modules share the bug, but using a single issue to cover all of them is both rude and inefficient.

grendzy’s picture

pillarsdotnet: #39 doesn't contain enough information to reproduce the issue. Now that we have the ability to update the issue summary, perhaps you can summarize with clear steps to reproduce in core. (Summary could be in this issue, or a different one - I don't really have an opinion there).

carn1x’s picture

subscribe

Not using Diff module

mikeytown2’s picture

@carn1x
Follow the directions in #52 on how to debug this

enricpera’s picture

#51 works for me, thanks!

davidwhthomas’s picture

Status: Needs review » Reviewed & tested by the community

#51 fixed the issue for me too, D7.8.

NROTC_Webmaster’s picture

#51 fixed it for me.

generalredneck’s picture

I think that patching core is the wrong way to go... Patching Diff should be the answer. Here's how I would roll it out...\

dagmar’s picture

@generalredneck #51 is a patch for the diff module.

generalredneck’s picture

@dagmar actually that's a patch for menu.inc. Which is drupal core, not the diff module, as indicated by
=== modified file 'includes/menu.inc'
within the patch file.

Patching the diff module itself is a lot less risky than patching menu.inc which is in core.

Here is an update removing $arg=NULL and making it just $arg as I went back and tested and adding =NULL actually doesn't get rid of the notices...

dagmar’s picture

Sorry, not #51, I meant #52. And yes, it is a patch for diff

diff --git a/diff.module b/diff.module
index efd177a..0238513 100644
--- a/diff.module
+++ b/diff.module
@@ -56,7 +56,7 @@ function diff_menu() {
     'title' => 'Diff',
     'page callback' => 'diff_diffs_show',
     'page arguments' => array(1, 4, 5),
-    'type' => MENU_LOCAL_TASK,
+    'type' => MENU_CALLBACK,
     'access callback' => 'diff_node_revision_access',
     'access arguments' => array(1),
     'tab_parent' => 'node/%/revisions/list',
generalredneck’s picture

Good call! I didn't see that one past all the code example he had... I appreciate the help!

#52 anyone who needs a fix that is less hacky than mine.

Kudos @dagmar and @mikeytown2

richsky’s picture

#52 give me notice: Use of undefined constant Y - assumed 'Y' in eval() (line 6 ...) content.module(1694)

Kudos @everyone

dww’s picture

I'm also seeing this on a D6 site with diff.module enabled. While #51 does indeed hide the notice, it's kind of yucky UI. When you view the diff between two revisions, since the menu item is "MENU_CALLBACK" it's just a page floating there without any tabs to get back to the node itself. #65 does actually work, both hiding the notices and maintaining this menu item as a tab so you can still get back to the node. However, it seems incredibly ugly to have to do this. I admit to not having carefully pondered every comment in this thread, but I guess there's no way around this bogus loader function. I though it was safe to use % if you don't need any special loading... go figure.

Alan D.’s picture

Did anyone try setting the load arguments? This could be a cleaner way of tackling this issue.

  'load arguments' => array('%map', '%index'),
markabur’s picture

Patched Diff module using #65 and my errors are gone. The Diff functionality itself seems to work just fine too.

sun’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.95 KB

Attached patch implements proper menu argument loaders for the revision ID arguments.

I don't know why this issue assigned to D6 — according to the 2,039,476 Google search result pages for this fancy error message, the same problem most likely still exists in D7.

The direct benefit of a menu argument loader is that an arbitrary non-existing revision ID immediately returns FALSE, and thus, automatically yielding a 404.

Unfortunately, the menu system in D6 and D7 only supports a single 'load arguments' definition for all dynamic arguments. We cannot declare that, since it would also be passed to %node, which in turn would result in node_load($nid, $nid/RECURSION) — in any case, any additionally passed argument to node_load() is interpreted as a particular revision ID to load, and since $nid != $vid, specifying 'load arguments' would make node_load() return FALSE for arg(1) already.

Therefore, this patch uses the lame (and discouraged) arg(1) within the diff_menu_revision_load() menu argument loader.

For performance, we're bailing out early if 1) $vid is empty or 2) $nid is empty.

For backwards-compatibility, the function signature of diff_diffs_show() is not changed and still accepts numeric $vid arguments.

mikeytown2’s picture

Alan D.’s picture

Happy to forward port to D7.

This patch is visually fine and appears completely harmless, but any takers for some D6 testers? Even though there is no update script, run update.php, which should clear the D6 menu caches from memory. If this doesn't, then an update script is required too :(

Note, other revisioning modules have had similar menu callbacks in the past. Please list any of these too if this patch hasn't helped. Note: Workbench Moderation probably triggers these notices, Revisioning is probably OK

sun’s picture

FWIW, the patch in #73 is against 6.x-2.3, which to my knowledge is ~6.x-2.x, and has been tested manually.

Alan D.’s picture

Re-roll against 6.x-2.x-dev

Can anyone confirm that a menu flush is not required for this?

i.e. running update.php should do it even though there is no updates :)

alberto56’s picture

dww’s picture

Haven't had a chance to re-test this. Love the approach.

One very minor code readability nit:

Now that the 2nd and 3rd params for diff_diffs_show($node, $old_vid, $new_vid) are either int vids or fully loaded node objects, perhaps we should just rename those params to old_revision and new_revision instead of leaving them as old_vid and new_vid. This doesn't change the API at all, callers don't care about this, only the internal implementation in this function would be more self-documenting. We'd of course still need the whole new block of code at the front of this function that checks the types of these parameters and does the right thing to ensure the rest of the function has valid new_node and old_node objects, but it seems cleaner if these params don't embed "vid" in the name if they're not necessarily (and not normally) just vids anymore.

Similarly, we should probably fix the PHPDoc for this function to reflect that these params accept mixed types now.

I'm not going to set this to 'needs work' b/c of this -- 'needs' is way too strong a word in this case. ;)

Otherwise, visually looks great to me. Bravo!

Thanks,
-Derek

dkl4’s picture

I was seeing the following notice on a Drupal 7.22 minimal install -- only Devel and my module. No other contrib and most non-essential core modules off.

Undefined offset: 2 in _menu_translate() (line 777

I'm building some menu paths and tabs which are a bit unconventional in the Drupal world, but which make sense to me. I'm using the %mymodule autoload feature. I only see the notice for the MENU_DEFAULT_LOCAL_TASK tab.

Comment 31 above made sense, so I put in the following stub function :

function mymodule_to_arg($arg) {
  return $arg;
}

It seemed to remove the notice with no observable side-effects... so far...

catch’s picture

Version: 6.x-2.x-dev » 7.x-3.x-dev
Issue tags: +Needs backport to D6
FileSize
2.5 KB

Here's a D7 patch with the feedback from #79 addressed.

douggreen’s picture

FileSize
1.88 KB

Why the change to node/%node/revisions/%/view? That seems to be causing me some problems.

+  $callbacks['node/%node/revisions/%diff_menu_revision/view'] = $callbacks['node/%node/revisions/%/view'];
+  unset($callbacks['node/%node/revisions/%/view']);

We should probably check that $vid is numeric in the loader function, otherwise, we end up trying to node_load($nid, 'view') in some cases where diff should not be used.

Shouldn't we also return FALSE and not NULL when the node_load fails altogether.

New patch attached addresses all of the above.

douggreen’s picture

Are we supposed to (a) always use placeholder names and menu to_arg/loader functions or (b) is it ever acceptable to just use %.

(a) If it's never acceptable to use %, then core uses the API improperly too.

(b) If it is acceptable to use %, then something might be broken in menu, because if any of these menu items was a MENU_LOCAL_TASK, we get the menu_translate warning.

I suspect this issue only shows up on the second placeholder in a menu definition, so I think these core definitions can trigger the problem. I tested a couiple of them. I ran into this problem putting a MENU_LOCAL_TASK tab on node/%node/revisions/%/view:

./block/block.module:  $items['admin/structure/block/manage/%/%'] = array(
./block/block.module:  $items['admin/structure/block/manage/%/%/configure'] = array(
./block/block.module:  $items['admin/structure/block/manage/%/%/delete'] = array(
./book/book.module:  $items['book/export/%/%'] = array(
./dashboard/dashboard.module:  $items['admin/dashboard/block-content/%/%'] = array(
./node/node.module:  $items['node/%node/revisions/%/view'] = array(
./node/node.module:  $items['node/%node/revisions/%/revert'] = array(
./node/node.module:  $items['node/%node/revisions/%/delete'] = array(
./openid/tests/openid_test.module:  $items['openid-test/redirected/%/%'] = array(
./user/user.module:  $items['user/reset/%/%/%'] = array(
./user/user.module:  $items['user/%user/cancel/confirm/%/%'] = array(
astutonet’s picture

Hi

I'm experiencing this same issue with the new version of the module webform (7.x-4.0-alpha9):

#2038933: Undefined offset: 4 in _menu_translate() (line 777 of includes/menu.inc) - Caused by Webform

The problem only occurs on pages webform and so far there is no some response.

I could see here that at some point this issue involved the D7 core, as in #27.

Then I ask: the patch in #27 can fix my issue?

Tks

Alan D.’s picture

Re-roll of douggreen version of sun/catch patch.

Renamed diff_menu_revision_load() to diff_node_revision_load() for clarity.

@catch: Why the change to node/%node/revisions/%/view, etc?

Alan D.’s picture

And an untested patch for Diff 6.x-2.x.

One rtbtc per version will see these committed :)

SocialNicheGuru’s picture

Deleted comment:

applied d6 patch to d7. trying to do too much too early in the morning. forgive me.

Alan D.’s picture

Issue summary: View changes

Appears to avoid issues related to non-mysql db too

#2321987: Diff-sqlsrv error, converting nvarchar to data type int

DrCord’s picture

@dkl4 (or anyone else) - did you ever find any problems with your usage of the subque_to_arg stub function you proposed using:

function mymodule_to_arg($arg) {
  return $arg;
}

I actually had to use:

function my_custom_url_part_to_arg($arg) {
  return $arg;
}

where my_custom_url_part is used in a url like admin/custom_folder/%my_custom_url_part

It solved the exact same error, I was not even using the Diff module, but was defining my own menu paths with modules combined with the Panels module and was having issues.

My understanding of why this works is that the subque_to_arg function is passing Drupal the value of the wildcard and so the path arguments is able to get a value in the $path array and therefore no more notices...am I correct about that reasoning? If so, this certainly sounds like a core menu bug...If not a core menu bug then needs to be documented in the hook_menu pages somewhere, and if it is on there, needs to be clearer, as I have read that page a lot trying to solve this.

taggartj’s picture

@DrCord:
thank You that fixed that nasty error message in custom module !
i was doing...

hook_menu...
  $items['something/%/%] = array(...

then changed to
$items['something/%somethig1/%somethig1'] = array(...
and added ...

function somethig1_to_arg($arg) {
  return $arg;
}
function somethig2_to_arg($arg) {
  return $arg;
}

and the error was gone

DrCord’s picture

Super glad I could help! I spent a long time figuring that out from scratch and so figured I should document it online fr others to not have to discover from scratch themselves :D

dobe’s picture

Status: Needs review » Reviewed & tested by the community

Tested, works great RTBC Patch 85 D7. I was not able to use diff with sql srv because of this issue.

The last submitted patch, 52: diff-676010-52.patch, failed testing.

The last submitted patch, 63: diff-remove_notices_in_menu_inc-676010-63.patch, failed testing.

The last submitted patch, 65: diff-remove_notices_in_menu_inc-676010-65.patch, failed testing.

The last submitted patch, 73: diff.menu-revision-diff-args.patch, failed testing.

The last submitted patch, 77: diff-676010-77-menu-revision-wildcard-revision-loaders.patch, failed testing.

Status: Reviewed & tested by the community » Needs work
Alan D.’s picture

anyone had this error recently?

4kant’s picture

I´m just having it ;-)
Latest core 7.53 and all modules up to date...
But in my case that seems to be caused by the module menu token - only on the /user - page.

Maybe I´ll have time to look into it later...

Alan D.’s picture

Re-roll if anyone wants to see if the patch is still needed / helps.

Alan D.’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Alan D.’s picture

Status: Needs work » Needs review

Really need to add a single test on D7 to prevent these fails...

00:22:00   ERROR: No valid tests were specified.
....
00:22:00 ERROR: Step ‘Publish JUnit test result report’ failed: None of the test reports contained any result
00:22:01 Finished: FAILURE
4kant’s picture

A supplement to my comment #100:
Yesterday I came here via Google and didn´t notice that this issue belongs to the project diff.
I´m sorry. I don´t have this module in my drupal installation.
In my case the issue has to be related to the module menu token.
The exact error message in my case is
"Notice: Undefined offset: 1 in _menu_translate() (in /includes/menu.inc on line 787)"
...

Alan D.’s picture

np. I had to use this patch myself back in 2013ish on a number of sites, but I have never seen the error myself over the last few years and Diff is installed on nearly every site we maintain (like 100's of sites)

Alan D.’s picture

Issue summary: View changes
Status: Needs review » Closed (cannot reproduce)

Closing as a can't reproduce, as the problem seems to have gone away back around 2015ish, and the change will interact with other contrib modules, triggering the potential for new issues & wasting the developers time altering other modules that extend / change the diff menu routes

frodri’s picture

Rerolling #86 for Diff 6.x-2.3.