Problem/Motivation

While working on #2315773: Create a menu link field type/widget/formatter the following problem was observed:

  • Added a content type with a translatable field
  • Mark the "menu parent" column as non translatable
  • Added a new content with setting the menu parent to "" (aka. put it below home)
  • Translate the content to the other language
  • Expected behaviour: The translation can be used as expected. The original translation will be untouched.
  • Actual behaviour: The original field item is dropped

Critical vs. Major

You probably don't run into that issue with core fields, even image or field item could be problematic here.

Proposed resolution

Fix the field synchronization part.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 2861367-2.patch2.71 KBdawehner
#2 2861367-test.patch1.74 KBdawehner

Comments

dawehner created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 2861367-test.patch1.74 KB
new 2861367-2.patch2.71 KB

Here is a test + a fix. Not sure whether this is the best approach.

dawehner’s picture

dawehner
Primary language German
commented

I'd argue this is borderline critical. I saw data being lost, even it might be an edge case.

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review

The last submitted patch, 2: 2861367-test.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: 2861367-2.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Major » Critical

As this is a data loss issue, let's raise this to critical for now even if it is edgecase-y.

catch’s picture

catch
he/him
Primary language English
commented

Discussed this on the committer triage call, two main questions:

1. As with #2766957: Forward revisions + translation UI can result in forked draft revisions I'm left asking why we allow translators to edit non-translatable fields in the first place - that's not really translation.

2. Shouldn't emptying a field remove the contents? Isn't that what happens when you aren't translating?

I've opened a new issue to discuss this, I'm not sure we can do anything here without resolving that one, so postponing for now #2878556: Ensure that changes to untranslatable fields affect only one translation in pending revisions.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Postponed
dawehner’s picture

dawehner
Primary language German
commented

I'm left asking why we allow translators to edit non-translatable fields in the first place

I think its more tricky, fields are partially translatable. In my case it was the title/description of the menu being translatable, but the menu parent not. I don't think any of the decisions in #2878556: Ensure that changes to untranslatable fields affect only one translation in pending revisions would change this decision.

2. Shouldn't emptying a field remove the contents? Isn't that what happens when you aren't translating?

Let me quote the issue summary:

Mark the "menu parent" column as non translatable

This is not an empty field value, its the root of a menu, and as such a total legitimate value.

catch’s picture

catch
he/him
Primary language English
commented

I think its more tricky, fields are partially translatable. In my case it was the title/description of the menu being translatable, but the menu parent not. I don't think any of the decisions in #2878556: Define desired behaviour for un-translatable fields. would change this decision.

OK so in that case, the form field for the menu parent should not be editable via the translation form, and the form fields for the columns that are translatable should? That still feels like the same problem as #2878556: Ensure that changes to untranslatable fields affect only one translation in pending revisions just we'd need to disable things at the form field/column level rather than/as well as at the field API field level.

dawehner’s picture

dawehner
Primary language German
commented

@catch
I guess so, but even then, if you start with an empty string (root parent), go to the translation page, no matter whether the form is disabled or not, the field sync process will run and detect the empty string. Maybe i'm stubborn, but I don't see how this is not a bug in content_translation.

catch’s picture

catch
he/him
Primary language English
commented
Status: Postponed » Active

So that's interesting but I don't see how the translation sync process can distinguish between an empty string in a text field vs. an empty string in a menu parent, should the menu be storing 0 instead of ''?

Let's re-open though, sounds like it might be fixable at least partly without doing the other issue.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I suspect this is related to the use/logic of \Drupal\Core\Field\FieldItemListInterface::equals().

dawehner’s picture

dawehner
Primary language German
commented

@Wim Leers
I think the reason why \Drupal\Core\Field\FieldItemListInterface::equals() might not be used here is because it needs more logic, due to the really obscure feature of translation groups, which allows to have parts of a field translatable and some others not. To be honest though, it took me a while to remotely get what was going on in this subsystem.

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

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Triaged D8 critical

@cilefen, @Cottser, @effulgentsia, and I discussed this issue and agreed it is indeed critical. The tests are excellent and demonstrate the data loss problem pretty clearly. Thanks!

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

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Assigned: Unassigned » plach

I'll have a look at this soonish.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). 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: 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.

stephencamilo’s picture

stephencamilo commented
Status: Active » Closed (won't fix)
matsbla’s picture

matsbla commented
Status: Closed (won't fix) » Active

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.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: -Triaged D8 critical +Triaged core critical

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.

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.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.