When using [menupath] as token, a dash (-) is inserted where it should have been a slash (/).

At line 58 in token_node.inc, $values['menupath'] is assigned a string with the menu path. The string contains spaces and other characters that are replaced by a dash (-) at line 562 in pathauto.module. Unfortunately, this last call also replaces the slashes (/) inserted in between the menu trail elements at line 58 in token_node.inc. Hence, instead of a/b/c the path becomes a-b-c.

Furthermore, the path is prefixed with the name of the menu.

This was tested on Pathauto 5.x-2.x-dev and Token 5.x-1.x-dev installed on a fresh Drupal 5.1.

CommentFileSizeAuthor
#12 token_6.patch3.14 KBlitwol
#11 token_4.patch3.1 KBlitwol
#4 token_menupath_notoplevel_0.patch539 bytesTBarregren
#2 token_menupath_notoplevel.patch774 bytesgreggles

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Title: [menupath] inserts - instead of / » menupath includes top level of menu

I believe the / vs. - problem is fixed in the current version - can you try again.

The problem with the top level menu showing up at the root is still true, though I'm not sure whether that's a problem or not.

Can you explain further what you feel the proper behavior should be on that and why?

Thanks for your patience!

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Title: menupath includes top level of menu » menupath should not include top level of menu
Project: Pathauto » Token
Version: 5.x-2.x-dev » 5.x-1.x-dev
Assigned: Unassigned » greggles
Status: Active » Needs review
StatusFileSize
new token_menupath_notoplevel.patch774 bytes

The menupath token description says "The menu path (as reflected in the breadcrumb), not including Home or [menu]. Separated by /."

As long as that's the case, it shouldn't include the top level item. Since we already have the [menu] token, anyone who needs it to include the menu can just use [menu][menupath] which should be fine.

Attached patch builds up the $trail only as long as the parent is not the top level (name) of the menu.

TBarregren’s picture

TBarregren commented

Sorry, but both problems remain. :-(

I have tested the latest version (i.e. 2007-Aug-15) of Token 5.x-1.x-dev and applied the patch and also the latest version (i.e. 2007-Aug-15) of Pathauto 5.x-2.x-dev. When using [menupath] in Pathauto I don't get abc/def/ghi as expected but primary-links-abc-def-ghi. Thus, both problems remain: instead of / I get - and the path is prefixed with the name of the menu.

This bug makes the [menupath]-token to not work as expected at all. Perhaps that is enough to qualify the bug as critical?

BTW, I appreciate very much all work with Token and Pathauto. They are both awesome , and should really be in core IMHO. Thanks!

TBarregren’s picture

TBarregren commented
StatusFileSize
new token_menupath_notoplevel_0.patch539 bytes

After correcting a typo in the patch everything works as expected.

When I took a closer look at your patch I found that a && was missing:

  while ($mid && $_menu['visible'][$mid] && $_menu['visible'][$mid]['pid'] != 0) {
                                         /\
                                        /||\
                                         ||
                                  The missinng &&

I include an updated patch.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Patch (to be ported)

quite right you are, sir!

Thanks kindly. Committed to the 5.x-1.x branch. Now it's up to eaton.

AdrianB’s picture

AdrianB commented

I had both problems ([menu] in [menupath] and dash instead of slash) with Pathauto 5.x-2.0-beta2 and Token 5.x-1.8. Switching to Pathauto 5.x-2.x-dev (2007-Aug-26) and Token 5.x-1.x-dev (2007-Aug-24) fixed the [menu] problem, but not the dash problem.

After reading this issue it seems like maybe the fix for the dash problem is not commited, right? If I'm not ready to try patches, should I downgrade to Pathauto 5.x-1.2?

AdrianB’s picture

AdrianB commented

I downgraded to Pathauto 5.x-1.2 to get the desired behaviour.

(I know the reason is to quickly attract testers but I disagree about having Pathauto 5.x-2.0-beta2 as the recommended version since there's to many oddities.)

susez’s picture

susez commented

I have tested pathauto 1.2, 2.x-dev, and 2.0-beta2, but the dash problem remains. I'm not sure if it is a pathauto or token bug. Any fix?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Upgrade token to 5.x-1.x-dev and it should be fixed.

Thanks.

susez’s picture

susez commented

I am using pathauto-5.x-2.x-dev and token-5.x-1.x-dev and the dash is still inserted where it should be a slash.

litwol’s picture

litwol commented
StatusFileSize
new token_4.patch3.1 KB

Friends, i hope my patch will solve this issue once and for all :)

the patch includes a lot of changes mostly due to deleting useless whitespace (my IDE does that automaticaly)

the actual change was very little.

i hope this will receive quick review and if it wasnt my code i would have RTBCed it already ;)

litwol’s picture

litwol commented
StatusFileSize
new token_6.patch3.14 KB

I've uplaoded the wrong patch, here's the correct one.

litwol’s picture

litwol commented

What i've done is just reverse the order of lines 62 and 63.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@litwol - your patch is against the 1.8 version of token and therefore will not apply properly to the head of development. Can you please test (and create patches against) the end of the DRUPAL-5 branch?

litwol’s picture

litwol commented

@greggles,

I have just checked out the HEAD and in there it seems to be already fixed in the same way my patch proposed the solution

on this regard i advise this issue to be labeled as fixed/closed.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I agree that it should be "closed" but I'm waiting for it to be applied to the 6.x branch.

@litwol - Thanks for your investigation and input.

tormu’s picture

tormu commented

[catpath] doesn't put slashes as the separator either, just dashes :O
Hope this could be fixed with the same patch.

tormu’s picture

tormu commented
Title: menupath should not include top level of menu » Dashes instead of slashes in the path
Priority: Normal » Critical

I read through all the comments and got the impression that the problem given in the old title was fixed, but the other problem, which I changed the title for, is not.

I have [catpath] as the pattern for the nodes type "page".
I have a vocabulary with a parent term "test" and child term "testing", and a test article is assigned to this child term.
So I should be getting test/testing as the path, but getting test-testing

Using the newest 5.x dev versions of both Pathauto and Token.

PS. [catpath] seems to be called [termpath] these days in Pathauto..

tormu’s picture

tormu commented
Status: Patch (to be ported) » Active

Just changed the status as I don't have a patch to provide.

dmitrig01’s picture

dmitrig01 commented
Title: Dashes instead of slashes in the path » menupath should not include top level of menu
Priority: Critical » Normal
Status: Active » Patch (to be ported)

the other problem

Please open another issue.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Patch (to be ported) » Fixed

Ported in eaton's work this past weekend.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

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