Multilevel menu fails to create active menu trail with same link_path in different languages

subscribe

subscribe

If anyone can give some basic outline how I could write a patch and, or where the failure occurs, then I'll have a go - just not sure where to start with this one.

subscribe

Im planning to work on this tomorrow to try to get a basic patch or override module built. Any pointers in the mean time would be most welcome!

This also is an issue with Drupal 7.

The attached patch fixes the problem in Drupal 7 by injecting the correct menu link item into the static cache of menu_link_get_preferred(). This is rather hacky because it means that the query will be executed on all sites, not just when requested.

Also, this is only possible in Drupal 7, thanks to drupal_static(). There is no way to solve this problem without hacking core in Drupal 6.

A clean solution would be to request a patch for core that either adds a query alter tag to the query in the function menu_link_get_preferred() or add a drupal_alter() on the returned results.

Looks good, though not sure about adding init hooks and more queries for this...

Instead of using menu_init, could we run that function from inside other i18n_menu ones? As I see it, if you don't use i18n_menu_* API to retrieve or alter your menus, multilingual menu items won't work anyway, so...

Maybe we could run it the first time i18n_menu_navigation_links() or i18n_menu_localize_tree() is invoked.

The problem is that there is no way of knowing if and when menu_link_get_preferred() is called. It is usually rather late but a module might trigger that call earlier.

But we can try to move it to another function, I'l have to check when they are called

As said, the only clean solution would be to open a core issue and request a query tag for that specific query so that we can alter it dynamically.

@Berdir,

Yes, I understand the issue. My point is that other functions not using i18n_menu api would fail anyway if we have these per language items, so maybe we could fix it in our own API.

And question 2: If we need it as early as possible, why not in hook_boot() ?

> As said, the only clean solution would be to open a core issue and request a query tag for that specific query so that we can alter it dynamically.

Right, I was thinking the same, though feeling too lazy for that :-/

Anyway, as we are doing a 1.0 stable release ASAP (deadline this weekend so it is ready for Drupalcon), feel free to commit this one if this is the best option we've got so far.
(Just add some @todo note and let this issue as open task to work on it later)

There is a workaround to this issue that I have found that doesn't require patches (tested in D6.22 with i18n 6.19):
Use the 'menu per language' method to create your menus, don't put all your menu items into a single menu.
When you use a single menu per language, nested menu items that have a fixed 'system' path are still shown when you switch language.
While this is a good workaround, it really should work using either method.

Patch committed. Thanks.

I have created an issue in the core queue for that : http://drupal.org/node/1854134

Ok, but this one is committed, so reopen if there's any results with the core issue.

The above solution didn't make it work for me.

In the patch I posted in http://drupal.org/node/1854134, I added a tag in the menu_link_get_preferred() query.

Then I altered the query using hook_query_TAG_alter().

Doing this, menu links are always filtered by the active language.

You can also remove the code of menu_link_get_preferred() from i18n_menu_init().

Sorry for the last one.

This one should work.

Now that I finally understood how to submit a patch correctly.

This patch needs to be used with the one posted here.

It will filter by language menu links, so that if you have two menu items with same link path but different language,
there won't be any collision when determining the active menu trail.

So in any case we should wait for the core patch, I guess.

