Synopsis

Changing the language of a translatable node with a translatable menu link from a "real" language (e.g. en) to a "pseudo" language (und or zxx) will result in an invalid argument exception: Invalid translation language (und) specified. or Invalid translation language (zxx) specified. respectively.

Note: If you are using the contributed token module, you will get a similar error in token_node_menu_link_submit() instead, since that function uses code very similar to _menu_ui_node_save() (see related issues in the sidebar).

Steps to reproduce

  1. Install Drupal 10.1.x-dev (Standard profile) in English language
  2. Enable Content Translation module
  3. Add a second language (e.g. German) at Configuration > Region and language > Languages
  4. Enable content translation with default settings for Content/Basic page and Custom menu link/Custom menu link at Configuration > Region and language > Content language and translation
  5. Add a new basic page in English:
    1. Fill in the title field (e.g. "123")
    2. Expand "Menu settings" in the sidebar and check "Provide a menu link"
    3. Leave everything else at default settings
    4. Click "Save"
  6. Edit your new basic page:
    1. Set the language to either "- Not specified - " (und) or "- Not applicable -" (zxx)
    2. In "Menu settings" in the sidebar, change "Menu link title" to "234"
    3. Leave everything else alone
    4. Click "Save"

Expected result

  • Language of basic page is either und or zxx, depending on your choice above
  • Title of menu link is "234"
  • Language of menu link is equal to language of basic page
  • Neither basic page nor menu link have any translations

Actual result

White page with error message:

The website encountered an unexpected error. Please try again later.

InvalidArgumentException: Invalid translation language (und) specified. in Drupal\Core\Entity\ContentEntityBase->addTranslation() (line 952 of core/lib/Drupal/Core/Entity/ContentEntityBase.php).

_menu_ui_node_save() (Line: 342)
menu_ui_form_node_form_submit()
call_user_func_array() (Line: 114)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (Line: 52)
Drupal\Core\Form\FormSubmitter->doSubmitForm() (Line: 597)
Drupal\Core\Form\FormBuilder->processForm() (Line: 325)
Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
Drupal\Core\Controller\FormController->getContentResult()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 583)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 163)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 58)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 698)
Drupal\Core\DrupalKernel->handle() (Line: 19)

Proposed resolution

The Menu UI module doesn't use the API correctly. It tries to add a translation when it shouldn't.

When updating a translatable menu link in _menu_ui_node_save() and the node is not translatable, don't try to add a translation for the untranslatable language, which would trigger the exception, but instead set the language of the menu link to the node language.

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

n/a

MR to commit

MR 5399

CommentFileSizeAuthor
#16 2904899-16.patch7.72 KBfeyp
#16 2904899-16-tests-only.patch6.35 KBfeyp
#16 2904899-interdiff-15-16.txt685 bytesfeyp
#15 2904899-15.patch7.72 KBfeyp
#15 2904899-15-tests-only.patch6.35 KBfeyp
#15 2904899-interdiff-6-15.txt5.81 KBfeyp
#6 fix-menu-link-to-language-neutral-2904899-6.patch7.46 KBidflood
#2 2904899-2.patch7.85 KBfeyp
#2 2904899-2-test.patch6.12 KBfeyp

Issue fork drupal-2904899

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

FeyP created an issue. See original summary.

feyp’s picture

feyp commented
Status: Active » Needs review
StatusFileSize
new 2904899-2-test.patch6.12 KB
new 2904899-2.patch7.85 KB

Attached is a patch against 8.4.x-dev.

The last submitted patch, 2: 2904899-2-test.patch, failed testing. View results

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

idflood’s picture

idflood commented
StatusFileSize
new fix-menu-link-to-language-neutral-2904899-6.patch7.46 KB

Reroll of patch in #2 for 8.8.x

esolitos’s picture

