Untranslated menu items are displayed in menus

Having a menu with mixed languages is never desired for site builders / end users

I would say this is mostly not desired, but never is not really possible to say about a feature. Say your menu contains IKEA product names, they are not translated anywhere. ÄPPLARÖ, BEKANT, etc. will be the same regardless of site language. Even if you don't translate the items themselves, they may still lead to translated pages with product information in different languages.

As for how to approach this, menu items in core at least come from one of 3 places:

• Menu items defined in core code. *.links.menu.yml
• Menu items defined from views
• Menu items defined as a menu link entity (either on the menu UI or on the node edit form)

The last one is the only case where we can explicitly tell whether the item has been translated to a language by looking at translation languages for that item. The items defined in views are translated in config translation. To be able to tell if you encountered the translated one or not, you need to reach back to loading the config translation for the view and see if the menu had a different textual value for the item. For *.links.menu.yml items, those are translated with the locale system and they may or may not have a translation there. Eg. 'Home', 'Create content', etc.

Edit: also note that contrib may source menu items from any other place, the system is pluggable. So each plugin need to decide for itself if the item is "translated" or not.

In the case of IKEA product names, I would think that the menu items should use one of the no language options, which we do want to show untranslated.

Yeah so long as those are menu link entities as well, which are the only ones that have a language defined. Views or links.yml sourced items don't.

It should only be the menu items, which has a defined language that should be hidden if not translated. This only applies to menu link entities I believe, which makes this quite tricky, since menu items can come from a host of different places.

If you limit this to menu link entities (those are the only ones that have a defined language), then its definitely simpler. I would argue that people who define a page from views will have similar expectations about not showing if lack of translations and if they have a setting to not show untranslated items, then they will consider it a bug if only some items are hidden.

I've tested this on Drupal 8.1.0 RC1 and seems like menu items are still present no matter if there exist a translation of the node it links to or not.

Is this not a major bug? I would say it is a really big drawback for multilingual site builders! Is there a work-around for this?

Also, when I try to translate the menu item in the node edit form it seems to be changed for all language (the Menu link title label seems to be not-translatable). I can also not find the menu items to be translated under configuration translation.

To me to seems like core functionalities for menu translation is not working.

This issue also affected us today. In our case, the menu items were translated, but the translations were not marked as published. Drupal chose to show the only published translation. This one goes into my D8 Gotcha list.. :)

wow, thanks so much for that info, I've been struggling with the exact same issue myself, trying to understand why different menu items are displayed in different languages; some of them are published and not!

I think this could be pushed forward by at bare minimum exposing the menu link language to hook_menu_preprocess. For example adding something like:

  public function getLangcode() {
    return $this->getEntity()->language()->getId();
  }

in core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php. This is obviously a crude way to do it (and only works for the Custom menu link?), but allows for some control over the rendering of (un)translated menu items.

A similar approach would work for other menu items, if a standard way of exposing the language property would be implemented across the board. For the menu listing view also, exposing a filter for the language would be a nice to have feature.

I agree with arskiainen on this. If the language of each menu link would be exposed on the hook_menu_preprocess we would have more control over the menu-items. This would solve issues I am having with the menu-items quite easily.

I cannot find any occurrence of hook_menu_preprocess(). Did you mean hook_preprocess_menu()? If so, I'd be wary about solving this problem in the theme layer.

I'd like to propose the introduction of a new PHP interface for menu link plugins, to expose their multilingual capabilities. Because the interface is optional (at least until Drupal 9), we will not break backwards compatibility. We can include an example implementation for links coming from menu link content entities, and create follow-up issues to implement similar functionality for menu links coming from other sources, such as Views.

We have a use case with 15+ languages
We don't want to manage every menu separately for each language, but do want to allow some languages to have content in the menu that is not yet translated elsewhere.

Looks like the content language access module solves this issue for us.

#14 worked for me. Thank you friend.

Note: Only works for internal paths.

Is there any workaround to this problem? Translation system with menus is unusable at the moment if every node has not a translation.

I tested #14 but I think it is not a solution when using node translation (same node id for original node and translations).

I think every node translation should have unique menu item with language code. Then menu items could be easily filtered by language.

Is there any workaround to this problem?

You can try https://www.drupal.org/sandbox/tuutti/2832329

It's not perfect, but has been enough for our needs so far.

@tuutti, Nice to see someone else working on workaround for this.
I'm also work on this, but we use different approach than overriding menu_tree manipulator service. We use hook_block_view_BASE_BLOCK_ID_alter() to filter menu items + add settings per block instance for multilingual things. Also we do override of MenuTreeStorage service to redefine plugin class for menu plugin to add getLanguage method.

Also I think that @arskiainen is right about getLanguage() method, or maybe change getEntity() method to public.

I will ask @matsbla to share this module.

Thank you @tuutti for this very timely lifesaver of a module! It works as advertised, only thing that stumped me for a bit is that it's not a drop-in replacement in the form that you current menu blocks will start working instantly after enabling the module, you need to place the menu block (of the new type) again. That is however fine as long as you know of it.

As for this entire issue I agree that this is quite a regression from d7 both for site builders and content producers. I've built numerous multi language Drupal sites and can't remember that the default behaviour in d8 core now is something I have ever wanted (not saying it *could* never happen). For site buildes it's a pain to maintain, place, configure and possible style one menu per language, and for the content producers they will not only need to translate the menu link, they also need to remember to switch to the new menu it should belong to. They will forget, again and again.

@vegardjo: well, the goal with core was first and foremost to solve storing translations and bring contribs on the same page that ANYTHING on the system may be translated. The idea was/is that if we succeed with that then contribs can interoperate on top of a fully multilingual system which is going to be the biggest win for people. We could have focused on solving specific scenarios for multilingual sites, and miss some of the fundamental elements that are shared. Instead we focused on the fundamentals. I don't think this is a regression from Drupal 7 since Drupal 7 did not support menu translation at all. So that if you need to install a module to configure how the translated menu items are displayed that would be less of a hassle compared to what you needed to do in Drupal 7 and therefore still an improvement. Also once again, critically all other modules would be achieved that the menu is/may be translated unlike in Drupal 7 where i18n side-loaded the menu translations in some specific cases only.

You're right Gábor, it's wrong calling this a regression since d7 core offered little here. My bad. I am very aware of the huge improvements to all things language in d8.

That said, this still stands out like a major bug to me, and something that it seems to me should be handled by core. If I'm not very wrong everything else is always filtered by the current language in a multi language setup: all content, all strings, all blocks have a setting for it, views in core has a built in filter for it, and in general everything you see in the UI has this, just simply *not* the menu links, where the default behavior is plainly showing the original menu item for all languages, if not specifically translated. I find it very hard to think of a use case where this is what you actually want, and for me this isn't a very specific scenario, it's pretty much every scenario.

