From a discussion with miro_dietiker:
Currently to feature a node translation set in a menu you have to (a) create a menu item for each node in the translation set, and (b) add all those items to a menu. This approach makes menu administration difficult. Say you have ten nodes you want in a menu, each of which is translated into 10 languages. Currently this means adding 100 menu items to the menu. Now you want to move content X to the 2nd spot in the menu. Settiong menu item weight is very difficult, particularly as the admin may not speak many of the site languages and so have no way of determining the relationship among a translation set.
Potential approaches:
1. Somehow link the different menu items that form a translation set, so they can e.g. be dragged together.
2. Have only one menu item per translation set, but localize it.
Approach 2. *might* be possible by altering i18nmenu_menu_link_alter() and/or i18nmenu_translated_menu_link_alter(). There we alter custom menu items.
i18nmenu_menu_link_alter() currently sets a flag to have the menu item sent for altering only if there is no language set.
So far in i18nmenu_translated_menu_link_alter() all we do is localize the title. But we could try a test to see if this is a node link and do our swapping there.
This whole approach would be tricky and potentially error prone as we would need to test menu item access to translated items etc. Still, it's probably worth considering.
Comment | File | Size | Author |
---|---|---|---|
#54 | i18n-386372-54.patch | 900 bytes | plach |
#48 | i18n-386372-48.patch | 832 bytes | plach |
#43 | i18n-386372-43.patch | 2.77 KB | plach |
#39 | i18n-386372-39.patch | 21.19 KB | plach |
#30 | i18n-386372-30.patch | 18.9 KB | plach |
Comments
Comment #1
brucepearson CreditAttribution: brucepearson commentedFor 1) shouldn't this be part of the i18nsync module. Could the i18nsync module keep all the menu weights the same for menus on translated nodes as the menu on the original node. The i18nsync module could also keep the parent menus in sync so that the whole menu structure for the translated content is the same as for the original content.
There would need to be a setting somewhere so this could be enabled or disabled as there would probably be situations where you want the menu structure different in other languages.
We do something similar to this in a module we are developing for assisting in translations. http://drupal.org/project/icanlocalize
Comment #2
miro_dietikerThanks for your submission, nedjo. This is right one of the things that irritated me (and doesn't look like correct localization if you know the ui localization workflows.. you much more ask "where can i enter the localized text for this menu entry?" instead of adding it a second time).
The whole menu handling in a bit strange. Menu points disappear (in language A) if they refer to nodes of language B. That's fine. But why are those language node properties not getting propagated to the menu item? (the menu item shows still "all language" when editing properties..)
And finally i don't understand how the language setting per menu are getting used at all. No matter what i choose there a menu item is still visible in different languages (even if the menu item refers to e.g. a module page). Why is there no column for language settings in menu item overview such as "manage content" has? I'd expect these two mechanisms to work identically in general - but they don't.
If we'd have a single (and much more simple) workflow of how things are getting localized, one might understand the whole picture much sooner.
Comment #4
mani.atico CreditAttribution: mani.atico commentedAt this moment I'm working in a module to synchronize menus. Hope I get it soon.
Comment #5
yang_yi_cn CreditAttribution: yang_yi_cn commentedsubscribe
Comment #6
yang_yi_cn CreditAttribution: yang_yi_cn commentedI wrote a tiny module to achieve the goal. Basically it implements a menu call back "localized/node/%", and you can put that in the menu path. When menu item is built but before rendering I use another hook to find the translation source and the translated copy in current language, then alter the href.
It works fine for me, but i think it's too small to put into another project, maybe someone can test it, or even integrate to the i18n module?
Comment #7
brucepearson CreditAttribution: brucepearson commentedWe have just added a similar feature to our module: http://drupal.org/project/icanlocalize
This relies on setting up a menu for each language according to http://drupal.org/node/313302
When this is done, our module will then keep any translated menus in sync with any changes to the original language menu. It will keep the menu order, hierarchy, enabled state and expanded state in sync. It includes options for selecting what to sync when you save changes to the original language menu.
Comment #8
mani.atico CreditAttribution: mani.atico commentedHi,
Managed to write a module to enable menu translations synchronization. It's roughly done and needs lots of testing and reviewing. I'll try to clean up the code, but first I want to check the module mentioned in #7 and see if it is worth working in this module. At the same time TESTING is needed. PLEASE TRY IT OUT and give me your feedback.
What's missing:
No tests have been made with three or more languages, although the code is written to support them.
Add 'translate' links inside the menu page and a language filter as well.
Remove the translated menu from the parent select box.
How to use:
admin/build/menu/i18nsettings: select which items to synchronize.
admin/build/menu/item/{menu item id}/translate: menu item translation table
Menu items assigned through node edit/add form are synchronized with the menus belonging to the node's translations.
There are lot of issues and stuff to mention about this module. Please review this approach.
Comment #9
yhager CreditAttribution: yhager commentedI am attaching a patch that implements option (2) from the original post.
Assume you have node/1 in en, and node/2 is its translation to es. If you have a menu item pointing at 'node/1' - when you view the site in es, it will switch to 'node/2'.
This is different from the above sync approaches, by using single menu item, and just modifying it as needed.
Comment #10
plachThe attached patch implements #2 from the OP.
It provides a checkbox for each menu that allows to enable node translation selectively. By default it's unchecked to preserve BC.
The approach implemented is the following: on menu item save if we have a node link its translation set is saved among the item link options. On menu rendering the options are checked to see if there is a translation available in the translation set for the current language. If a suitable translation is found its nid is replaced in the item's href.
Serveral checks are implemented to ensure that the menu item's translation cache is always up-to-date.
Comment #11
plachthe patch :)
Comment #12
plachThis is a version for i18n 6.x-1.4
Comment #13
plachAttached you can find a revised version (dev and 1.4) which addresses a couple of issues with the menu form item in the node form and with access control.
@Jose Reyero:
Please, let me know if you are interested in including this feature in i18nmenu as it can easily moved to a dedicated project otherwise :)
Comment #14
miro_dietikerplach i'll review your suggestions asap.
meanwhile i'm also i18n co-maintainer and try to push such improvements whenever they make sense and don't conflict with special use-cases. (i need to check your solution in deep detail on my own to be sure)
if jose takes the ball i'd also be fine for sure!
Comment #15
plach@miro_dietiker:
Sure, take all the time you need. I was just wishing to know if this had any chance to get into i18n (possibly in the next release :)).
Comment #16
miro_dietikerI've skimmed through the code.
Found inputs like:
Then some more questions like
This code contains some complexity and some hook implementations are not documented yet.
Comment #17
Jose Reyero CreditAttribution: Jose Reyero commentedAbout this, the idea is ok though the code looks a bit complex to me. To add to the questions above:
- Are we actually enforcing node access for all translations?
- Wouldn't some options like syncing module weights make more sense in i18nsync?. Would it work for translations on different menus?
- I'd like to explore other options, like icanlocalize solution (#1), they're publishing some new modules atm, so before adding any new feature let's see whether there's already one there,
- It is a bit restrictive if it requires setting language negotiation to 'All content'. I'd rather have a global option that would also disable language conditions for all menu queries (as it impacts all menus it should be a global option).
- Some explanation text would be good for the node's menu options like 'This will apply to all the translations too'
For next release, that will be ASAP, we are focusing on some important bug fixes, so no new features for that one. See #795038: Putting out a new 6.x-1.5 release.
Comment #18
Jose Reyero CreditAttribution: Jose Reyero commentedOne more thing, it would be great if the menu item was the 'translation set' instead of the node, so we could also support a common child item for all languages like in this case, #810952: Workaround menu item duplication for translated node
Comment #19
ndm CreditAttribution: ndm commentedI have tried the patch from #13, it works for me.
My comments :
-i don't like to have the obligation to set multilingual without negociation.
Have you a solution to guard content selection?
-i really appreciate the system which disabled the node menu system in translated content.
Great patch which must be integrate in the i18n core.
Comment #20
plachIn the attached patch I tried to implement the suggestion from #16-#18 to use the tnid in the menu item data structure. To address #18 and more generally support active trails the patch introduces a hacky technique borrowed from Menu Trails (see
i18nmenu_nodeapi($node, 'view')
).@miro_dietiker:
Yes, the idea is that when saving a menu link item we cache all of its translations so we don't need db queries when actually displaying translated menu items in
i18nmenu_translated_menu_link_alter()
.I don't get what you mean:
$key
is never used in the query. Renamed it to$field
, anyway.Ok for the type check but additional parameters are supported through the following
implode()
call, this way we can support paths like'node/1/edit'
.When we have language-specific menu items node translation simply does not kick in.
The menu item gets deleted too and it does not appear in the menu anymore.
Yes. The item title translation keeps happening through i18nstring.
This cannot happen: the menu form items is not shown for node translations to avoid creating additional menu items.
Mainly to support backward compatibility. Moreover one might wish to keep using the title-translation-only approach.
@Jose Reyero:
Will you expand on this, please? I'll gladly streamline code if you provide me some guidance.
Not exactly: we cannot afford running a complete node access check on every translation. The source translation (which is the node the menu item actually points to) gets a
node_access('view')
-like check in http://api.drupal.org/api/function/menu_tree_check_access/6. The remaining translations get only a check on their status (seei18nmenu_node_translation()
for details).Sorry, I can't understand these questions. Would you clarify please?
We started developing this solution because in the project we are working right now (a large site fully translated in at least 5 languages) the client simply found unacceptable having to replicate the menu items for each language: the menu admin interface becomes unbearably cumbersome and unusable. Hence we are forced to go on with this solution, to have something ready for our deadline. I converted this patch in a standalone module we deployed on the site, however we will keep supporting this patch and we will remove the module if this gets committed.
The problem is with the check in http://api.drupal.org/api/function/menu_tree_check_access/6 : all the other options make the menu items disappear for some languages. I'm not sure how to limit
i18n_db_rewrite_sql()
to act only on non-menu queries: do you have any concrete idea to suggest?Comment #21
miro_dietikerPlach, sounds reasonable and still a way to go after having everything clarified. Thank you for all the updates.
For the menu entry translation:
So it doesn't seem possible to have the menu title editable for current language in the ui.
This means translation still occurs simply via locale interface only (and l10n_client)
Now here's another challenge:
I really can imagine that a page has a footer that needs tnid translation while the same page has a main menu which gets replaced per language. I've seen pages that allow to change language to different main pages via a main menu - not being the primary language switcher. Thus the tnid system needs to be inactivated for those primary menus (or entries).
I'm still not sure about my opinion on how to switch on/off this feature.
I might be more lucky if we could enable this function per menu item (such as a per menu item checkbox - "switch to translation from tnid if available") instead of the global per-menu option. Only if enabled the nid gets permuted with tnid.
This checkbox could also be changed in the node-edit-form as same as in the generic menu system edit item form.
Since node access is dependent the current resulting menu (depending user, languge, ...) - wouldn't it be better to provide a rendered menu object per language (which is access proof) instead of creating a monster amount that is not cleanly checked?
Comment #22
plachOn node translations instead of hiding the menu widget, we could replace it with a "menu title" text field which would pass the string to tt().
This would put more burden on the user's shoulders, unless perhaps the current per-menu checkbox acted only as a default value for the proposed per-item checkboxes.
I ain't sure about what you are proposing here: would you rephrase, please?
Comment #23
miro_dietikerI would always enable the menu translation system by default (checkbox prechecked) if current node type has enabled translation. Well you're right. one option more all over...
Then having a translated menu title would be very very nice to the user!
We can ignore the last caching and node-access related paragraph.
Jose - what's your preferred level of configurability?
Comment #24
plachI'm worried about backward compatibility: IMO the default menu status should be FALSE at least on updating. For fresh installs we might want to switch to TRUE as default based on the content type multilingual settings.
Comment #25
plachThe attached patch implements #22-#23.
Now node translation is by default set to TRUE on new installations. To preserve backward compatibility the patch introduces an update function explictly setting node translation off by default for existing sites.
I tried to address also the content negotiation issue by making i18nmenu replicate menu blocks and temporary disable content negotiation while rendering the menu block.
Comment #26
plachComment #27
INAScon CreditAttribution: INAScon commentedTesting your patch #25 and wondering how this should work.
I have a node/1 in English and its translation node/2 in Dutch. Furthermore, I have a menu item, set to all languages, with the title Help linking to node/1. If I edit node/2, I see the translation in the menu section of the edit form. So that looks fine. If I switch to Dutch with the language switcher however, the menu item dissappears from the menu.
I guess I'm doing something wrong here. Can you please help so I can test drive your great efforts on this issue.
Comment #28
plach@narends:
You need to enable the
[i18n]
version of the menu block (you will find a "i18n" version of each menu block in the block configuration page). Otherwise you will need to set content language negotiation off.Comment #29
plachThe update function was wrong: the attached patch should fix it.
Comment #30
plachSorry, just found a hunk in the previous patch which performs an unintended change.
Comment #31
plachComment #32
stoltoguzzi CreditAttribution: stoltoguzzi commentedis this patch agains the latest dev-version?
Comment #33
plach@stoltoguzzi:
yes, it should
Comment #34
NicoH CreditAttribution: NicoH commentedHey I've tested the patch and I found a problem.
I've made some menu items in the menu 'primary links'. Then I've translated the items from german to english. So far so good..
If I switch the language now, the translated menu items(english) won't appear.. only if I enable the 'i18n primary links block' or if I go under /admin/build/menu it will be correctly displayed
Comment #35
plach@NicoH:
see #27-#28
Comment #36
TimeFor CreditAttribution: TimeFor commentedCan't seem to apply the patch from the directory. Am I missing something basic?
Comment #37
miro_dietikerI've tested the whole complexity possible (all cases to be expected) with i18nmenu and found many issues with the current patch.
In node edit form
- only show in menu fieldset "Enable translation" if nodetype is translateable. also check nodetype settings in other cases.
In node edit form of a translated node (with enabled node translation)
- title changes to the translated menu item don't update it.
In a node edit form if a translated node (with disabled node translation in source node)
- "Enable translations" is present and does strange things. Note this should either be removed or it should act as tnid node menu translation enable.
In admin/build/menu when editing a menu item
- "Enable node translation" is always unchecked and won't get saved.
If we enable node translation in a node edit (source)
- We should check all translations and remove (disable?) their menu entries
When disabling menu translation and editing a translated node
- node edit still shows menu
I don't understand why we switch off the i18n mode in hook_block. I see no reason to do that (unconditionally).
Note that this is no solution - as NicoH told - since the regular page.tpl.php variable also need to be i18nified. in preprocess_page we're even overriding this by i18nmenu. Currently $primary_links is broken regarding your implementation.
Then there are some questions
- Will the menu item language be ignored? (should we try to remove language selector here?)
- In menu item edit: should we check for path pattern to "Enable translation" checkbox visibility? (might be done in jquery client-side)
- If disabled node translation the menu system still does strange things. Are we able to simplify something here?
I expect there will be some "details" more after fixing these things. So the module needs another deep review after.
Comment #38
Jose Reyero CreditAttribution: Jose Reyero commentedI can see there's a lot of work on this issue and also the feature is a desirable one. But really, this adds an incredible amount of complexity to the (already quite a big hack) i18nmenu.
Also it still has a lot of issues and maybe we should be focusing our energy on Drupal 7 ports better than adding another really difficult to port feature to 6.x
My main question is: can we trim it down and make it simpler? Some ideas for that would be:
- Just global options, no more per menu options.
- Do it on the synchronization layer. I mean we just store nodes/menu items as usual, then after a node/menu item update, we sync all the translation set menu items, no options, no crap it should be "all or none" the same weight, location in the tree (though it it's below another node we should find a suitable parent).
- Then, if we still have one item per node, there's a remaining issue with the menu admin page. I'm thinking we can just apply 'language selection' to that page so you only see nodes for the current language there (currently it is disabled for admin pages). As items for nodes will be 'synchronized' after updating you really just need to edit items in one of the languages.
@plach,
Does this make sense / would still be useful for your user story? Btw, I've added you to the commit list for the module in case you want to commit some quick bug fix.
So as said before the patch on its current approach doesn't make too much sense on my (personal) cost-benefit analysis. However if any of you guys (@plach, @miro_dietiker) want to commit to maintaining the i18nmenu module with this patch on it, you are really welcomed to move on with this and eventually commit it.
Comment #39
plach@miro_dietiker:
The attached patch should fix the issues I was able to identify when translation support is disabled.
I could not reproduce this issue: would you please investigate further and provide a more detailed path to reproduce the bug?
This happens only for non-node items and is an intended behavior: it makes no sense to enable node translation on non-node items.
I ain't sure this is a good idea as IMO node translation should have less priority than the item language: when one explicitly assigns a language to a menu item, node translation has no meaning. Instead we could hide the "Enable node translation" checkbox when an item has a language assigned (I implemented this in the attached patch).
This is by design too. If one disables node translation the usual behavior is restored: a node translation form currently presents the menu item element prepopulated with a copy of the source node's menu item.
Node translation really does not play well with language selection: any mode different from "off" causes the node menu items to be messed up. IMO menu items should be subject only to their language setting, if specified, otherwise they should be always visible (possibly translated). Please keep in mind that language selection is restored immediately after rendering the menu tree so we should not lose any capability associated with it.
You are right. The attached patch should fix this (see
i18nmenu_theme_registry_alter()
andi18nmenu_navigation_links_prepare()
).This surely could improve usability, I'll put it on my TODO list.
I really need some detail to answer this.
@Jose Reyero:
Unfortunately I cannot afford to spend the time needed to implent a completely new approach. Moreover I'm actively using the standalone-module version of this patch in a couple of project and changing approach would mean more testing and training people again. Hence, I'm sorry, but the answer is no.
Thanks Jose, very appreciated :)
First a preamble: since my only viable alternative to getting this into i18nmenu is creating a new project I'd have no problem to become a maintainer of i18nmenu.
However, given that Jose seems inclined to explore different approaches and Miro and I seem not to be on the same line about what should happen when enabling node translation on existing sites and more generally on backward compatibilty, perhaps we should actually release this as a standalone project. There should be no problem in doing this on the technical side and in this scenario we could be sure that the users, by enabling the module, really want node translation on their sites.
Moreover an eye on the future: I think menu node item translation will be far less useful when field translation will be available in Drupal 7, as every node will be available in multiple languages, so this solution would make porting i18nmenu to 7.x easier.
Comment #40
vivianspencer CreditAttribution: vivianspencer commentedsubscribe
Comment #41
Jose Reyero CreditAttribution: Jose Reyero commented@plach,
Ok then, I think the best solution is to make this a different module. I'll add a link from i18n project page.
I'm assuming it will work with i18nmenu (right?) instead of being a replacement for it. If so and there is any change to i18nmenu needed to make the integration simpler, we can work on that
Comment #42
plach@Jose:
Yes, the current standalone versione depends on i18nmenu. I'll publish it ASAP.
@Miro:
If you wish to keep working with me on it you're welcome :)
Comment #43
plachHere it is: http://drupal.org/project/i18nmenu_node.
As Jose noted in #41 some minor changes can be retained from the latest patch.
Comment #44
Jose Reyero CreditAttribution: Jose Reyero commentedOk,
Committed the small patch in #43 (needed to apply manually)
Added link to the new module to the project page.
So it seems we are all set for now, follow up here: http://drupal.org/project/i18nmenu_node.
Comment #45
plachThanks Jose!
Comment #46
miro_dietikerplach, fine to see this.
I'll review the module. Note that most of my questions where to get clarification in intention and provide some thoughts. So looking forward to support your progress.
Now that this functionality is separated completely we should about making i18nmenu base code more straight. Is there some simplification possible? (e.g. don't consider menu item node case ever)
Comment #47
plachAFAICT there is not much node-specific code in i18nmenu, just
i18nmenu_form_alter()
andi18nmenu_nodeapi()
and removing them IMO would seriously compromise i18nmenu's functionality.Comment #48
plachA possible minor optimization.
Comment #49
miro_dietikerplach - what does this fix change in functionality? (what kind of issue does it fix?)
Comment #50
plachIt does not change anything: simply only the needed hook is invoked to load the menu data structure into the $node object instead of all the hooks implementing hook_nodeapi('prepare').
Comment #51
miro_dietikerSetting back to active as we need some further optimization to optimally interfer with this module.
If the module about node translation set is missing on a site, we still should optimize the situation.
If i add a node (which has a language) to the menu, it is being added for all languages.
Why does i18ncontent not sync the language into the menu item (also on node updates)?
I'd consider it a very special case that my node from language A in a menu would needed to be shown in language B.
Is this case so common?
This might affect the i18nmenu_node somehow.
Comment #52
plachI ain't sure about this: if a node menu item has a language assigned i18nmenu_node does not kick in.
Comment #53
miro_dietikerplach - cool.
It turns out that my i18n menu does it perfectly right now. Seems to be some edge case of e.g. changing languages in the node.
UPDATE: We need to check if menu item language is also updated if content language is changed.
It could be this (missing menu item language update) was the source of most of our issues.
Then there's a question to the translation system.
Should we avoid passing menu items with assigned languages to the translation layer? If a menu item is language specific it doesn't make any sense to pass it. We even might remove the translations if we change the language into neutral.
Comment #54
plachAt the moment node language is not taken into account (but is preserved) if a node has already a menu item associated or has no translations, hence node language matters only when creating a node translation. The item language has to be changed manually. Supporting language change would mean changing the current behavior.
It's already so: in
i18nmenu_menu_link_alter()
the menu item is marked to be altered only if it has a language associated. Hence if the menu item has a language associatedi18nmenu_translated_menu_link_alter()
is not invoked.Do you mean removing the string translations? I ain't sure this is a great optimization.
By the way, I found that manually applying #43 introduced a small bug fixed by the attached patch. I committed it to the DRUPAL-6--1 branch.
Comment #55
miro_dietikerI'll check some workflows in adding nodes and switching languages and adding menu items to understand if some more updates are needed in i18nmenu from my side.
Comment #56
plachSee #967688-9: Introduce an API function for removing a string translation.
Comment #57
JonMcL CreditAttribution: JonMcL commentedHello,
Just came across this thread and I'm curious if any progress has been made since 2010??
I have been hoping for functionality like this for a while. I will try patches, but if this has gone somewhere else, I would appreciate a heads-up.
Comment #58
plachThis ended up in a separate module: http://drupal.org/project/i18nmenu_node
Comment #59
plachComment #60
JonMcL CreditAttribution: JonMcL commentedBut no Drupal 7 love, huh?
Unfortunately Entity Translation hasn't worked out well enough for me in it's current state so I'm stuck with Content Translation.
Thank you for the effort though!!
Comment #62
bradjones1Changing this to 6.x since the module that came out of this discussion has no D7 port. :-(Comment #63
bradjones1I apologize for my misbehavior immediately above - for dense minds like mine: See i18n_menu_node
Comment #64
drzraf CreditAttribution: drzraf commentedany hope the separated (must-have) feature provided by i18nmenu_node being merge into i18n_menu ?
Comment #65
Jose Reyero CreditAttribution: Jose Reyero commentedPlease don't reopen old closed issues, add a new one with a proper updated description. And also explain why it is a problem having the feature in a different module.