Configurable field translatability is not properly switched [#2148795]

Problem/Motivation

Steps to reproduce:

- install Drupal
- enable Content translation (will enable Language module)
- add a language or two
- go to content translation settings under regional and language
- enable articles for translation with all defaults left intact (all fields except image files)
- go create an article
- translate the article to any other language (NOTE: the translated body will overwrite the original body)
- translate the article to one other language (NOTE: the translated body will again overwrite the original body)

Now you ended up with two translations of a node with all node views showing the last body value entered. If you look in the DB, the language code of the body field is the original language code of the node but the value is the last one entered in the last translation of the node. If you look into the configuration, the body field is marked as translatable (from content_translation.settings.yml):

...
node:
  article:
    content_translation:
      enabled: true
      fields:
        title: true
        body: true
        comment: true
        field_image: true
        field_tags: true
...

Note the same problem does not apply to the tags field or the base fields (practically: title).

Proposed resolution

Make body properly translate again. Write tests.

Remaining tasks

Fix.
Write tests.
Review.

User interface changes

None.

API changes

None.

Comment	File	Size	Author
#13	ct-settings_broken-2148795-13.patch	6.22 KB	Gábor Hojtsy
#13
#6	ct-settings_broken-2148795-4-fail.patch	2.87 KB	plach
#6
#4	ct-settings_broken-2148795-4-fail.patch	2.87 KB	plach
#4
#4	ct-settings_broken-2148795-4.patch	6.18 KB	plach
#4

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 3 December 2013 at 12:59

Title:

Body is not made multilingual (even those other fields are)

» Body is not made multilingual (even though other fields are)

Comment #2

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 3 December 2013 at 18:16

Issue tags:

+Needs tests

Let's get some tests for this.

Comment #3

plach

he/him

Italian

Venezia

CreditAttribution: plach commented 3 December 2013 at 23:32

Assigned:

Unassigned

» plach

On this

Comment #4

plach

he/him

Italian

Venezia

CreditAttribution: plach commented 4 December 2013 at 01:47

Title:	Body is not made multilingual (even though other fields are)	» Configurable field translatability is not properly switched
Assigned:	plach	» Unassigned
Status:	Active	» Needs review

File	Size
ct-settings_broken-2148795-4.patch	6.18 KB

ct-settings_broken-2148795-4-fail.patch	2.87 KB

The problem is quite simple: we were not correctly handling configurable field translatability changes. This seems to happen only to node bodies because in the OP use case we have two bundles, but tags are attached only to articles. When saving the new settings the translatability status was saved for each bundle and the last one prevaled. Instead we need to save one global value for all field instances. As a bonus while testing this I found an additional bug in the Field UI field settings page and fixed that one too. Tests cover both.

Comment #5

plach

he/him

Italian

Venezia

CreditAttribution: plach commented 4 December 2013 at 01:48

Issue tags:

-Needs tests

Comment #6

plach

he/him

Italian

Venezia

CreditAttribution: plach commented 4 December 2013 at 01:58

File	Size
ct-settings_broken-2148795-4-fail.patch	2.87 KB

Re-uploading the test only patch (which is supposed to fail)...

Comment #7

4 December 2013 at 03:26

Status:

Needs review

» Needs work

The last submitted patch, 6: ct-settings_broken-2148795-4-fail.patch, failed testing.

Comment #8

swentel CreditAttribution: swentel commented 4 December 2013 at 08:51

@plach re: the Field UI bug you noticed, is that related with #1831608: Show or hide the "Make field translatable" checkbox on the add field form depending on translatability also ? Been trying to reroll that one yesterday but had weird test failures .. on the body .. :)

Comment #9

plach

he/him

Italian

Venezia

CreditAttribution: plach commented 4 December 2013 at 10:20

@swentel:

The body test failures might be related to the original issue here :) Not sure about the Field UI bug: we were not updating the CT settings, which will be the only one used to track translatability once we are done with the Field API refactorings.

Comment #10

plach

he/him

Italian

Venezia

CreditAttribution: plach commented 4 December 2013 at 10:11

Status:

Needs work

» Needs review

Comment #11

plach

he/him

Italian

Venezia

CreditAttribution: plach commented 4 December 2013 at 10:20

Ok, we should be good here. Patch to review/commit is #4.

Comment #12

amateescu CreditAttribution: amateescu commented 4 December 2013 at 10:57

+++ a/core/modules/content_translation/content_translation.admin.inc
@@ -333,24 +333,32 @@ function content_translation_form_language_content_settings_submit(array $form,
+  // @todo Remove this once configurable fields rely on entity field info to
+  //   track translatability.

Is there an existing issue we can point to here?

Comment #13

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 4 December 2013 at 12:58

Status:

Needs review

» Reviewed & tested by the community

File	Size
ct-settings_broken-2148795-13.patch	6.22 KB

The patch looks complete. It has solutions for both the global setup form and the field level setup form. I also verified that the patch works with a simplytest.me instance (set up an article and verified body translatability). Uploading a minimally modified version which qualifies that @todo with a link to #2018685: Remove field_is_translatable() which I believe is the best place to link this. I think this should be ready to go.