But to move forward, since you have the bigger picture: do you feel that this thing is something that is out of scope for core to solve? If so that's ok, and we'll just have make a note of it and wait for contrib to solve it.

I'd like to propose the introduction of a new PHP interface for menu link plugins, to expose their multilingual capabilities.

Even if we decide not to include the actual filtering in core, this would help tremendously on implementing this in contrib land.

The patch contains a rough example of how this could be implemented for MenuLinkContent and how links could be filtered on a menu block level.

1. +++ b/core/lib/Drupal/Core/Menu/LanguageMenuLinkManipulator.php
   @@ -0,0 +1,53 @@
   +  public function filterLanguage(array $tree) {
   +    $current_language = $this->languageManager->getCurrentLanguage()->getId();
   +
   +    foreach ($tree as $key => $link) {
   +      if (!$link->link instanceof MenuLinkTranslationInterface) {
   +        continue;
   +      }
   +      // Hide menu links that does not have translation for current language.
   +      if (!$link->link->hasTranslation($current_language)) {
   +        unset($tree[$key]);
   +      }
   +    }
   +    return $tree;
   +  }
   

   See the reason we are not doing this is that its not anywhere near clear that without making this configurable, this is something we should hardcode. If you want to put product/brand names in a menu (eg. you are selling watches or car tires), then those not showing up in the menu under certain languages pages is a bug. If you want to put the maintainers of a site section in a menu (users), not showing those because usernames are not translated would be a bug.

   So this points to the key reason at why we are not doing anything yet in core. We would need to figure out the ways core wants to support AND make it configurable so in cases where its a feature rather than a bug would be covered.

   It is true we are hiding content for things that are not translated in node views for example, but views are also configurable, so anyone can change the behavior.

2. +++ b/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php
   @@ -237,6 +238,16 @@ public function isTranslatable() {
   +    if (!$this->isTranslatable()) {
   +      return TRUE;
   +    }
   

   This would not satisfy the definition of the interface just how you personally intended to use it?

The name of hasTranslation() does not really reflect what I intended it to do, but the idea behind hasTranslation() is that it should determine whether the menu link should be visible in the current language context.

For example Drupal\menu_link_content\Plugin\Menu\MenuLinkContent should be visible only if:

1) Menu link has translation for the current language
2) Menu link language is either 'Not specified' or 'Not applicable' (thus the isTranslatable() check)

and Drupal\views\Plugin\Menu\ViewsMenuLink should be visible if:

1) View has configuration override for given language (translation)
2) Some configuration tells to do otherwise (maybe Rendering Language setting?)

Other menu link types would obviously require an additional configuration, but the general idea is that we have an interface that determines if the menu link type is translatable and a method that determines if menu link has a translation and whether it should be visible or not.

For example Drupal\menu_link_content\Plugin\Menu\MenuLinkContent should be visible only if:

1) Menu link has translation for the current language
2) Menu link language is either 'Not specified' or 'Not applicable' (thus the isTranslatable() check)

I don't think this is quite that simple. I knew clients where they did not have resources to translate everything 100% but wanted to display those parts of the site untranslated, so the experience is just degraded not cut off entirely. IMHO whatever solution we have in core needs to be configurable, which is primarily the reason we did not put any menu translation display solution in core, because it needs more consideration. If we put something in that is hardcoded, that still would be a regression compared to i18n in D7 which was configurable.

Hi, there.
I want share another solution for filtering the menu links using more filters.
https://www.drupal.org/sandbox/matsbla/2831709

provides multilingual features for menu blocks, to filter out menu items that do not have translated labels or link to untranslated content.

Tried both https://www.drupal.org/sandbox/tuutti/2832329 and https://www.drupal.org/sandbox/matsbla/2831709 but the firs is much better because the menu is clean also when logged in. Pretty much drop-in replacement - only thing to do is to replace the blocks. Nice job!

Thanks for making these sandbox projects, both look worth investigating! I wonder what the status is for @tuutti 's and @matsbla 's sandbox projects: Are they going to become full projects at some point in time or are you waiting for this issue to be resolved in Drupal core?

It is indeed easier to use full projects rather than sandbox modules in projects.

