Add Entity Translation (entity_translation) support for menu items using Menu Translation

About entity translation related patches, we should wait at least until it has some tagged release. For the rest of the issues, not sure which part still applies, but anyway we better don't mix these small issues with such a big patch.

Rewording as a feature request.

Subscribing

@das peter thanks
the patch applied cleanly and I can see (not translated/localized) menu item appear when I view site in different language - link also points to the correct content (path is correctly pointing to the translated node)

BUT how do I get to localize the text for the menu or description for the menu ?
shouldn't the option be available at the edit for for the language like the rest of the fields and URL alias ?

I have the menu configured to "Translate and Localize" is that the correct setting ?

@das-peter thanks for the answer

In #1123610: Menu item translation we try to solve the same problem using entity_translation modification
maybe helpful so there is no duplication of effort

Please read and help with #1271196: Please created tagged release ASAP (help integrating with other modules)

That one is atm the only major roadblock for adding any i18n/ET integration.

It seems we already have an alpha1 of entity translation so we can move ahead with this one.

About the patch:
- We cannot just check for entity type (node, etc.) we need to check for specific node types and, I guess, use entity_translation_node_supported_type()
- A missing critical feature of the patch is properly handling the node translation tab depending on whether the node type is entity translation enabled.

Forget about the second point as it seems ET module handles it properly.

Subscribe

Committed the first small part, the function i18n_entity_translation_enabled()

Related issue that might make this unecessary: #1174242: Properly override the node/%node/translate menu router path.

after #13 the new patch becomes get reduced to the attached one

but it produces the following notice

Notice: Undefined index: i18n_mode in i18n_menu_link->get_translate_mode() (line 65 of G:\sites\all\modules\i18n\i18n_menu\i18n_menu.inc).

and all menu items irregardless of language appear on menus

Does anyone have a solution for the undefined index problem in #15?

I thought that the attached patch was meant to fix this. Did you give it a try?

...btw, I see that the patch fixes some trailing spaces and corrects indentation, but I believe that this part should be left alone:

     if (isset($object['options']) && !is_array($object['options'])) {
       $object['options'] = unserialize($object['options']);
-    }
+  }
     parent::__construct($type, $object);
   }

That curly bracket was just fine were it was before.

Yes, I tried it and I get the undefined index error in line 65 of i18n_menu.inc

To describe it in more detail, I did this:

• I created some menu links and set the language to "Language neutral"
• I installed i18n and applied the patch from #15
• I changed the language of one menu item to German
• After saving the link I get the error message and the "Edit Menu Link" and "Translate" tabs disappear.

This means I can not create a translation anymore.

I analyzed the patch before I applied it and came to the same conclusions as in #17 and did not change the indentation. But if I did, it would not make a difference.

I can translate the menu items if I do not set a language for the original link. This means in my case that I must set the German version as Language neutral. In this case I can translate it to English.

But menu items with child links are always shown in the Language neutral version, no matter which language is chosen by the user.

@Martin Mayer
maybe you are looking at this bug #1266486: entity translated children menus are not present when parent is viewed [menu entity_translation support]

Yeah, I was about to say that this sounds like a whole different issue too.

@Martin Mayer: What I was asking back in #17 was if the patch in #15 solved the "Undefined index" error for you (since that is what it's meant to do).

@GiorosK: This must be a different issue. Parents and children are shown. But the parent on the base level is always shown in the "Language neutral" version. If parents are not at the base level, they are shown translated if the user chooses a different language.

@klonos: It does not solve the "Undefined index" error for me. As I described in #18, I do get the error, when I change the setting of a menu item from "Language neutral" to "German". As soon as I save it, the error appears and the tabs disappear. You can see this in the attached screen shot.

What I wanted to say with #19 is that it is a workaround to keep the German version as language neutral and to translate it to English. Then no error occurs.

Here's an updated version against current dev...

Changes:

