It looks like the menupath token is problematic with pathauto in the following situations:

- node created with menu item added, but node left unpublished

What this does is alias the node to the parent menu item, causing a duplicate alias or rather an alias of the parent item with a -0 after it. I can't then even get to the original alias without the -0. Has anyone else see this?

CommentFileSizeAuthor
#10 token_node.inc-339211-10.patch1.07 KBtravismiller
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benorgan’s picture

I'm experiencing this too, does anyone have a fix?

benorgan’s picture

Version: 6.x-1.11 » 6.x-1.12
Priority: Normal » Critical

Changed version, also tested with dev version and same behaviour

greggles’s picture

Marked #526222: When creating an unpublished node and using menupath as the alias the alias is incorrect as a duplicate.

@benorgan - do you plan to work on this? Marking it as critical doesn't actually help to get the bug fixed sooner.

benorgan’s picture

Priority: Critical » Normal

This is indeed a duplicate of that comment.

I do have some time to work on this but I don't really know how to go about it.

I understand it's something to do with using the menu_tree_all_data function from includes/menu.inc which then calls menu_tree_check_access but this function limits it's query to nodes with status = 1 (i.e. published). So I guess there needs to be another way to get the menu information for the token, but I don't know Drupal well enough to know where to look.

Was marked as critical as I really need this for a site that's going into production.

mikshimonster’s picture

#529284: use path module to render url

I'm having the same issue here. I wasn't sure which module was responsible for this. I dont have any programming skills either...

If your intention is to create multiple pages in unpublished/pending mode for a reviewer/editor, it would be nice to see the followings when creating each page (btw, im building a marketing site and not a community):

  1. creates unpublished/pending menu item (in menu) that can only be viewed by certain users (reviewers/editors) who can view unpublished contents. this menu item must also be visible in Parent Item when creating another page so you can attach a child page to it.
  2. pathauto creates the correct menupath for the unpublished/pending revision.
  3. when published, both menu item and page become available to public.

something like this, acting as staging server. i guess its easier to say than something done :)

alexharries’s picture

One possible solution would be to present a version of [menu-trail-parents-raw] with slashes between the parents (at the moment this token has no slashes), and then use:

[menu-trail-parents-raw]/[title-raw] (or, even better, present [menu-trail-parents-raw] with a trailing slash if and only if it is longer than 0)

... instead of:

[menupath-raw]

It looks like this might give a virtually identical result, and will ensure there's a sensible title. Example URLs might then be:

Location on menu: foo/bar/turkey/mango/noodle
[menupath-raw] = foo/bar/turkey/mango/noodle
[menu-trail-parents-raw] = foo/bar/turkey/mango/
[title-raw] = noodle

Location on menu: bananana/noodle
[menupath-raw] = bananana/noodle
[menu-trail-parents-raw] = bananana/
[title-raw] = noodle

Location on menu: Not on menu, page title: "wibble"
[menupath-raw] =
[menu-trail-parents-raw] =
[title-raw] = wibble

Could this work?

alexharries’s picture

A very quick update - the missing slashes in the [menu-trail-parents-raw] token was a bug and not by design - the new version of menutrails includes slashes in the token by renaming it to include "-path", as follows:

[menu-trail-parents-path-raw]

... which means [menu-trail-parents-path-raw]/[title-raw] should work quite well now. Trialling it on http://www.kssdeanery.org now...

mikshimonster’s picture

In theory, it should work. But it doesn't. I think that the durpal or the modules think that the menu parent path is one below since the menu for the page that you are creating hasn't been created. Thus in pending/unpublished mode, the result is:

first-level/second-level/forth-level
(see that it is missing third-level)

Location on menu: first-level/second-level/third-level/forth-level
[menupath-raw] = first-level/second-level/third-level
[menu-trail-parents-raw] = first-level/second-level/forth-level
[title-raw] = forth-level

Nice try though.

Another one that i tried was [menupath-raw]/[menu-link-title]. It works nicely when it is in pending/unpublished. But when it is published, yes it pastes menu link title twice.

travismiller’s picture

i hack-solved this by modifying token_node.inc:_menu_titles