+++ b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php
@@ -151,6 +151,7 @@ public function build() {
     $manipulators = array(
       array('callable' => 'menu.default_tree_manipulators:checkAccess'),
       array('callable' => 'menu.default_tree_manipulators:generateIndexAndSort'),
+      array('callable' => 'menu.language_tree_manipulator:filterLanguage'),

This is the most pita here.
Every contrib that needs to add more manipulators require to override this block or implement own.

SO probably that need follow-up for make sort of hook for this place

I agree with @andypost. We should discuss how to expose altering of that thing.

Thanks for making these sandbox projects, both look worth investigating! I wonder what the status is for @tuutti 's and @matsbla 's sandbox projects: Are they going to become full projects at some point in time or are you waiting for this issue to be resolved in Drupal core?

Probably not gonna be a full project anytime soon (unless someone else promotes it), because I'm too lazy to apply for a vetted git access.

@tuuti Would gladly help you with it. Sent a pm.

tuutti opened one issue here about merging the modules, which I've been late to answer, but I finally did now;
https://www.drupal.org/node/2847313
maybe others want to give their feedbacks and ideas about what functionalities that are most useful to have in a contrib module

we also applied for full project here;
https://www.drupal.org/node/2835768

Thanks for the updates! I'll write a comment in #2847313.

I've created an recursive function for checking links in main menu, if they are translated into the current content language - if they're not, I remove them from output.

use Drupal\Core\Menu\MenuLinkInterface;
use Drupal\menu_link_content\Plugin\Menu\MenuLinkContent;
use Drupal\Core\Language\LanguageInterface;
/**
* Implements hook_preprocess_menu().
*/
function YOURMODULE_preprocess_menu(&$variables) {
  if ($variables['menu_name'] == 'main') {
    $language = Drupal::languageManager()->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
    foreach ($variables['items'] as $key => $item) {
      if(!$variables['items'][$key] = YOURMODULE_checkForMenuItemTranslation($item, $language)) {
        unset($variables['items'][$key]);
      }
    }
  }
}

function YOURMODULE_checkForMenuItemTranslation($item, $language) {
  $menuLinkEntity = YOURMODULE_load_link_entity_by_link($item['original_link']);

  if ($menuLinkEntity != NULL) {
    $languages = $menuLinkEntity->getTranslationLanguages();
    // Remove links which is not translated to current language.
    if (!array_key_exists($language, $languages)) {
      return false;
    } else {
      if(count($item['below']) > 0) {
        foreach ($item['below'] as $subkey => $subitem) {
          if(!$item['below'][$subkey] = YOURMODULE_checkForMenuItemTranslation($subitem, $language)) {
            unset($item['below'][$subkey]);
          }
        }
      }
      return $item;
    }

  }
}

function YOURMODULE_load_link_entity_by_link(MenuLinkInterface $menuLinkContentPlugin) {
  $entity = NULL;
  if ($menuLinkContentPlugin instanceof MenuLinkContent) {
    list($entity_type, $uuid) = explode(':', $menuLinkContentPlugin->getPluginId(), 2);
    $entity = \Drupal::entityManager()->loadEntityByUuid($entity_type, $uuid);
  }
  return $entity;
}

I've tried Menu Multilingual and it works fine for me. But in my opinion this should merged into core respective it should be implemented into core functionality. There are different scenarios where this is useful and necessary like unpublished translations or language dependent contents. The latter is a requirement of about 80% of all multilingual websites I made in the past (no matter what CMS). And it's a requirement of the current D8 project too. So I would say this functionality is mostly desired ;)

I think the key problem is you would need to have some kind of per menu or per menu item (or content type or menu source) setting. Certain listing pages for example would make sense regardless of the listing page label translated, eg. if its "Microsoft", "Apple", "Samsung", etc. While others would not make sense. Also some people do want their content to not be partial even though it may not be fully translated. So I think while some simple cases are simple, the tricky part is figuring out the set of configuration options that cover an 80% use case AND not get in the way for someone who wants to do more advanced things. Which is basically why this is not *yet* in core.

I've got a use case to add to the pile.

My menu links to a view that displays videos. The view has a contextual filter that filters down by taxonomy attached to the video. e.g. /videos/1/64

We've just been asked to add a new category and a new video. The English video is ready, but the French is not. But the English video has to go up on our site ASAP.

I'd like to be able to disable the menu link to /fr/videos/1/64, but the English translation is showing up in my menu heirarchy. I'm not sure if I can change a menu or view setting that hides the English menu link on the French site, which goes to a view that displays no results.

Is there a setting or sandbox module that would work best for this? I was thinking of hacking it in with CSS - not the best solution.

Use case:
Current client wishes to exclude specific links created in master/default language from specific locales/translations.

Proposed solution:
Allow enable/disable menu item flag to be localized. Currently toggling off the enable flag eliminates menu item from default and all translations of a given menu. Instead allow translated items to be disabled. If translation exists and it is disabled the system should neither show it nor the default version, even if the default language version is flagged as enabled. Items without translations continue to show the default version. If a parent item of a menu tree is disabled for a given locale, neither it nor its children will display in the menu for that locale.

Proposed Implementation
The menu_link_content_data table contains a langcode and default_langcode field along with all the other information defining a menu item including the enabled field. Modify the menu rendering logic and the menu item translate form to accommodate the selective toggling of the enable field for specific menu items by the given language/locale.

Will follow with an attempt at a patch.

+++ b/core/lib/Drupal/Core/Menu/LanguageMenuLinkManipulator.php
@@ -0,0 +1,53 @@
+        unset($tree[$key]);

You can't unset tree item. You should use the following approuch here

$tree[$key]->link = new InaccessibleMenuLink($tree[$key]->link);
$tree[$key]->subtree = [];

see \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::checkAccess for details

@andypost nailed it in #2466553-30: Untranslated menu items are displayed in menus.

If custom MenuLinkTreeManipulators were easily alterable/discoverable any module could nuke menu items as they wish.

Also needed to remove links to untranslated nodes from the menu. We use menu blocks provided by core as well as Superfish and Menu Block so I patched the build method of those modules simply so I could add my custom menu link tree manipulator to the array of callables.

In lieu of having ten main menus (one per language), we also have a field on our content types which admins can tick to remove a node in a specific language from the menu. Again, this was easily taken care of by our custom menu link tree manipulator save for the fact core does not provide a way for it to be picked up.

Simplest fix would be to add an alter hook to MenuLinkTree::transform?

$this->moduleHandler->alter('menu_link_tree_manipulators', $manipulators);

But since menu.default_tree_manipulators is just a service, are we meant to override that instead?

@hitfactory it's bad idea to add alter hook into MenuLinkTree::transform() because it's responsibility of calling code to define required manipulators (it may cause security issues if some alter will kill access checking)

So getting #2854013: Allow SystemMenuBlock tree manipulators to be altered in will help to move forward here

In my very cursory understanding of this issue, and also by virtue of the fact that this has 55 followers, which is quite high, it seems like the current core behaviour is not what's generally desirable.

It sounds like what we might want to do is "flip" the behaviour where right now, we're supporting advanced use cases, but making people go to contrib to solve 80% use cases, and instead do the other way around once #2854013: Allow SystemMenuBlock tree manipulators to be altered is in.

But, it also sounds like we need to come to consensus on what the "80% behaviour" actually is. I see at least 3 contrib projects mentioned above that people claim solve the problem. We should distill those down into a set of core behaviours and create a "spec" for how we want that to work.

Tagging for UX team review on a future call to help facilitate this.

I was having the same issue so I've come up with a custom solution.

I think it's a good fix for most of the use cases explained above.

Having a new MenuLinkTreeManipulator service looks good to me. It can be extend or customize to match future use cases.

My use case

1. I created a menu with three not-translated links (let's say 1 link in FR, 1 link Spanish, 1 link English).
2. I place this menu in header.
3. No matter the current interface's language, the three links are displayed.

Solution

To have a custom MenuLinkTreeManipulator service.

See this contrib module for a living example:
https://www.drupal.org/project/menu_manipulator

Manual fix

1) Create a custom MenuLinkTreeManipulator plugin in your custom module (see attached file MenuLinkTreeManipulators.php)
modules/custom/mymodule/src/Menu/MenuLinkTreeManipulators.php

2) Expose this as a service

services:
  mymodule.menu_tree_manipulators:
    class: Drupal\mymodule\Menu\MenuLinkTreeManipulators
    arguments: ['@access_manager', '@current_user', '@entity_type.manager', '@language_manager']

3) Load and filter your menu (e.g. in a preprocess hook)

   // Get current language ID.
   $current_langcode = \Drupal::languageManager()->getCurrentLanguage()->getId();

    $menu_tree = \Drupal::menuTree();
    $manipulators = [
      // Only show links accessible to the current user.
      ['callable' => 'menu.default_tree_manipulators:checkAccess'],
      // Use default sorting.
      ['callable' => 'menu.default_tree_manipulators:generateIndexAndSort'],
      // Filter by the Current Language.
      ['callable' => 'mymodule.menu_tree_manipulators:filterByCurrentLanguage'],
    ];

    // Generate the menu tree.
    $menu_name = 'main-navigation';
    $parameters = $menu_tree->getCurrentRouteMenuTreeParameters($menu_name);
    $mymenu_tree = $menu_tree->load($menu_name, $parameters);
    $manipulated_mymenu_tree = $menu_tree->transform($mymenu_tree, $manipulators);

    // Get menu as renderable array.
    $variables['menu_left'] = $menu_tree->build($manipulated_mymenu_tree);

