Problem/Motivation

Getting

Call to a member function toArray() on null in Drupal\layout_paragraphs\Plugin\Field\FieldWidget\LayoutParagraphsWidget->formMultipleElements()

when trying to create a translation of a node with a layout paragraph.

There are two reasons in the LayoutParagraphsWidget.php:
1. The widget doesn't seem to sufficiently check for "isTranslatable" of paragraphs items, especially in a combination where layout paragraphs are not translatable (which makes a lot of sense, I guess) but content paragraphs are translatable

2. $paragraph vs. $item->entity vs. $entity isn't well-used in
LayoutParagraphsWidget::formMultipleElements():

[...]
if ($item->entity->hasTranslation($source_langcode)) {
  $entity = $item->entity->getTranslation($source_langcode);
}
// The paragraphs entity has no content translation source field
// if no paragraph entity field is translatable,
// even if the host is.
if ($item->entity->hasField('content_translation_source')) {
  // Initialise the translation with source language values.
  $item->entity->addTranslation($langcode, $entity->toArray());
  $translation = $item->entity->getTranslation($langcode);
  $manager = \Drupal::service('content_translation.manager');
  $manager->getTranslationMetadata($translation)
    ->setSource($item->entity->language()->getId());
}
[...]

instead $paragraph should be used consistently, as in ParagraphsWidget from Paragraphs module.

In this case it leads to a situation where the first condition is false so that $entity is not defined, but used in the second code!

Steps to reproduce

Preconditions:
Set layout paragraph translation setting to "Not applicable (zxx)" while paragraphs containing content are translatable.

Of course, the entity reference revisions "paragraphs" field is NOT translatable!

Then:

  1. Create a node with a layout paragraphs layout and a text paragraph in Language A
  2. Visit the translation page (e.g. node/XX/translations/add/A/B) to translate to language B
  3. See the error.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#12 layout_paragraphs-fix-langneutral-translation-3263594-2.0.x-12.patch4.49 KBanybody
#12 layout_paragraphs-fix-langneutral-translation-3263594-1.0.x-12.patch4.79 KBanybody
#4 layout_paragraphs_setting.JPG27.42 KBanybody

Issue fork layout_paragraphs-3263594

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

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Parent issue: » #3239818: Child paragraphs are always created in English
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Related issues: +#3259729: Fatal error saving translation when content moderation is enabled
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
StatusFileSize
new layout_paragraphs_setting.JPG27.42 KB
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Priority: Normal » Major

This is definitely major and should be ensured to be fixed in 2.x!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: Can't translate due to "Call to a member function toArray() on null in Drupal\layout_paragraphs\Plugin\Field\FieldWidget\LayoutParagraphsWidget->formMultipleElements()" error » Incorrect handling of untranslatable (layout) paragraphs in translatable environment
Status: Active » Needs review

Anybody opened merge request !63

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

MR !63 fixes the problems and cleans up the undefined variable handling against 1.0.x!

The previous code didn't handle untranslatable paragraphs correctly and tried to translate them. Now, if an untranslatable (e.g. layout) paragraph is part of the paragraphs list, it's handled and skipped correctly for translation.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Version: 1.0.x-dev » 2.0.x-dev
Status: Needs review » Needs work

Now switching to 2.0.x-dev as similar code exists in LayoutParagraphsBuilder::initTranslations() with the same problems.

Anybody opened merge request !64

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs work » Needs review

@justin2pin: I allowed myself to clean up the variable with this:

// Now we're sure it's a paragraph:
$paragraph = $item->entity;

as writing
$item->entity wasn't fun, was less verbose and had no benefit here. Hope that's OK, think it's an improvement after the
if (!empty($item->entity) && $item->entity instanceof ParagraphInterface) { check!

Furthermore please mind that this code was leading to an undefined variable under these circumstances:

// Make sure the source language version is used if available.
// Fetching the translation without this check could lead valid
// scenario to have no paragraphs items in the source version of
// to an exception.
if ($item->entity->hasTranslation($source_langcode)) {
  $entity = $item->entity->getTranslation($source_langcode);
}

and couldn't have any effect that way before how that variable was used.

This way it now makes sense for me:

// Make sure the source language version is used if available.
// Fetching the translation without this check could lead valid
// scenario to have no paragraphs items in the source version of
// to an exception.
if ($paragraph->hasTranslation($source_langcode)) {
  $paragraph = $paragraph->getTranslation($source_langcode);
}

I compared all that to the ParagraphsWidget and tested it in several scenarios. Works fine now!

I think we need tests for 2.x for the multilanguage scenario and I think MR63 should still be committed to 1.0.x-dev together with other RTBC'd issues, so that at least 1.0.x-dev has these fixed eventhough there won't be any further tagged release.

As this is a critical bug, what do you think about adding LayoutParagraphsMultilanguageTests? Should we commit this first and create a follow-up or first write the tests?

How would you test them? From the UI with FunctionalJavaScript or just as Functional tests?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
StatusFileSize
new layout_paragraphs-fix-langneutral-translation-3263594-1.0.x-12.patch4.79 KB
new layout_paragraphs-fix-langneutral-translation-3263594-2.0.x-12.patch4.49 KB

Here are the patches from MR!63 and MR!64 as files for composer patches:

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
justin2pin’s picture

justin2pin commented

@Anybody - thanks for this! I'm fine with switching $item->entity to $paragraph. I 100% agree we need test coverage for translations. I think we probably need to write those as FunctionJavascript tests.

Merging and marking as fixed - will open a separate issue for tests.

  • justin2pin committed f29bb96 on 2.0.x authored by Anybody 
    Issue #3263594: Incorrect handling of untranslatable (layout) paragraphs...
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thank you very much @justin2pin! I agree FunctionJavascript make sense to ensure it represents the real user path, language, ...

What do you think about the suggestion to commit the 1.x MR!63 to 1.x-dev and also other 1.x RTBC'd patches to keep the 1.x dev version up to date?

realityloop’s picture

realityloop commented
Status: Needs review » Fixed

should this be marked as fixed now?

Status: Fixed » Closed (fixed)

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

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Version: 2.0.x-dev » 1.0.x-dev

Perhaps we should then reopen this for 1.x?