esolitos
he/him
Primary language Italian
Location Trondheim
commented
Version: 8.6.x-dev » 8.9.x-dev
Status: Needs review » Needs work 
+++ b/core/modules/menu_ui/menu_ui.module
@@ -105,7 +106,13 @@ function _menu_ui_node_save(NodeInterface $node, array $values) {
+        if (!in_array($node->language()->getId(), [LanguageInterface::LANGCODE_NOT_APPLICABLE, LanguageInterface::LANGCODE_NOT_SPECIFIED])) {

I feel that there should be a method pn the Language object like: "canTranslate" to avoid this check.
This would also be useful in several other places where an "AddTranslation" is called, for instance I found this bug report coming from a Paragraph issue.

What do you think?

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Just ran into this issue and was wondering I wasn't ever running into this before, seems to be an edge-case, but still existing.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

feyp’s picture

feyp commented
Status: Needs work » Needs review
StatusFileSize
new 2904899-interdiff-6-15.txt5.81 KB
new 2904899-15-tests-only.patch6.35 KB
new 2904899-15.patch7.72 KB

I'm not convinced that the API change proposed in #7 is in scope here. Adding such a method would constitute a b/c break and would delay this bug fix until Drupal 11, which I don't think is reasonable. If we want to add such a method, this could easily be done in a separate issue later.

Attached are updated patches for Drupal 10.1.x-dev and an interdiff with #6. I renamed the test class to MenuUiContentTranslationTest so that other integration tests with content_translation could be added there in the future, if needed. I also replaced the two test methods with a data provider. The rest of the changes in the test file are for code that was deprecated in Drupal 8 or Drupal 9.

feyp’s picture

feyp commented
StatusFileSize
new 2904899-interdiff-15-16.txt685 bytes
new 2904899-16-tests-only.patch6.35 KB
new 2904899-16.patch7.72 KB

cspell again...

The last submitted patch, 16: 2904899-16-tests-only.patch, failed testing. View results

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs Review Queue Initiative

Can the issue summary be updated. to include proposed solution.

Also the steps were written for D8 are they still the same for D10? Fine if they are just want to double check.

feyp’s picture

feyp commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update

Thanks once more for your review @smustgrave!

The steps to reproduce still apply for the latest 10.1.x-dev, which is also confirmed by the new test coverage. I still did a new manual test following the STR in the IS and updated the core version, a few UI strings that changed slightly since 8.4.x and also updated the stack trace in the actual result. I also updated the IS with the missing info from the current issue summary template including a proposed resolution.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks! That made it much more clear and was able to replicate.

Patch fixes the issue.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 16: 2904899-16.patch, failed testing. View results

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -D8MI, -content-language

Meant to cleanup tags.

Also random failure.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 16: 2904899-16.patch, failed testing. View results

feyp’s picture

feyp commented
Status: Needs work » Reviewed & tested by the community
Related issues: +#3361121: [random test failure] InstallerExistingConfig[SyncDirectory]MultilingualTest::testConfigSync

Random test failure, see #3361121: [random test failure] InstallerExistingConfig[SyncDirectory]MultilingualTest::testConfigSync, retest was green. Back to RTBC per #20.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs followup

Doing RTBC triage. Thank you all for an up to date Issue Summary, it truly helps.

Reading the proposed resolution I wonder why this just being applied when saving a menu_link in the UI. Surely this can be done outside of the UI? And thus things will break? Am I not understanding something?

I think a followup is needed for #7, but as always, search first. Adding tag.

I do not see any comments here explaining what was done for a code review. And it is still listed as a remaining task.

Setting to NW because of my question above.

feyp’s picture

feyp commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs followup

Reading the proposed resolution I wonder why this just being applied when saving a menu_link in the UI. Surely this can be done outside of the UI? And thus things will break? Am I not understanding something?

This is not an API issue. The issue is the way the Menu UI module uses the API to update a menu link. It tries to create a translation when it shouldn't. I tried to rephrase the proposed resolution so that this is hopefully more clear. Feel free to improve on that.

I think a followup is needed for #7, but as always, search first. Adding tag.

Filed #3379839: Add an API method to LanguageInterface to simply checking, if a language is a pseudo language. If this needs to be postponed against a Drupal 11 meta or any tags or something, I'd appreciate your guidance.

Didn't do the review, so can't comment on that. Back to Needs review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Follow up has been filed. Not sure if it should be postponed on this but a user has already started work on it.

#20 did the steps in the IS to verify the issue and the patch solved the issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/menu_ui/menu_ui.module
@@ -95,7 +96,16 @@ function _menu_ui_node_save(NodeInterface $node, array $values) {
+        if (!in_array($node->language()->getId(), [
+          LanguageInterface::LANGCODE_NOT_APPLICABLE,
+          LanguageInterface::LANGCODE_NOT_SPECIFIED,
+        ], TRUE)) {

This can be simpler. Looking at \Drupal\Core\Entity\ContentEntityBase::isTranslatable() - we can change this to
if (!$node->language()->isLocked()) {

See 

  /**
   * Locked indicates a language used by the system, not an actual language.
   *
   * Examples of locked languages are, LANGCODE_NOT_SPECIFIED, und, and
   * LANGCODE_NOT_APPLICABLE, zxx, which are usually shown in language selects
   * but hidden in places like the Language configuration and cannot be deleted.
   *
   * @var bool
   */
  protected $locked = FALSE;

But I think the following change is even better...

  if (!empty($values['entity_id'])) {
    $entity = MenuLinkContent::load($values['entity_id']);
    if ($entity->isTranslatable() && $node->isTranslatable()) {
      if (!$entity->hasTranslation($node->language()->getId())) {
        $entity = $entity->addTranslation($node->language()->getId(), $entity->toArray());
      }
      else {
        $entity = $entity->getTranslation($node->language()->getId());
      }
    }
    else {
      // Ensure the entity matches the node language.
      $entity = $entity->getUntranslated();
      $entity->set($entity->getEntityType()->getKey('langcode'), $node->language()->getId());
    }
  }

As it is using $node->isTranslatable() so we're a bit more logical. Ie. we say if we can translate this menu link and this node then make the languages match. If not then the menu_link must match the language of the node.

FeyP opened merge request !5399

feyp’s picture

feyp commented
Issue summary: View changes

Thanks @alexpott for the review.

W/r/t pointing out LanguageInterface::isLocked(), I closed the follow-up #3379839: Add an API method to LanguageInterface to simply checking, if a language is a pseudo language issue as "Works as designed".

I agree that the changes you suggested are shorter, more logical and more readable, so I updated the code.

I took the opportunity to switch to an MR based workflow. Added the MR under review to the IS as requested elsewhere by @xjm.

feyp’s picture

feyp commented
Status: Needs work » Needs review

Doh, forgot to set to Needs review...

feyp’s picture

feyp commented
Issue summary: View changes

I should have updated the proposed resolution in the IS also, done that now.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Appears feedback has been addressed

Thanks for updating the issue summary :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed ff1090382f0 to 11.x and 91acedade93 to 10.2.x. Thanks!

  • alexpott committed ff109038 on 11.x 
    Issue #2904899 by FeyP, idflood, smustgrave, alexpott: Invalid argument...

  • alexpott committed 91acedad on 10.2.x 
    Issue #2904899 by FeyP, idflood, smustgrave, alexpott: Invalid argument...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

feyp closed merge request !5399