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

Comments

dawehner created an issue. See original summary.

dawehner’s picture

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

dawehner’s picture

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

dawehner’s picture

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

Issue summary: View changes
dawehner’s picture

Issue summary: View changes
xjm’s picture

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

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: Define desired behaviour for un-translatable fields..

catch’s picture

Status: Needs work » Postponed
dawehner’s picture

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: Define desired behaviour for un-translatable fields. 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

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: Define desired behaviour for un-translatable fields. 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

@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

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.