I have same issue. I tried #47 which seems to be nice solution (thank you @matthieuscarset) but I had problem with calling private method and code also doesn't filter submenu items based on current language (only top level).

I have extended #47 code to fix my problems. Everything is same as in #47 but service is

services:
  mymodule.menu_tree_manipulators:
    class: Drupal\mymodule\Menu\MenuLinkTreeManipulators
    arguments: ['@access_manager', '@current_user', '@entity_type.manager', '@language_manager', '@entity.repository']

For modified version (fix private method call and filter submenu items based on current language) of MenuLinkTreeManipulators please see attachment.

Thanks for the update @marysmech.
I've just tested your changes and everything is working fine.
Your way of retrieving the Menu Link's language is definetely better.

Hopefully we can see this integrated in core soon...

What are the next steps to get this fix in core?
I would like to do it but don't really know how to start or if community is even in agreement with this solution..

Should I open a new thread?

Meanwhile I have created a contrib module to help others: https://www.drupal.org/project/menu_manipulator.

I tested both https://www.drupal.org/project/menu_multilingual and https://www.drupal.org/project/menu_manipulator modules and menu_multilingual seem to be a better UX/site-building experience.

For example, when enabled "Filter a menu by the current user's language" my admin menu started filtering links etc.

#50 I believe the next step would be to update the issue summary taking #46 in consideration and get feedback from UX.

Thanks for having tested and compare both modules @iztok in #51. Based on your feedback, I have improved "menu_manipulator" so that users can now select which menus have to be filtered by language.

Thank you for your answer @johndevman in #52. I understand this issue is now waiting for the UX team feedback. Hopefully it will happen soon because this issue if 3 years old...

I'll be glad to help getting new behavior in core for Menus.

Please explain what task needs to be done.

Attached patch can be applied towards current 8.6.x

From there on, modules that extend SystemMenuBlock
need to add ['callable' => 'menu.language_tree_manipulator:filterLanguage'], in their manipulators list.

I tried the attached patch with a patched version of Superfish and I got the expected results.

Hello,
Thank you for your patch #55. It works well with root menu items, however, it does not filter untranslated menu items with depth greater than 0.
Maybe the $tree variable should be searched recursively in LanguageMenuLinkManipulator?
Thanks in advance.

We discussed this issue at the end of the weekly Usability meeting today (2019-01-29): http://youtu.be/l6M75JbEroQ

We agreed that there is not much we can say without some sort of research project: user testing or a survey of multilingual site builders. We do not know how else to decide which option is the 80% use case. So I am removing the "Needs usability review" tag.

If anyone wants a usability review for this issue, please update the issue summary with at least

1. the proposed resolution
2. available work-arounds

The work-arounds should include contrib and sandbox modules, as discussed in the comments, and perhaps even code snippets posted to some of the comments.

Even with that information, it will be hard to say whether any particular solution is better than what core already has. That is, the preferred solution may be to use a contrib module.

Just to add my opinion here: I think the main issue is that currently it is not possible to deactivate a menu item per language. I think the suggestion mentioned in #40 is a good idea. As the standard behavior stays as is and it just opens up the possibility to not show certain items in one language by making the button translatable.

Expanded patch #55 to include menu items in subtree.

Attached patch is the same as #55 but can be applied in 8.7.1.

Updated IS and rerolled the failed 8.6 patch.

I tried to browser through the different comments and update the IS according to #56 with different proposed solutions and work arounds.

I'm not sure if we can find a solution that cover 80%. I think multilingual sites have very diverse requirements. To manually turn on/off menu items per language might work for small sites with few languages, but would be very cumbersome for sites with many menu items or many languages. Some sites might also want to hide menu items depending on if the content that it is linked to is translated (in addition to the menu item). Sites with different localizations of same languages (like French + Canadian French) might want to enable fallback languages for menu items.

I think if this should be solved in core it should follow with some basic config options to provide flexibility for each menu block. As example, in many cases I would guess it would make sense to filter out untranslated menu items in the main menu, but not in the footer menu (with links to legal notes). Then make it easy for contribs to alter the behaviour or add new multilingual config options.

I agree with #58 to support #40.

But meanwhile, I'm using #61, which is reroll of #55.

I also agree with #63 and #58 to support #40 because as said in #62 we won't be able cover all cases, but have a flexible solution then.

#61 seems to need a new reroll against 8.7.x

Patch needs a reroll against 8.8.x (and 8.7.x if relevant)

Reroll for 8.7.x attached, 8.8 reroll still needed

I've applied the patch because I had the same issue, however, after applying the patch, the untranslated children of translated menu items are still shown. I've altered the patch to make this work recursively so that the untranslated children are also hidden.

Hi,

I have reviewed the patch #68 with 8.9.x latest code. Below are my updates.

I have created the page and add that page to the main menu. In my case, there are two languages English and french.

And I had not translated any node but still, all menu item is showing in both languages.

Below is an output screenshot in the English language.

Below is an output screenshot in the French language.

Am I doing anything wrong in my testing?

Thanks,
Ren

I've applied the patch in #68 using the following configuration environment and have successful and predictable results when configured correctly.

For these steps, I've used en (English - default) as a source language and de (German) as a destination language. Standard core menus are used for reference. The patch has been applied and the supporting multilingual configurations exist and are functioning correctly.

I have not yet tested the recursive menu item configurations.

Assumptions (Site & Content Configuration)

1. The core multilingual modules are enabled.
2. At least two languages exist.
3. The content entity type you are referencing (node/media/taxonomy) is properly configured for translation in the Content and Language settings. (e.g., Page)
4. Content exists for the translatable content entity in both source language (en) content and translated (de) content. e.g. a Page node of node/123 has a translation node/de/123.
5. Custom Menu Items are configured for translation in the Content and Language settings for the Menus you need filtered. (e.g., Main navigation, Footer)

Content Creation Instructions (after site configuration):