• No whitespace changes anymore.
• Added entity translation check via entity_translation_node_supported_type()
• Removed 'Undefined index' notice (#15)
• "Fixed" i18n_menu_i18n_string_objects(), which means that strings for ET-enabled menu items are not removed anymore during a "string refresh"

Outstanding issues:

1. i18n_menu_link_has_entity_translation() needs more work; it currently does a node_load() for every node that has a menu item (I think?); definitely needs caching or some other kind of clever performance improvement.
2. The "source string" of a menu link is always stored according to i18n_string_source_language(), not respecting the "source language" of a menu item. In other words, if my default site language is English, and I create a new node in German, then the German menu title is stored as the english source string.
3. As mentioned previously, integration with entity translation UI would be great. Ideally I'd like a "menu item translation form" on the respective entity translation form.

Not sure what the best way is to solve the second point. Ideas?

Attached patch solves issues #2 and #3 from my previous comment.

For the "source string issue" (#2) I decided to force all menu items (that belong to entity_translation based nodes) to always be saved in the language specified by i18n_string_source_language().

This means, that e.g. if your site language is english, and you edit the english entity-translation of a german node, then when you save the menu_link translation, it will actually update the original menu_link with the new string. When you edit the original german node, it will save the the german title+description a a translation of the english menu item.

This is working well for me, but this definitely needs testing in other configurations...

I have also added elements to the entity_translation form to allow easy translation of menu items (see screenshot).

Putting this on "Needs review" as I would like input from someone who has more experience with i18n, before I put more work into this issue... am I on the right track with this patch?

This one should pass the tests I broke previously...

The idea looks generally good but the main issue with this patch is that it adds a lot of code depending on ET which:

- Is nowhere near a stable release (I wouldn't want an otherwise stable module to depend on an alpha one)

- It is a different module, mainly.

There are other issues too, like translating links that have a language which is not consistent with the rest of i18n/strings feature.

The best thing we could do for now is just adding the minimum number of checks to make sure we keep hands off menu items handled by entity translation. If anyone wants a tighter integration with ET, better start a new module 'i18n_entity_translation' or similar.

About i18n_menu_link_has_entity_translation() I have some concerns about performance, I see too much node loading there, which in D7 is a really 'expensive' operation. Would it be possible to fix it when creating the menu item by adding some hidden 'node type' property? (Both in i18n and in ET since we'll keep hands off node forms managed by ET)

So this 'needs work' means basically needs simplification to the bare minimun needed

Okay, I'll see how I can split things up and move entity_translation stuff to a separate module...

@bforchhammer:

I don't think we need a separate module, I'll be glad to commit any sensible i18n integration code directly onto the main ET codebase (we already have some :). Please follow @Jose's advice and make the patch here the bare minimum the make menu items localizable when they relate to nodes being ET-enabled.

@plach,

If ET had a stable release I wouldn't mind that much :-)

But anyway, if we start mixing up too much of one module's code -on any other of the modules-, maybe we are doing something wrong about how modules and APIs are laid out.

Thanks for your comments!

I have split the previous patch up into a new patch and a separate module.

The new patch is now fairly small. I've added a new api function i18n_menu_link_mode($menu_item), which returns the translation mode for a menu item... by default this falls back to i18n_menu_mode($menu_item['menu_name']), but other modules can change it on a per-menu-item basis. That's essentially everything we need to allow ET integration.

The remaining code, I have decided to put into a separate module for now; I can also post it as a patch for the ET module if that's preferred. The code can be found in this sandbox (see repository).

The issues raised in #30 (such as performance/node_load call) still exist, but are now part of the module instead of patch...

If you don't use RSS, subscribe/follow here: #1433630: Initial release of i18n Entity Translation? / Current status / What needs testing ...a.k.a. "Are we there yet?"

Though the previous patch did too much, I find this one does too little :-)

Also it 'fixes' maybe some other unrelated issues: $item['link_title']. Is that something that needs fixing?

This is a new patch that (though incomplete) is more like I think the patch should do:

- Keep hands off menu items handled by other translation systems (ET)

- (Experimental). Adds $item['options']['node_type'] to node menu items so we don't need a full node loading later for checking to which node type a menu item belongs.

@bforchhammer,
Let me know whether this makes sense for you and also whether we could find something middle way between both patches.

@catch,
Since this basically adds language to ET nodes' menu items but allows editing it later (so it can be set to language neutral and displayed for all languages) Would this work for ET? Or would you prefer not adding language to the item in the first place?

- (Experimental). Adds $item['options']['node_type'] to node menu items so we don't need a full node loading later for checking to which node type a menu item belongs.

I did something similar with i18nmenu_node and it worked well, I think it's the correct way to do this.

@catch,
Since this basically adds language to ET nodes' menu items but allows editing it later (so it can be set to language neutral and displayed for all languages) Would this work for ET? Or would you prefer not adding language to the item in the first place?

I suppose this is for me :)

Well, I think that for ET nodes it would be better not to have the menu item language at all: we can work on making the Menu widget in the node edit form actually multilingual (exploiting i18n_menu) on the ET side.

I talked to reyero on IRC and we came to the conclusion that it would be enough to just set the language of the menu link to LANGUAGE_NONE for nodes with entity translation...

I've applied that to the sandbox module, and it actully works with the current unpatched version of i18n_menu... (see menu-link-language-none branch, commit-diff).

Also it 'fixes' maybe some other unrelated issues: $item['link_title']. Is that something that needs fixing?

I left that in from the original patch; I think it fixes one of these:

Localization was only possible if a link description was set.
String translation info for title field didn't point to the appropriate field_name.

Edit: Potential problem of this approach is that the user cannot choose to have a menu item in only one language. It will always appear for all languages.

Edit: Potential problem of this approach is that the user cannot choose to have a menu item in only one language. It will always appear for all languages.

This is a good point: perhaps the best solution here is just making i18n treat ET node links as they were not node menu items.

My understanding of the current idea is the following:

• By default, store ET related menu links as LANGUAGE_NONE (--> so that they can be localized)
• Allow user to change the language of a menu item, which hides the item in all other languages (--> switch over to menu link translation sets?)

Some thoughts on required changes:

• Node editing: needs to set language to LANGUAGE_NONE for new items, and preserve existing menu link language otherwise.
• Node/Entity translation:
  • If the language of the source node's menu item is NONE: display string localization form for title+description (see existing code in sandbox)
  • Otherwise: display "full" menu item form, which creates an entirely new menu item (in the same translation set as the source node's menu item)
• Menu item editing:
  • Show language selector for ET-based nodes (limited to NONE + entity language?); Jose's patch #37 already does this I think.
  • If language is removed: remove respective menu links in translation set (?)
  • If language is set: add new menu links for existing node translation which have localized menu items (?)

What are your thoughts on the last point, i.e. what to do when the menu item language is changed? Not sure what the ideal use case looks like in this case.

This is a new version of the patch picking ideas from @plach and @bforchhammer comments.

So, for ET nodes (or nodes without language enabled):
- Store menu item as LANGUAGE_NONE
- Allow changing language in menu edit
- Keep that language, if changed, for next node edits.
For these nodes i18n_menu will just initially set language to none and preserve it if changed.

Notes:
- About which languages to allow for the item, I think better ET does some form_alter.
- The other unrelated issues with link_title, etc, please open a separate issue if you want them fixed.

Please, test, review, feedback.

The patch looks good and behaves as expected. The only remark I have is that if an ET-enabled node has as language a non-default one, the menu item original values cannot match it, as they are always considered as inserted in the default language. Not sure how address this or whether it's actually addessable with the current architecture. If there is nothing that can be done on the i18n side about this, I'd say the patch is RTBC.

+++ b/i18n_menu/i18n_menu.module
@@ -544,8 +544,8 @@ function i18n_menu_form_menu_edit_menu_alter(&$form, &$form_state) {
+  // Check whether this item belongs to a node object and it is a supported type

@@ -664,27 +664,52 @@ function i18n_menu_item_get_language($item) {
+      // Set menu language to node language but only if it is a supported node type.
...
+ * Check whether a node type has multilingual support (but not entity translation)

Not sure whether atm i18n strictly follows core coding standards, but in this case there are these issues with the lines above: missing trailing dot and/or not wrapping at column 80.

Last time I began to receive error messages like this:
Warning: array_merge() [function.array-merge]: Argument #2 is not an array in entity_translation_menu_alter() (line 207 of /var/www/sky37.pp.ua/public_html/sites/all/modules/entity_translation/entity_translation.module).
Post it here to inform those of you who implement ET related menu links support at i18n, maybe it is related somehow. More detailed description of this problem:
http://drupal.org/node/1440476

Looks good on my end as well.

+++ b/i18n_menu/i18n_menu.module
@@ -664,27 +664,52 @@ function i18n_menu_item_get_language($item) {
+      // Preserve the menu item language whaterver it is.

Minor typo whaterver

The only remark I have is that if an ET-enabled node has as language a non-default one, the menu item original values cannot match it, as they are always considered as inserted in the default language. Not sure how address this or whether it's actually addessable with the current architecture

The way I tried to address this in one of my earlier patches, is to hook into menu_link_update, and -- in case the entity language does not match the default language -- update/add a string translation for the menu item instead. Similarly, when we edit the translation of an entity and its language does the default language, we can update the original menu item.

The way I tried to address this in one of my earlier patches, is to hook into menu_link_update, and -- in case the entity language does not match the default language -- update/add a string translation for the menu item instead. Similarly, when we edit the translation of an entity and its language does the default language, we can update the original menu item.

Yes, sounds reasonable. I was thinking about something like that too.

Sorry, #44 has nothing to do with i18n and all this work here.

Ok. Committed the patch in #42, fixing that typo and some documentation lines.

So all menu items for ET enabled nodes will default to LANGUAGE_NONE now and that menu item's language is editable by the user.

Thanks everybody.

Thanks!

This will allow us to smoothly support multiple nodes under the same menu item.

Thanks Jose!

I'm working on the entity_translation part at the moment. I'll update #1433630: Initial release of i18n Entity Translation? / Current status / What needs testing ...a.k.a. "Are we there yet?" when I'm done and we can think about whether it should go into the entity_translation module or stay separate.

This doesn't seem to be having any effect. Here are my steps:

1. Ensure that the translation mode for Navigation is "Translate and Localize"
2. Add a "Language neutral" link to it, click Save.
3. Edit the new link -> Translate
4. Click on "Translate" for French. (English is the default.)
5. Change the Title to the French version, save.
6. I now see this in the logs: "Created string menu:item:1723:title for text group menu: English title"
7. Check "Menu" over at /admin/config/regional/translate/i18n_string, click "Refresh".
8. Run "drush cc menu".
9. Load a page that shows the English link; it shows up.
10. Switch to French; the French version of the link is missing.

Am I missing something here? Or are there more UI bits to work on first?

colan: I followed your steps, and for me the french link does show up in the end... Not sure what went wrong for you... Note that the language of the menu item still needs to be neutral in the end; if you set it a a specific language it will only show up in that language.

Btw. I have now updated the sandbox code, please check it out, test and review :)

Confirming colan's experience. I've been attempting to get language switching menus working with Entity Translated nodes for several hours without success. Is it possible for someone to tarball a test source and database snapshot so I/we can confirm our settings. There are so many variables with this configuration it's really difficult to determine what might be causing the set-up to fail. If you're interested a detailed break-down of my installation and the steps taken can be found here. I've tried this with both the stable and dev releases of the i18n module without success. Am I correct in assuming that the current dev package contains current patches as described in this issue?

Alternatively - I'd be happy to send one of the devs a tar-balled version of my test install.

Thanks.

I tried again after upgrading everything. (Drupal core is now the latest stable. ET is the latest dev. i18n is the latest dev.) Still nothing. I've had to resort to having separate menu items for French & English, and I can't use relative URLs like "node/123". I have to use absolute URLs to trick the system into thinking these are external links. It looks like only one language can claim each of the internal ones. It appears to be the same problem whether it's the same menu item or two different ones. Is there a way to lift this restriction?

For reference, there is a problem with refreshing menu strings, see #1444492: Menu string refresh deletes all strings.

I finally discovered that #1398770: Menu item localizations to entity-translated nodes disappear with Multilingual Select module (Entity translation compatibility) was my problem. This stuff now works for me. Huzzah!

I have followed this issue with great interest (and confusion) but I am hoping to try to get some help with testing the patch in #42 above. I am using the Entity Translation Module and as I undertand it there's a problem creating menus in multiple languages because Entity Translation only has one node for many languages, that's actually one of the reasons I chose to go that route as I wanted low overhead for nodes and so far it works great except for multi-lingual menus

1. I "checked" the patch against the latest dev of i18n (2012-Feb-19), is that the correct one to test with? I got this message:
   

   git apply --check 1182058_i18n_menu_et_support.patcherror: patch failed: i18n_menu/i18n_menu.module:544
   error: i18n_menu/i18n_menu.module: patch does not apply

2. Also the steps in #52 above, does that assume that you've run the patch successfully?
3. and if so, what does this mean from #53?
   

   Note that the language of the menu item still needs to be neutral in the end

   -- is that different than "Translate and Localize"?

Thanks

Jose Reyero: Ok. Committed the patch in #42, fixing that typo and some documentation lines.

The code is already in the dev branch, no need to apply patches anymore.

The steps above should work as long as you don't refresh the menu strings in step 7 (see #1444492: Menu string refresh deletes all strings), and don't have the multilingual select module enabled (see #1398770: Menu item localizations to entity-translated nodes disappear with Multilingual Select module (Entity translation compatibility)).

The language mode of the menu needs to be set to Translate and Localize; The language of the actual menu item needs to language neutral (which it is already unless you adjusted it manually).

@bforchhammer -- thanks, worked like a charm! I have that working now, this really goes well with Entity Translation in keeping with the "one for many" idea. One of the keys was:

the actual menu item needs to language neutral

-- I actually had to delete my existing menu items as that setting was greyed out but it was no big deal, in the end it was worth it.

OK, have repeated all of those steps above (excluding #7). Deleted menus, recreated menus and still nothing. I get the same menu text (Primary: EN) for both language options. it would be really helpful if someone can link/upload a test harness database with a working example of this. I'm wondering if there is some other conflict causing this not to work. Had PathAuto on but now disabled that.

@mattp52 -- Did you check to make sure you have the multilingual select module disabled? If I have some time, I can try to post a snapshot of my site but I don't see that happening until later next week, I'm buried with work right now.

@highrock -- Yes I've disabled "Multilingual Select" but to no effect. Am running a 7.12 base with 7.x-1.4+3-dev i18n modules. Have observed behaviour with, and without, the hack code posted here to force URL switching on Entity translated nodes. I've attached a csv db report of all current enabled modules in my test site.

If you could fire through a snapshot that'd be really useful. Next week'd be fine, thanks.

Clarifying title for easier searching.

Sorry to reopen this issue, but I 'm in the same situation as mattp52 in #63, so I think something is definitely broken here. I don't see why the issue has been closed in the first place anyway. If you want me to create a separate issue, please let me know.

- Very simple site with French as default language, English enabled,
- latest dev version of i18n and ET.
- i18n mutilanguage selection module is disabled
- disabled the ET managed node's menu creation
- manually created a menu entry leading to French version of the node
- the string does appear in the string translation interface, it is translated, but the french string always appears in the menu, whatever the current node's language.

Hint : on admin/config/regional/translate/translate, the language code is stroke through (fr), although a translation has been saved.

Hint 2 : I tried to translate the site's slogan with the i18n variables module, and the behaviour is similar : string translated according to the interface, but never appears.

@Counterzo,

From 'Hint 2' it seems there's something wrong with your setup / languages / etc, looks like the interface is not switching language in the first place, thus it is a different issue.

(It may be 'Admin language', ET node pages, etc... Please check)

Enabling interface translation worked. I willfully disabled it to be sure it didn't interfere with the process. I never saw menu translation was admin interface related in the various docs.

Anyway, thanks a lot.

@Countzero,

At this point, i18n's menu translation for ET nodes == interface translation

42: 1182058_i18n_menu_et_support.patch queued for re-testing.