this makes [menupath-raw] consistent regardless of selecting "published" on the node. with this implementation, [menupath-raw] should never include the node's own link title. like #8 above, use [menupath-raw]/[menu-link-title] to include the node's link title in the pattern.

old:

      $titles[] = $current['link']['title'];
      if ($current['link']['href'] == "node/". $nid) {
        break;
      }

new:

      if ($current['link']['href'] == "node/". $nid) {
        break;
      }
      $titles[] = $current['link']['title'];
travismiller’s picture

Status: Active » Needs review
FileSize
1.07 KB

Here's another approach.

This gets the link titles from the parent menu links referenced directly in the node's menu link (p1-pX) rather than looping over the results of menu_tree_all_data. This also consistently includes the node's own link title as seems to be the intent of this function. Patch attached.

old:

function _menu_titles($menu_link, $nid) {
  $tree = menu_tree_all_data($menu_link['menu_name'], $menu_link); 

  // Get mlid of all nodes in path - top-most parent to leaf node.
  $parents = array();
  for ($i = 1; $i < MENU_MAX_DEPTH; $i++) {
    if ($menu_link["p$i"]) {
      $parents[] = $menu_link["p$i"];
    }
  }
    
  // Build the titles in this hierarchy. 
  $titles = array();
  $current = array_shift($tree);       
  while ($current) {
    if (in_array($current['link']['mlid'], $parents)) {
      $titles[] = $current['link']['title'];
      if ($current['link']['href'] == "node/". $nid) {
        break;
      }
      // Go deeper in tree hierarchy.  
      $tree = $current['below'];       
    }
    // Go to next sibling at same level in tree hierarchy.
    $current = $tree ? array_shift($tree) : NULL;
  } 
  return $titles;
}   

new:

function _menu_titles($menu_link, $nid) {
  // Get mlid of all nodes in path - top-most parent to leaf node.
  $parents = array();
  for ($i = 1; $i < MENU_MAX_DEPTH; $i++) {
    if ($menu_link["p$i"]) {        
      $parents[] = $menu_link["p$i"]; 
    }
  }
    
  // Build the titles in this hierarchy.
  $titles = array();
  foreach ($parents as $parent) {
    $ml = menu_link_load($parent);
    $titles[] = $ml['link_title'];
  }
  return $titles;
}
mikshimonster’s picture

I tested puckbag's patch above and it's looking good on both unpublished and published path. You should use [menupath-raw] only without [menu-link-title] to make this work.

Thank you puckbag!!!

anfrage’s picture

Problem still exists. It would be very nice if the solution would be included into a next version of Token.

Thank you very much!

Dave Reid’s picture

Status: Needs review » Closed (duplicate)
Kafu’s picture

+follow

yang_yi_cn’s picture

Version: 6.x-1.12 » 6.x-1.15
Status: Closed (duplicate) » Needs review

@Dave Reid

I don't think this is a duplicate of #333590: Node menu token fails (node grants not saved yet, menu access checking, oh my!).

I tried your patch in http://drupal.org/node/333590#comment-3109074 , (I converted drupal_static() to static for D6, of course), and that patch didn't fix the problem.

#333590 is more related to node_grant_access, while this bug is not related to node access. Even I'm user 1 and no node grants are set for the content type, saving an unpublished node still will make the path saved as it's parent's path.

The logic of the #10 fix here is basically try to skip all the menu access control and just get the text of the titles to make the path.

In my opinion it's a better approach as I don't think there's any security issue by just generate a path from text. The real access control needs to handled by other modules. I think this patch should also apply to #333590 instead, not vice versa.

Think about this scenario:

Menu tree is A / B / C

People don't have access to B, but have access to C.

There's nothing wrong that when people visit C, they see the path as A/B/C. When they try to click A/B, they still get access denied. The path text itself should not be related to menu access control at all.

This is my 2 cents.

Dave Reid’s picture

Status: Needs review » Closed (duplicate)

Thats exactly what #333590: Node menu token fails (node grants not saved yet, menu access checking, oh my!) is also going to do: loading the menu items without any access control.