1. Create The Translated Content
1. Create and translate your content (e.g., Page of node/123 de/node/123), publish it.
2. Create a custom menu link (within the menu you've configured for translation). It's Title and Description are translatable. Choose the link to the translated, published page (node/123). The destination URL will be the same in all languages by default in core.
3. Visit both languages and see that the menu link exists and is correct.
2. Create The Source-Language Only Content
1. Create the content in the source language (e.g., Page of node/456), publish it.
2. Create a custom menu link (within the menu you've configured for translation). It's Choose the link to the untranslated, published page (node/456). The destination URL will be the same in all languages by default in core.
3. Visit both languages and see that the menu link node/123 does exist and is correctly displayed in both languages. The node/456 page will only show in the source language.

Considerations:

1. If you are using other modules to interact with menus, it may alter expected display behavior. I've seen this happen on sites extended with Superfish and with other menu customizations.
2. If the custom menu links aren't translated, the translated content won't show up, even if the destination node is translated. BOTH content entities (the menu link AND the destination content entity) must be translated to get this to work.

The patch from #68 works for me too. Thank you.

From a quick review the patch seems to do be ok, however there are a couple of fixes needed.

1. +++ w/core/lib/Drupal/Core/Menu/LanguageMenuLinkManipulator.php
   @@ -0,0 +1,58 @@
   +   * Generates a unique index and sorts by it.
   

   This method description doesn't seems correct.

2. +++ w/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php
   @@ -256,6 +257,17 @@ public function isTranslatable() {
   +    if (!$this->isTranslatable()) {
   +      return TRUE;
   +    }
   

   In the context of the patch I understand the logic of this choice, but could benefit of a small comment.

   Overall would be probably even better to return FALSE if it's not translatable and let the ManguageMenuLinkManipulator handle this by calling isTranslatable().

Re-rolling this for Drupal 9.1.x, and taking into account the comments from #73.

I agree that the newly added MenuLinkContent::hasTranslation() method should return TRUE or FALSE depending on whether the entity has a translation or not, and move the use of the isTranslatable() call to the LanguageMenuLinkManipulator::filterLanguage() method.

Also providing a patch for Drupal 8.9.x as I need that one :)

With #57 in mind, I'm wondering if this issue should only focus on getting the interface implemented which would allow contrib. modules to efficiently implement this behaviour.

#75 works for me on 8.9.x, however it would be nice to have the option to untranslated menu items across all languages (i.e. only hide other translations).

#75 works for me on 9.1

Just checked #75 with 9.0.7: the original menu link still displayed in case if the translation is unpublished.
Expected that the original would not be displayed in that case.

Seems that we also need take content_translation_status into account.

Here is my proposition on how we can handle content_translation_status. I'm expanded the existing hasTranslation method for it.

Can you provide an interdiff, please?

Our site is on 8.9.6 and it seems like patch from #68 and #75 have different results.

In our site we have English, Finnish and Swedish languages. The main menu has only Finnish translations at the moment with no English or Swedish translations at all. Patch number #75 seems to always show Finnish menu items in the English version but #68 hides them correctly.

Sounds like a similar problem as #80.

9.x patches don't seem to apply for 8.9.x branch.

I'm using #75 on 8.9.7 and it works as expected, I don't have issue described in #83 - I have Polish and English menu items with some links only available in Polish. They show only in Polish and not English, as they should:
Polish: https://static.investmag.pl/dp2/16033551021102210544.jpg
English: https://static.investmag.pl/dp2/16033551284612564748.jpg

I don't use any module like Menu Link Manipulator, Menu Block (only the core equivalent) or anything like that.

#75 works for me on 9.1

So, some visual bugs are introduced with this patch. See screenshots:

The interface selected language in the drop-down is not selected when editing the menu link. In this screenshot it should be Vietnamese. But it was some random other language. In this case, I had to manually change it from Spanish to Vietnamese.

The enable/disable flag on the menu overview page doesn't reflect the correct status for the interface selected language. These links shouldn't be enabled in Vietnamese, but they were in English (or the source language).

And there is a bit of a head scratcher when you see that enabled/disabled is not language specific, but it actually seems like it is in practice.

In my case, I have a menu link to an external site that I'd like to remove in some of my translated language. After testing this patch, I'm guessing this is a feature that is out of scope for this issue.

#75 works for me on 9.1

Note: @GregBoggs you should be able to hide/show external link by adding/removing translation of the menu item itself.

It's nice that #75 works for some people. However that doesn't change the fact that it has to address the comments in:

• #80: Is it fixed by #81?
• #86

Some interesting notice we got from @jpbeaulieu while working on menu_multilingual

When the menu has lots of items & translations then the consumption of memory goes up linear since entity storage has static in-memory caching.
So I think it is good idea to find a balance between memory consumption and speed optimisation.
Maybe using some memory limit check and cleanup static cache via resetCache() for the items that are going out of this limit?

I think this problem should have an own issue, but I think posting it here it is a good way to point out it.
There is a service module to test performance of having a lot of entities and translations that could be used to test memory problem with menu content items: stress_test_add_translations

Added new patch for 8.9 on top of #75 with the only difference that the current language is selected based on the content language, not the interface translation language.

Hi, will the patch in #92 work with Drupal 9.2.3 as well? We have patched with #81 and not #75.

Update: no, it does not. Just tested it.

See the attached patch based on 9.3.x, which also works for 9.2.6.

Resolved custom commands failures & added an interdiff.

Consider the following scenario:

1. I create a node in FR and publish it
2. I create a DE translation of the node and publish it
3. I want to unpublish FR version of the node, but leave the DE version published

This is not possible, because MenuLinkContent::hasTranslation() checks only existence of the translation, but not its status. I'd suggest that hasTranslation also checks entity access, something like

  /**
   * {@inheritdoc}
   */
  public function hasTranslation($langcode) {
    return $this->getEntity()->hasTranslation($langcode)
      && $this->getEntity()->access('view');
  }

We cannot rely on `menu.default_tree_manipulators:checkNodeAccess`or `menu.default_tree_manipulators:checkAccess`, because they will keep menu links in the tree when at least one language variant is accessible by the user (i.e. there is at least one published translation), not necessarily the currently displayed translation

Exactly the same that I wrote around a year ago at #80 :)

Here's a patch that checks if the translation exists and the user has access to it (unpublished nodes will be shown only to users with appropriate permission)

Apparently, patch in 100 is missing newly added file, resubmitting (interdiff is still the same)

Sorry for several patches, wrong `git diff`-ing

...and bring back missing

<?php
use Drupal\Core\Url;
?>

#105 seems to fix the problem for us.

Here is a new patch identical to #105, but with automated (Kernel) tests.

Fixed typo reported by CSpell.

I dare call this RTBC

I applied this patch. How do we prevent menu items of another translation/language from showing when on another language? Observe:

The English links should not show here as we are on a Hebrew page. There are no translations for those two links.

1. +++ w/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php
   @@ -245,6 +247,15 @@ public function isTranslatable() {
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function hasTranslation($langcode) {
   +    $entity = $this->getEntity();
   +    return $entity->hasTranslation($langcode)
   +      && Url::fromUri($entity->link->uri)->access();
   +  }
   
   +++ w/core/modules/system/src/Plugin/Block/SystemMenuBlock.php
   @@ -189,6 +189,7 @@ public function build() {
          ['callable' => 'menu.default_tree_manipulators:checkAccess'],
   

   I don't think we should be check access and whether the entity has the translation in the same moment. Feels really odd. Why is the menu.default_tree_manipulators:checkAccess not sorting this out for us? And if this is necessary this needs a really go explanation of why. Also maybe the access check could go in the manipulator if it is necessary.

2. +++ w/core/lib/Drupal/Core/Menu/LanguageMenuLinkManipulator.php
   @@ -0,0 +1,57 @@
   +  public function filterLanguage(array $tree) {
   
   +++ w/core/lib/Drupal/Core/Menu/MenuLinkTranslationInterface.php
   @@ -0,0 +1,21 @@
   +  public function hasTranslation($langcode);
   
   +++ w/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php
   @@ -245,6 +247,15 @@ public function isTranslatable() {
   +  public function hasTranslation($langcode) {
   

   Needs a return typehint.

3. +++ w/core/lib/Drupal/Core/Menu/LanguageMenuLinkManipulator.php
   @@ -0,0 +1,57 @@
   +    $current_language = $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
   

   Why LanguageInterface::TYPE_CONTENT and not LanguageInterface::TYPE_INTERFACE? Most menus are are considered interface - no? If this is correct then we need a comment to explain why.

@alexpott, thank you for the review, this is indeed an important issue for proper core translation handling!

Re:

Why LanguageInterface::TYPE_CONTENT and not LanguageInterface::TYPE_INTERFACE? Most menus are are considered interface - no? If this is correct, then we need a comment to explain why.

Uff, this needs a general and upstream discussion in #2864972: Content and interface translation don't clearly separate (or even upper) for the concept, I think. This isn't only a problem for menu items, but also for blocks and other areas, as Drupal can't just say it's this or that. It depends on the way Drupal is used.

Here, I'd personally see it as content translation related, if it's not an admin menu.
So my vote would be to discuss a setting on menus to decide per menu, if it's content or interface. For me the rule of thumb is that interface translation is the admin language and content language is what visitors see.
I guess this didn't receive much focus yet as most core maintainers are from the US and their interface language mostly is English. If your native language is e.g. Spanish, German or French and you're maintaining a site with Chinese or Russian content (but don't understand it), things are still hard with Drupal 9... But let's keep that separate! Just to point out why this can't be a side-topic and isn't a simple decision for this issue.

To have a solution here, I'd vote to simply decide for one and postpone a possible change?

Thanks in advance!

Note: There is a related issue in contrib module Twig Tweak: https://www.drupal.org/project/twig_tweak/issues/3033832

Tried to address point 2 of comment #110, still needs work for other points.

Hi,

We discussed this a while ago with the Drupal UX group. The issue would benefit from updating the issue description and including some screenshots. It is not quite clear what the expected outcome is and the thread has over hundred comments.

There are 4 Proposed resolutions listed

1. Display menu items either without a specific language or in the current language.
2. Make it possible to config each menu to display menu items either without a specific language or in the current language.
3. Make it possible to hide/display menu items on certain languages
4. Do nothing and let solutions live in contribs

Which resolution is attempted by the patches posted in the issue thread?

Overall resolution was, that it should be made sure that current default behaviour is no changed. This in order not to make core update to change how existing installations work.

@simohell #115

I have now run into the same issue.

My pick of a possible solution would be: add an option (checkbox) to menus to only display links in the currently language and links without a specific language.

I tried #114 on my site (9.4.4) and it worked and did what I wanted (it hid any untranslated menu links)

-

Here is a reroll of #114 for 10.1.0.

Removing the "Needs usability review tag" since #113 doesn't mention what needs to be reviewed.

This addresses a deprecation warning and #110.1.

I agree with #111 that it depends on the menu, but using \Drupal\Core\Language\LanguageInterface::TYPE_CONTENT is a sensible default. We probably need a follow-up to make this configurable.

I documented the currently proposed solution which is to display menu links for the current language, including links without a language. This makes it possible to support use cases like #1 by explicitly marking the links as language not applicable.

This does leave out some complex use cases where you want to rely on fallbacks, and want to also override the translation for a specific language because entities without a language cannot be translations. I think it would be fine to leave this for contrib since this seems more of an edge case.

This will result in a behavior change, but that's to be expected from a bug fix. We need to document in a release note and change record what the change is, and what is expected from the maintainers of the site.

Addressing phpcs violations

Added autowiring for the new service

Tried testing without the patch

Enabled a 2nd language (Dutch)
Create a page in English
Translated for Dutch

Added a menu link the Main nav
Translated menu link for dutch

English link appears on english page
Dutch link appears on dutch page

Is there a step I'm missing?

Moving to NW for a change record though for the new service and interface.

Added change records.

Also tested by creating 3 menu links: 2 with a Spanish translation and 1 without.

Without the patch when I choose Spanish for the page, the 'Contact' menu link is still there

With the patch when I choose Spanish, the 'Contact' menu link is gone as expected.

Will be great to get this.

Needs work for merge request comments

We need more site builder focused Change record that details the behavior for non-developers. Basically we need to let people know that certain items in menus may no longer show based on language.

We could add explanation in this CR about to get the old behavior back through custom code. This would probably be to change the class for the new service and override filterLanguage to do nothing

Added site builder focused CR mentioned in #132.

+++ b/core/lib/Drupal/Core/Menu/LanguageMenuLinkManipulator.php
@@ -0,0 +1,57 @@
+      if ($link->link instanceof MenuLinkTranslationInterface) {
+        // If the link is translatable, but has no translation, hide it.
+        if ($link->link->isTranslatable() && !$link->link->hasTranslation($current_language)) {
+          unset($tree[$key]);
+        }

We discussed this briefly on Slack with @lauriii and @catch.

I read all my previous feedback again, and I think my feedback from previous comments are still valid.

Catch raised a good point that if you add a foreign language and start translating pages, suddenly your content menu items will be gone as they don't yet have translations. He also raised the same I did multiple times is that its hard to tell for people which items are coming from content items, links.yml, views, other config, etc. so they likely can't predict how different items would behave with this solution.

I still think its not as simple as to punt all of those to a separate issue. The behaviour of menus would be very confusing. I also still think that punting this problem space to contrib where a complex solution or multiple complex solutions can exist is not a problem. We don't have complete solutions of other things in core either but rather focus on shared parts.

That said, @laurii raised that maybe the menu item exposure setting and behaviour should be on the content entity bundle settings then where the configurability of menu items is. I think that would be an improvement but would still have the drawbacks of very different behaviour depending on which translation system translates (or not translates) the menu item.

Pushed a commit that adds a generic hook_entity_access which checks for the new setting, and removes access to entities that are untranslated when \Drupal\language\Entity\ContentLanguageSettings::$show_untranslated_content is FALSE.

There are some challenges with this approach:

1. \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::menuLinkCheckAccess does NOT actually check if the user has access to the custom menu link, it checks if user has access to the target route. This can be confirmed by checking \Drupal\menu_link_content\MenuLinkContentAccessControlHandler::checkAccess which requires administer menu permission to view the custom menu links.
2. When checking the target route access, if the target is a node, \Drupal\node\NodeAccessControlHandler::access allows users to bypass the access control handler which makes the site building experience confusing because they would see the untranslated menu links, but the end user would not.
3. This also means that whether the menu link is visible or not, depends on if there is a translation for the target, regardless if the menu link has been translated or not, which is not the intended behavior. This would mean that if you link to a View for example, the menu link would be always visible regardless of its translation because the View would be always available in every language.
4. We could add an additional menu link tree manipulator to remove custom menu links that are untranslated when this configuration is enabled. However, there's no API to do this at the moment. See #3091246: Allow MenuLinkTree manipulators to be altered and #3091246: Allow MenuLinkTree manipulators to be altered where contrib maintainers are asking for this API to be added.

Based on #136, tried to push forward on a solution that implements the new API, moves the functionality to custom menu links module, and makes it configurable per-block basis. I still believe we should implement something like #136 in the long run (maybe first in contrib?), but since most of the changes in the original proposal are needed anyway, we might as well try to start with the smaller scope. 😊

The new API !4589 should address #3091246: Allow MenuLinkTree manipulators to be altered and #2854013: Allow SystemMenuBlock tree manipulators to be altered too because it implements a new chained menu tree manipulator which can be called where it makes sense. I added it to all cases where the menu tree is being manipulated but we may want to revert it back to just \Drupal\system\Plugin\Block\SystemMenuBlock, and add rest of the uses in a follow-up. Main reason to do that would be that we'll have to manually construct context for each use depending on what makes sense for the use case. For example, in the case of \Drupal\system\Plugin\Block\SystemMenuBlock the MR is currently passing the block plugin instance so that the manipulator can decide how the links should be manipulated based on that.

He also raised the same I did multiple times is that its hard to tell for people which items are coming from content items, links.yml, views, other config, etc. so they likely can't predict how different items would behave with this solution.

I think this is a pre-existing problem which is made slightly worse by this issue. IMO we should have a separate issue to improve the UI to explain the difference between the different sources, and what they mean.

From a technical point of view, I think the solution is elegant and it's a desired new API (I've recently needed to override menu blocks in a project for altering manipulators).

As hide_untranslated_menu_links is FALSE by default, current behavior is kept, which is one of the concerns I had. I think this solution covers all the other concerns that Gábor raised though the years.

> He also raised the same I did multiple times is that its hard to tell for people which items are coming from content items, links.yml, views, other config, etc. so they likely can't predict how different items would behave with this solution.

This is a common issue in different translation scenarios. Is this string coming from content? Config? Interface translation?
Sadly is a drawback that comes with the power of Drupal, and I don't think it can be fixed through anything than education. The good thing is that it's a checkbox people can experiment with. Which also solves

> Catch raised a good point that if you add a foreign language and start translating pages, suddenly your content menu items will be gone as they don't yet have translations.

You can temporarely disable while you work on your content. So even if this is not perfect, I think is a great step forward to make menus + translations less of a problem for end users.

Unless we need an upgrade path to set the value, which I'm not sure of as it's the default, we can RTBC this one.

I think we could mark the 'hide_untranslated_menu_links' as nullable.

Opened a follow-up to improve the menu overview: #3382634: Distinguishing different types of menu links from each other is difficult.

He also raised the same I did multiple times is that its hard to tell for people which items are coming from content items, links.yml, views, other config, etc. so they likely can't predict how different items would behave with this solution.

I think this is a pre-existing problem which is made slightly worse by this issue.

OK if people strongly believe that making this worse is a good tradeoff for solving part of the problem then let's do it. It would be important that core solves this in a way that contrib can still easily swap out / override for "real" solutions that actually consider Drupal's complexity IMHO.

For example, this implementation considers this the "current language" (and it does not explain on the UI that it considers this the current language):

$current_language = $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();

On the other hand sites may consider their menu "interface" and thus remove items not in the interface language even if the page's content language is different from its interface language.

The differentiation between "interface blocks" and "content blocks" in terms of language support are explained in https://www.hojtsy.hu/blog/2013-jul-09/drupal-8-multilingual-tidbits-7-b... btw :)

This seems like a large enough problem that it would be acceptable to introduce the required complexity to come up with a solution that can address the problem at least in majority of the use cases. The majority use case, which is the public facing multilingual site navigation, specifically requires this to be based on \Drupal\Core\Language\LanguageInterface::TYPE_CONTENT. The great thing about the current implementation is that it's done completely in the custom menu links module, and it can be toggled off. Also, new manipulators can be introduced as required. 😊

Maybe we could change the description for extra clarity:

When selected, custom menu links without translation in the current content language are hidden.

If we're happy with the current solution in a high level, next step would be to update the change records to match the new implementation.

++ amazing stuff

I think the implementation and UI looks great, and is exactly the use case I’ve had in all multilingual websites I’ve built so far.

++ to get this in

But I would be happy as well to just get the interface and manipulator in and split the implementation to follow up

Updated the change records. Also added the applies method to \Drupal\Core\Menu\MenuLinkTreeManipulatorInterface so we can ensure that all implementations have a separate applies method.

Just tested on Drupal 10.1.2 and patch #125 works but the plain diff from the MR does not apply.

Not sure what's the differences are.

Which is the most updated code? path or MR?

@matthieuscarset the MR should be the most recent approach (slightly different from the approach in the patch #125). I have applied the diff from the MR to 11.x-dev without any problems.

-------------

I think that the current approach here with MenuLinkTreeManipulators is good and big advantage is this:

The great thing about the current implementation is that it's done completely in the custom menu links module, and it can be toggled off. Also, new manipulators can be introduced as required.

Similar behavior was required on most of our multilingual sites and reading the whole issue, it seems to be highly desirable feature among others too. So I think it will be great to have this possibility directly in the core and hopefully this can make its way into 10.2.

Discussed with @lauriii - we decided to tweak the new API to be clearer about the difference between the new menu tree manipulator and the existing ones. Specifically we need to always provide context for these global manipulators whereas the current manipulators eg checkAccess don't need this. This should also make it easier for contrib modules to provide new menu link tree contextual manipulators without having unintended side effects. And if they do have such side effects make them easier to fix because the API will guarantee that the context is passed in.

The MR should be ready for another review. The changes were mostly internal. From the perspective of the consumer of this API this should be a positive change because this makes the context available in more cases, and provides a deprecation that will make sure contrib also provides the context going forward. The API change to the signature of \Drupal\Core\Menu\MenuLinkTree::transform is documented in https://www.drupal.org/node/3380512.

Just posting an updated patch for D9, based on the idea from patch #75. We were running patch #114 for some time but started having issues when using language access and having different interface languages in combination wit users requesting non existing pages...
Patch from #75 did work for us, so I updated it with the more modern coding standards from #114.

I'm hoping to update to D10 soon and try the actual real proper fix from @lauriii!

The CR https://www.drupal.org/node/3380509 needs to be updated for the recent move to using a tag named menu_tree_contextual_manipulator plus it perhaps needs expanding to explain the motivations behind MenuLinkTreeContextualManipulatorInterface and how important it is to use applies to narrow down the possible side effects to the ones you want.

Good catch! Updated the CR 👍

I've gone through all the comments and code reviews and assigned issue credit. Hopefully got this right as there certainly are a lot of contributors interested in this one!

Reading all the comments on the issue - I think we need to address @niko-'s comment in #42 as we're still doing an unset here.

I believe I have addressed the feedback. I also made \Drupal\Core\Menu\MenuLinkTreeContextualManipulatorInterface extend \Drupal\Core\Cache\CacheableDependencyInterface because we need cacheable metadata from the manipulator itself. For example, \Drupal\menu_link_content\LanguageMenuLinkTreeManipulator needs to be cached based on the languages:language_content cache context.

I realized what I implemented in the MR is not entirely correct; the cacheable dependency should be added only when the transform has been applied on a certain menu link tree. However, I'm not sure how we can do that because the cacheable dependency is handled in \Drupal\Core\Menu\MenuLinkTree::build which doesn't have access to the context so that we could call \Drupal\Core\Menu\MenuLinkTreeContextualManipulatorInterface::applies again. Also, the return value of \Drupal\Core\Menu\MenuLinkTreeInterface::transform is \Drupal\Core\Menu\MenuLinkTreeElement[] which means there isn't a good way for us to just inject that in the return value of the transform.

The current implementation doesn't have a dependency to menu_link_content module anymore so I've moved it under system module since the whole functionality is tied to SystemMenuBlock now.

As a side note, the ->isTranslatable() check seems to fail in ContentEntityBase::isTranslatable() if content_translation module is not enabled because the translatable key is populated by content_translation module in function content_translation_entity_bundle_info_alter(&$bundles).

It's perfectly valid to have multilingual menu links without content_translation module, although not very common 🤔

I tested this briefly without content_translation module, added a couple of menu links in different language and the "untranslated" links are hidden if you remove the isTranslatable() check.

Discussed #162 with @alexpott and we agreed that even though that is a pretty strange use case, we should drop the ->isTranslatable() check.

@alexpott also mentioned that it would be great if we could harden the unit tests with ->shouldNotBeCalled() e.g. $non_applicable_manipulator->getCacheContexts()->shouldNotBeCalled().

I tested this briefly without content_translation module, added a couple of menu links in different language and the "untranslated" links are hidden if you remove the isTranslatable() check.

It also seems to hide zxx and und links because hasTranslation() returns FALSE for them, as expected. One way to fix this would be to always return TRUE for locked languages in Drupal\menu_link_content\Plugin\Menu\MenuLinkContent::hasTranslation():

  public function hasTranslation(string $langcode): bool {
    if ($this->getEntity()->language()->isLocked()) {
      return TRUE;
    }
    return $this->getEntity()->hasTranslation($langcode);
  }

Discussed #162 with @alexpott and we agreed that even though that is a pretty strange use case, we should drop the ->isTranslatable() check.

I incorrectly assumed earlier this would work just by removing the isTranslatable() check, but it doesn't work with other menu link plugins or Not applicable/Not specified links because they will always fail to hasTranslation() check and get removed, so we definitely need it.

After digging a bit, the root issue seems to be how ContentEntityBase::isTranslatable() works without content_translation module.

At the moment ::isTranslatable() works like this:

  public function isTranslatable() {
    // Check the bundle is translatable, the entity has a language defined, and
    // the site has more than one language.
    $bundles = $this->entityTypeBundleInfo()->getBundleInfo($this->entityTypeId);
    return !empty($bundles[$this->bundle()]['translatable']) && !$this->getUntranslated()->language()->isLocked() && $this->languageManager()->isMultilingual();
  }

So basically it checks if:

1. Entity bundle has translatable=true. The "translatable" key gets populated in content_translation_entity_bundle_info_alter for entities that has content_translation = enabled third party setting
2. The entity's original language is not "locked", so basically either "Not specified" (und) or "Not applicable" (zxx)
3. The site has more than one language enabled

Meaning it will always fail to step 1 without content_translation module.

I think there are a couple of ways to fix this, assuming we want to support this use case:

1. Fallback to entity type's translatable annotation if the bundle info has no translatable key. Something like:

  // \Drupal\Core\Entity\ContentEntityBase
  public function isTranslatable() {
    // Check the bundle is translatable, the entity has a language defined, and
    // the site has more than one language.
    $bundles = $this->entityTypeBundleInfo()->getBundleInfo($this->entityTypeId);

    if (!isset($bundles[$this->bundle()]['translatable'])) {
      $bundles[$this->bundle()]['translatable'] = $this->getEntityType()->isTranslatable();
    }
    return !empty($bundles[$this->bundle()]['translatable']) && !$this->getUntranslated()->language()->isLocked() && $this->languageManager()->isMultilingual();
  }

I committed this change earlier today and you can find the test result here: https://git.drupalcode.org/issue/drupal-2466553/-/pipelines/29526,

2. Override ::isTranslatable() in \Drupal\menu_link_content\Entity\MenuLinkContent with something similar.

Both changes might be out of the scope of this issue though.

Patch for D10 based in my D9 patch from #151. Just here to more easily migrate from D9 to D10 without having to worry about changed behaviour during the update.

Patch #166 worked in our project (D10.1.5). Thanks!

We were previously using the patch from #81 in our Drupal 9 site.
After upgrading to Drupal 10 we found that the newer patches (tried #125, #166) lead to an issue where unpublished menu translations were still being shown (and in some cases, the incorrect translation was showing).

To fix this, I've taken the `hasTranslation` function from #81 and added it to #166, see the attached interdiff for details.

Apologies if this distracts from the longer term solution that is being progressed in this post, but sharing in case it is useful to others.

Rebased to adapt to recent changes in the core.services.yml file

Rerolled the MR to a patch to work on D10.2.

fixed the phpcs checks

I'm not a huge fan of the context information provided to the tree manipulators. But I'm not sure there is an easier method. Tests are now green again.