Example of menu:
book ('link_path' => taxonomy/term/1/all)
-- art
-- politics
Problem: After clicking on the book link, the menu won't expand to show its children like it normally should.
Reason: A call to get_menu_item() on the page 'taxonomy/term/1/all' will return an item with a href field of 'taxonomy/term/1'. This href won't match the link_path for the book item in the menu, which in turn won't be marked as a member of the active trail, which leads to it's children not showing.
Fix: By having taxonomy/term/%/all in the menu router get_menu_item() will return the correct href
The problem also exits in D6, D5 has the intended behavior.
Comment | File | Size | Author |
---|---|---|---|
#13 | core_251868-10.patch | 681 bytes | egfrith |
#10 | core_251868-10.patch | 679 bytes | egfrith |
#4 | core-251868-4.patch | 835 bytes | egfrith |
#1 | taxonomy.module.patch | 1.06 KB | twiinz |
taxonomy.module.patch | 1.06 KB | twiinz | |
Comments
Comment #1
twiinz CreditAttribution: twiinz commentedMy bad.
Comment #2
catchSeems reasonable to have an explicit callback for this path, however there's now loader functions for %taxonomy_terms, so could use a re-roll for that.
Comment #3
egfrith CreditAttribution: egfrith commentedI'm interested in this patch, as if/when it gets into D7 it might fix this problem for me in D6. I hope to look at the patch in D7 sometime soonish.
Comment #4
egfrith CreditAttribution: egfrith commentedI've updated the patch now against the latest HEAD. It's very similar to the patch at comment #1, except that it now has the reference to %taxonomy_terms and is minus the file argument.
Comment #5
egfrith CreditAttribution: egfrith commentedThought I had changed the status -- but evidently not.
Comment #6
egfrith CreditAttribution: egfrith commentedJust realised that that this needs an invocation of menu_rebuild() to take effect - would it be worth putting this in an update function in the taxonomy.install file? Or is it too trivial a change to warrant this?
Comment #7
8bitplateau CreditAttribution: 8bitplateau commentedIve been having the same issue in D6, is there a patch against that version ?
Comment #8
egfrith CreditAttribution: egfrith commentedI think the first patch in the issue might work. Once this issue is fixed in 7.x, we can change the version to 6.x and roll a proper patch for 6.x.
Comment #9
chx CreditAttribution: chx commentedAre we fixing the symptom instead of the cause? We already have taxonomy/term/%taxonomy_terms defined and we use the "magical" argument passing to get the 'all' to taxonomy_term_page. How is this taxonomy specific? Any other path defined with an additional argument would have the same problem. You need a menu.inc patch not a taxonomy.module.
Comment #10
egfrith CreditAttribution: egfrith commentedHere is a patch to menu.inc that fixes the problem for me, though we will see what the testing bot thinks!
The problem appears to be in _menu_translate(), where the extra magic arguments are not being passed to $link_map and hence the href. This patch ensures that any extra arguments (in the taxonomy example above, "all") are being added to the path.
Comment #11
egfrith CreditAttribution: egfrith commentedComment #13
egfrith CreditAttribution: egfrith commentedComment #14
pwolanin CreditAttribution: pwolanin commentedI'm not sure this is a good idea, versus just defining an additional router path that includes the 'all'. Seem like it's inviting the opposite problem - e.g. any extra trailing path parts now produce a mismatch like reported here.
Comment #15
catchI think I agree with pwolanin - or at least, defining the extra router path is both more self documenting and definitely won't break anything. Could you re-roll for that?
Comment #16
egfrith CreditAttribution: egfrith commentedI could reroll with documentation, but before doing so, I'd like to know what chx thinks, as he originally suggested patching menu.inc.
Comment #17
ericjam CreditAttribution: ericjam commentedSubscribing. Here is an example of my problem:
On the /grapevine/ page the Menu highlights that item (ie: attaches .active)
Clicking on Musings which is a sub item removes .active from the primary Tax1 and doesn't add .active to Tax2
I should note Musings is also a primary level term that visually is applied as a sub term in menu. Both Tax 1 and Tax 2 come off the main.
(www.twincityscene.com/grapevine)
Comment #18
Azol CreditAttribution: Azol commentedThis issue could also be reproduced with menu links similar to taxonomy/term/any_ID/any_depth, not only for "all" argument.
Comment #19
TDobrowolski CreditAttribution: TDobrowolski commentedIs there still any solution for this issue????
Comment #20
egfrith CreditAttribution: egfrith commentedThere are two solutions for this issue (#4 and #13), but the problem is that a consensus has not been reached about what the correct approach is. Someone needs to have a proper look at what the Right Thing to do is. I would love to be that "someone", but I haven't found the time yet, and it would take me a long time as I'm not very familiar with the menu system... It will be at least a month before this gets to the top of my priority list.
I'm putting this issue back in the 7.x queue, where it might get more attention, especially in the bug-squishing phase which D7 is moving into.
Comment #21
Xen CreditAttribution: Xen commented#13 breaks tabs on /user page. When clicking on 'Register' it causes 'Log in' to have the path user/register. Possibly messes with local tasks elsewhere.
Comment #22
yingtho CreditAttribution: yingtho commented#4: core-251868-4.patch queued for re-testing.
Comment #23
crifi CreditAttribution: crifi commentedDoes this patch also solve the additional problem reported in #372095: Menu doesn't expand if pointed to /taxonomy/term/[id]/all? Also parameters with more terms like /1+2+3 and /1,2,3 and combinations like /1+2,3 are affected...
Comment #24
EvanDonovan CreditAttribution: EvanDonovan commentedI just flagged #372095: Menu doesn't expand if pointed to /taxonomy/term/[id]/all as duplicate, on the presumption that they were the same issue, or at least that that issue shoudl be against 7.x as well. My apologies if that was incorrect.
Comment #25
crifi CreditAttribution: crifi commentedIn my opinion the correct approach is to fix this against the menu system (menu.inc) since the taxonomy module no longer support multiple tids and depth modificator #503456: Remove multiple tid and depth handling for core taxonomy paths.
Comment #26
yan CreditAttribution: yan commentedPatch from #14 works fine for me in Drupal 6.19.
Comment #27
crifi CreditAttribution: crifi commentedIn Drupal 6.19 the patch #13 does not work for me in all cases (see Description #23) and breaks tab-menus. By the way tests should be against the latest head of Drupal 7. I set the status back to "needs work", because I think we need a new patch.
Comment #28
crifi CreditAttribution: crifi commentedThis is a summary for the circumstances of this bug under drupal 6x and 7x (with views).
== Drupal 6 ==
OK taxonomy/term/1
X taxonomy/term/1/all
X taxonomy/term/1+2
X taxonomy/term/1+2/all
== Drupal 7 (with the new views module enabled which handels combined arguments) ==
OK taxonomy/term/1
OK taxonomy/term/1/all
OK taxonomy/term/1+2
OK taxonomy/term/1+2/all
I don't no why, but drupal 7 is no longer affected.
Comment #29
crifi CreditAttribution: crifi commentedUpdate: It seems that this problem occurs with clean urls enabled. On a fresh Drupal 6 installation with disabled clean urls I can not reproduce the error. With clean urls enabled it emerges.
In Drupal 7 it's working with both, clean urls enabled and not enabled.
I think this issue is in dependency to #284899: Drupal url problem with clean urls
Comment #30
crifi CreditAttribution: crifi commentedThe patch http://drupal.org/node/284899#comment-2545242 works for my problems but is still not ready to use. All followers should test, if this issue here is a duplicate to our issue #284899: Drupal url problem with clean urls.
Comment #31
crifi CreditAttribution: crifi commentedUnder the presumption, that this is a duplicate of the bug described, i close this bug as a duplicate. Please reopen if i'am wrong.
Comment #32
tamsoftware CreditAttribution: tamsoftware commentedThe bug arrived here when upgrading to Drupal 7. On Drupal 6 taxonomy menus worked perfectly for me, with clean urls. I upgraded to 7 and menus won't expand. The content is there when I look into the menu structure, and if I force expansion manually the links work.
Deactivating clean urls in 7.0 doesn't fix.
I'm reopening ; if it shouldn't be, my apologies
Franck
Comment #33
crifi CreditAttribution: crifi commentedFor clarification:
- This bug here concerns menu items with taxonomy URLs like this: taxonomy/term/1+2/all or taxonomy/term/1/all
- It doesn't concern any special code of the module "Taxonomy Menu" http://drupal.org/project/taxonomy_menu
- Can you give some examples for the menu items which doesn't work?
- You know that Drupal 7 has no multiple argument 1+2 or 1,2 support in core? You have to activate the taxonomy controller of the views module.
Comment #34
crifi CreditAttribution: crifi commented@TAM Software: This could be also this bug #942782: Custom menus never receive an active trail.
Comment #35
jordotech CreditAttribution: jordotech commented#13 patch seems to have worked for me for Drupal 6.20, will let you know if I run into any problems. Update: I spoke too soon, having same menu tab problem etc. Hope this gets resolved, looking into it.
Comment #36
crifi CreditAttribution: crifi commentedAgain: The patch in #13 isn't working for all cases. See comments #27 and #14/#15. I think the patch #13 solves symptoms instead of the cause.
We still need more information why the menu isn't working in some cases even with patched clean URL...
Do you have already patched the bug I mentioned #284899: Drupal url problem with clean urls?