@Berdir,
As you are following both issues, please feel free to commit this one if you think it makes sense (Hope you don't mind me assigning the issue to you).
Should we rollback the previous one if the core patch gets in? (looks like a cleaner solution, doesn't it?)

@EtienneRd,
For the same link path but different language, wouldn't it be easier just using localizable links?
Also, the previous patch is not working for you?

The patch in #23, combined with the core patch works when you have a single menu in which you put menu items for each language.

The patch #23 can yield 2 menu links if there is both a LANGUAGE_NONE and a $language->language menu link in the menu_links table. One of them is chosen quite randomly.

I don't know the solution to this. We need a query that selects $language->language if it exists otherwise LANGUAGE_NONE. I think the first result will be selected so an OrderBy that puts LANGUAGE_NONE to the end might do the trick too.

I used the workaround of setting my LANGUAGE_NONE link to a defined language.

Postponing until core patch gets in.

hello, any progress on this issue, please ?

I updated the patch #23, because the tag name has changed to 'preferred_menu_links' in patch #11 on issue #1854134-11: Add a query tag to the query in menu_link_get_preferred() to allow modules to alter the query.

#31 worked for me in the following circunstances (without any core patch):

- 2 menu links to a taxonomy term
- The menu links has language, and are properly translated (one in catalonian, the other in spanish)
- The vocabulary is configured as "Localize. Termes are common for all languages, but their name and description may be localized."

Note: after applying the patch, I had to clean the cache in order to work

Thanks for the patch, I hope it to be set as RTBC

.

#31 also worked for me.Thanks.

#31 works for me as well.

#31 also worked for me.Thanks.

Please add to dev

#31 works for me too. Thanks!

#31 works for me too. Thanks!
Please add to dev...

1. +++ b/i18n_menu/i18n_menu.module
   @@ -857,3 +857,24 @@ function i18n_menu_init() {
   +
   

   unnecessary whitespace

2. +++ b/i18n_menu/i18n_menu.module
   @@ -857,3 +857,24 @@ function i18n_menu_init() {
   +  $tables =& $query->getTables();
   

   $tables = $query->getTables() should be fine here?

3. +++ b/i18n_menu/i18n_menu.module
   @@ -857,3 +857,24 @@ function i18n_menu_init() {
   +      if($language)
   

   if (

Rerolled #31 with the changes specified in #39.

This patch works for me. Thanks!

Marking RTBC as this only adds some brackets and a comment. Shouldn't we rollback the code from #13 as well?

#43 worked for me. Thanks!

Using Stefan.r patch #43 in combination with a rollback of commit from #13 (from 5 years back)

Rolling back code from #13

reroll for test

#45 is a rollback of this commit:

commit 2a15f1a577603640b42c3e4a3175101102fa1019
Author: Jose Reyero <consulting@reyero.net>
Date:   Sun Sep 18 16:00:32 2011 +0200

    Issue #881322 by Berdir: Fixed Multilevel menu fails to create active menu trail with same link_path() in different languages.

in combination with patch 43 by @stefan.r

see what the testbot says

note: that if we remove i18n_menu_init
then the pending patch in:
#1630706: i18n_menu very slow during install profile due to hook_init() behavior
will not work or will have to be refactored

however,

major performance gains will be achieved by removing i18n_hook_init

see:
#1630706: i18n_menu very slow during install profile due to hook_init() behavior

the performance patch in the above issue will no longer be necessary

HOWEVER, if we combine a refactored patch #43 with patch #1 from
#2447261: Add the 'preferred_menu_links' tag in the i18n_menu_init()

then we'll have best performance and deal with #2447261 at the same time be able to close that.

ok, so scrap #47 ,

#45

or
#43 ?

ah, I see the problem with #47, I'll create a new patch later today or tomorrow.
should be great

Ok, this is fixed in the dev branch, its a bit higher risk change than what was added to the 7.x-1.14 release so lets let this simmer in dev for a while.

The fix as committed using patch #45 knocks off these issues as well:

#2646572: Replace code in hook_init() with hook_query_preferred_menu_links_alter().

AND

resolves this:

#1630706: i18n_menu very slow during install profile due to hook_init() behavior

and possibly also resolves this issue:

#2447261: Add the 'preferred_menu_links' tag in the i18n_menu_init()

Thanks @stefan.r

and everyone else.

This fix is in 7.x-1.x dev , if you're happy with this fix in there, I'll tag a new release for this called 7.x-1.15

I'm expecting that @stefan.r suggestion to remove the i18n_menu_init in commit in #55 will improve performance although I haven't yet done any profiling. See
#1630706: i18n_menu very slow during install profile due to hook_init() behavior

I'll be listening for your feedback.

Is this patch (#43) still necessary for latest Drupal 7 version?

What if the patch is still applied?

No patch is necessary for the latest i18n 7.x-1.25
this was fixed and closed a long time back.

If you have some new issue that you think might be related please create a new issue with the explanation and details and you can put a link to it here. All new issues should be new issues, closed fixed issues can get links if they're suspected to be related issues.

If you're having issues with the latest release please open a new support request issue with the explanation and we can investigate .

As a best practice, you should always make a backup of your database before upgrading and you should also try upgrades in a test environment first before upgrading on a production system.