Translatable fields with synchronization enabled should behave as untranslatable fields with respect to pending revisions

in the alternative mode we would hide (or disable?) only the form element related to the untranslatable property (the upload item in the case of an image field), while leaving the other ones (alt/title) editable

My humble opinion is that this should be the default mode. If not it will not be possible to edit alt/title attributes in a draft, you can only change them if you directly publish them. My guess is that many users will even consider that as a bug, and most users would like to be able to edit translations of alt/title attributes in drafts just like they edit any other translatable fields.

My humble opinion is that this should be the default mode.

That's out of scope for this issue. The "Hide non translatable fields on translation forms" checkbox on /admin/config/regional/content-language was already added to core in #2878556: Ensure that changes to untranslatable fields affect only one translation in pending revisions with a default state of off. But let's perhaps open a new issue to discuss the pros and cons of changing that to default to on when content_moderation is enabled?

I think for this issue, we just need to make sure that the behavior and validation that's conditional on that checkbox also applies to non-translatable properties within translatable fields.

we would hide (or disable?) only the form element related to the untranslatable property... while leaving the other ones ...editable

I agree that that would be the expected behavior for when that checkbox is checked.

However I'm not sure how easy it is to implement such a solution in a generic way.

That's worth exploring. It's a similar problem as what is solved by WidgetBaseInterface::flagErrors(), but a little different. We may need to expand that API to cover other cases of mapping properties to widget sub-elements, such as this case of the translation form needing to hide/disable some of those subelements but not others.

I'm wondering whether there are acceptable alternative solutions we could implement.

For purposes of unblocking #2860097: Ensure that content translations can be moderated independently, what if all we did was still show the full widget (with both its translatable and untranslatable properties/elements), but we add a validation error if the translator actually tries to change an untranslatable element? It's not the most ideal solution from a usability standpoint, but at least in the meantime, until we get a better UI in place, it would prevent getting into the data integrity problem reported in #2860097-153: Ensure that content translations can be moderated independently.

So #6 implements @effulgentsia's suggestion at the end of #5:

For purposes of unblocking #2860097: Ensure that content translations can be moderated independently, what if all we did was still show the full widget (with both its translatable and untranslatable properties/elements), but we add a validation error if the translator actually tries to change an untranslatable element?
[…] it would prevent getting into the data integrity problem reported in #2860097-153: Ensure that content translations can be moderated independently.

Updated the IS for that.

I would like to see a failing test-only patch. So I created that. Upon walking through that locally, I first thought that this:

  public function testPendingRevisionValidation() {
    foreach ([TRUE, FALSE] as $untranslatable_field_widget_display) {
      $this->doTestPendingRevisionValidation($untranslatable_field_widget_display);
    }
  }

meant that in one case, we expect a validation error, and in the other we don't. I was wrong. What this is testing, is that we get this validation error regardless of whether the Hide non translatable fields on translation forms (untranslatable_fields_hide) setting is enabled for a particular bundle. In other words: testing with that setting on and off ensures that this validation is error is triggered regardless of configuration; the validation error is triggered when a particular set of changes in properties is attempted.

At least, that is my understanding, with my limited background about how all this works or is supposed to work. Hopefully that helps the next reviewer.

Patch review:

1. +++ b/core/modules/content_translation/src/ExtendedFieldTranslationSynchronizerInterface.php
   @@ -0,0 +1,37 @@
   + * @internal
   ...
   +interface ExtendedFieldTranslationSynchronizerInterface extends FieldTranslationSynchronizerInterface {
   
   +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -5,11 +5,12 @@
   -class FieldTranslationSynchronizer implements FieldTranslationSynchronizerInterface {
   +class FieldTranslationSynchronizer implements ExtendedFieldTranslationSynchronizerInterface {
   
   +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
   @@ -0,0 +1,130 @@
   +    if (!$this->synchronizer instanceof ExtendedFieldTranslationSynchronizerInterface) {
   

   I was going to ask why this should be internal, and why this should be a new interface, because https://www.drupal.org/core/d8-bc-policy#interfaces allows us to update the existing interface.

   But I think I already found the answer: the validator is now able to check for this interface, and therefore this validator only has effect for this one implementation. Other implementations are allowed, but are warned that the implementation may change, since it's marked @internal. A "proper" API may follow in the future.

2. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -28,6 +29,45 @@ public function __construct(EntityManagerInterface $entityManager) {
   +    return $field_definition instanceof ThirdPartySettingsInterface &&  $field_definition->isTranslatable() &&
   +    ($translation_sync = $field_definition->getThirdPartySetting('content_translation', 'translation_sync')) ? $translation_sync : NULL;
   

   Nit: s/&& /&& /

   Also: that is one very very very long return statement. Can we split it up for legibility?

3. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraint.php
   @@ -0,0 +1,22 @@
   +  public $message = 'Non translatable field elements can only be changed when updating the current revision';
   

   Nits:
   s/Non translatable/Non-translatable/
   s/field elements/fields/ (because validators don't run only for entities changed via forms, they run for entities being changed from any place)

4. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
   @@ -0,0 +1,130 @@
   +    if ($entity->isNew() || $entity->isDefaultRevision() || !$entity->getEntityType()->isRevisionable()) {
   

   This effectively is returning early when it's not a pending revision, because we only want to protect the case of a pending revision in this validator.

   I think a comment to that effect here would go a long way in making this more understandable/maintainable in the future.

+++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraint.php
@@ -0,0 +1,22 @@
+  public $message = 'Non translatable field elements can only be changed when updating the current revision';

The validator implements what this message says. But is that what's desired? This means that I can't use content_moderation to work on a draft of a node, and while working on that draft, change the image that I want in the image field. Even when I'm editing in the entity's original language, not a translation.

This is in contrast to the EntityUntranslatableFieldsConstraint that was committed in #2878556: Ensure that changes to untranslatable fields affect only one translation in pending revisions, which says:

public $message = 'Non translatable fields can only be changed when updating the current revision or the original language.';

Seems to me that that "or the original language" is an important feature to retain, even for field elements, not just fields.

Agreed with @effulgentsia on wording. Also @timmillwood pointed out elsewhere that "Non translatable" should be "Non-translatable" I grepped core, and there are more hyphenated than non-hyphenated. Native English speaker feedback would be nice :) We'll need an issue to clean this up even after this one. (Minor).

Things that weren't clear + nits in a first review pass over this patch:

1. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -88,6 +90,24 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +      // If changes to untranslatable fields affect are configured to affect
   

   s/affect are/are/

2. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -88,6 +90,24 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +      // is pending revisions, otherwise multiple revision translations would be
   

   s/is/in/

3. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -88,6 +90,24 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +      // to make sure that we reconcile any previous changes in the default
   ...
   +        $sync_langcode = $entity->getUntranslated()->language()->getId();
   +        $force_sync = TRUE;
   

   Nit: "reconcile" vs "sync" — why not also say "sync" in the comment? Different terminology suggests different things/processes are at play here, introducing doubt when trying to understand this (at least for me).

4. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -140,6 +164,29 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +   * Ensures that field values are actually synchronized.
   

   Syncs the field values of all given columns.

5. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -140,6 +164,29 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +   * @param string $sync_langcode
   +   *   The
   

   Incomplete comment.

6. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -140,6 +164,29 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +  protected function enforceSynchronization(array &$field_values, $sync_langcode, array $columns) {
   ...
   +          foreach ($columns as $column) {
   

   Shouldn't "columns" here be "properties"?

7. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraint.php
   @@ -17,6 +17,6 @@
   -  public $message = 'Non translatable field elements can only be changed when updating the current revision';
   +  public $message = 'Non-translatable field elements can only be changed when updating the current revision or the original language.';
   

   See #8.3, second nit.

8. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
   @@ -13,7 +13,21 @@
   + *   translations, synchronized field properties can be changed only in default
   

   It says "properties" here, but "column" above. See prior point.

9. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
   @@ -13,7 +13,21 @@
   + *   the default revision translation, synchronized field properties can be
   

   "default revision translation" is hard to interpret. Is it "the default revision of a translation" or "the default translation of a revision"?

   Either of those would be clearer than what's currently there.

10. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
    @@ -13,7 +13,21 @@
    + *   reconciled when saving changes to the default translation as the new
    

    "reconciled" here again, see prior remark.

11. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
    @@ -90,7 +109,31 @@ public function validate($value, Constraint $constraint) {
    +            // We already checked the active translation above.
    

    I don't see anything about "active" translation above, only about the "default" translation. Are those things the same?

Adding credit to @matsbla for discovering the bug: thank you!

@matsbla: Can you confirm if this patch (in conjunction with #2860097: Ensure that content translations can be moderated independently) fixes everything that you discovered?

The patch looks good. But it now goes so deep into Entity/Field API land that it makes me unable to sign off on this. Hoping @effulgentsia or another Entity/Field API maintainer can RTBC this.

Really great to see so rapid fixes on this!

I tested the patches and the validation error works as expected.

However I did find another issue. The problem now is when you've enabled that checkbox ("Hide non translatable fields on translation forms") which enable you to save drafts in original language where you can change a non-translatable image file (which is great and makes sense). Once you now publish that draft it seems to me like that image file is not being synchronized. So only the original language gets that image file, whereas all the other languages will keep the former image file. The synchronization seems to work only if you publish directly, but not if you publish a draft.

Also it seems to me that when you have a pending draft where you made changes to non-translatable image files it is not possible to add drafts for translations into new languages anymore; when you do that the form is loaded with the most recent draft of the original language (containing the new not published non-translatable image file, which you're not allowed to save as a draft in translations). I'm not sure if this is by design, but for me it seem a little bit strange that I'm going to start translate from the draft in review and not the published version of the original language.

Thanks for #19! For others looking at this, here's the steps to reproduce it:

1. Install Standard Profile from 8.6.x branch tip + #2860097-157: Ensure that content translations can be moderated independently + #17.
2. Enable Content Translation and Content Moderation modules.
3. Add a 2nd language to the site.
4. Configure CT to apply to Articles and check the "Hide non translatable fields on translation forms" checkbox.
5. Configure CM's Editorial workflow to apply to Articles.
6. Create an article node with an image and publish it.
7. Create a translation of it and publish it.
8. Edit that node in its original language, change the image, and save the node as Published.
9. View the node in the 2nd language and verify that the image updated there as well.
10. Edit the node again in its original language, change the image, and save the node as Draft.
11. Edit it again in its original language, change nothing, and save it as Published.
12. View the node in the 2nd language. Expected behavior: the image should be updated to what was uploaded in step 10. Actual behavior: it's still the same as it was in step 9.

The reason for this looks to be:

+++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
@@ -92,6 +151,10 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
             $unchanged_items = $entity_unchanged->getTranslation($langcode)->get($field_name)->getValue();
             $this->synchronizeItems($values, $unchanged_items, $sync_langcode, array_keys($translations), $columns);

These lines are unchanged in the patch, but it looks like $unchanged_items is based solely on the 1st language, so in step 8 it's different than what's in $values, and therefore synchronizeItems() synchronizes that difference to the 2nd language. However, in step 11, it's the same (since step 10 and step 11 are done in two separate steps), so synchronizeItems() doesn't synchronize the new image to the 2nd language.

+++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
@@ -101,6 +164,29 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
+  protected function enforceSynchronization(array &$field_values, $sync_langcode, array $columns) {
+    $source_items = $field_values[$sync_langcode];
+    foreach ($field_values as $langcode => &$items) {
+      if ($langcode !== $sync_langcode) {
+        foreach ($items as $delta => $item) {
+          foreach ($columns as $column) {
+            $items[$delta][$column] = $source_items[$delta][$column];
+          }
+        }
+      }
+    }
+  }

I haven't tested the code path for this, but it strikes me as odd that the implementation is so different than in synchronizeItems(). Seems like unlike the existing code in synchronizeItems(), this new code is assuming that prior to synchronization all languages have the same number and order of items? My hunch is that that's not a correct assumption, but if it is, why is it?

+++ b/core/modules/content_translation/src/ExtendedFieldTranslationSynchronizerInterface.php
@@ -0,0 +1,37 @@
+interface ExtendedFieldTranslationSynchronizerInterface extends FieldTranslationSynchronizerInterface {

I don't think we should add a new interface. I think we need to add methods to FieldTranslationSynchronizerInterface itself. Our BC policy allows that. I think it's preferable for modules/sites that implement a custom synchronizer (do any actually exist?) to have PHP give them a fatal error pointing to them to needing to add the new method(s) rather than having those sites silently not having this validator run.

+++ b/core/modules/content_translation/src/ExtendedFieldTranslationSynchronizerInterface.php
@@ -0,0 +1,37 @@
+  public function getOriginalEntity(ContentEntityInterface $entity);

I don't think this belongs on this interface (or on FieldTranslationSynchronizerInterface). Perhaps we should open an issue to add this to some interface within Drupal\Core\Entity? In the meantime, could we either duplicate this as a protected method in the places that need it or add a @internal trait if we want to avoid that duplication?

+++ b/core/modules/content_translation/src/ExtendedFieldTranslationSynchronizerInterface.php
@@ -0,0 +1,37 @@
+  public function getSynchronizedFieldSettings(array $field_definitions);

Something like this probably does belong on FieldTranslationSynchronizerInterface. But can we make it a narrower API? Otherwise, it's just a dumping bag of whatever is in the config variable, which we shouldn't be leaking to callers that don't need to care about that internal structure. How about getSynchronizedProperties() or getPropertiesToSynchronize()? Passed a single field definition (not an array, because the caller can call it for each field that it's interested in separately). And where the implementation is responsible for dealing with "groups" as an internal implementation detail: the caller just gets an array of the property/column names that need synchronizing. I think for purposes of naming the method, "properties" is better than "columns", since the "column" terminology is a historical implementation artifact, not something we need to leak to the API. As a separate issue, perhaps we want to rename the $columns parameter in synchronizeItems() to $properties, since I think in PHP, renaming a parameter name is neither an API change nor a BC break?

+++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
@@ -0,0 +1,203 @@
+    // When changes to untranslatable fields are configured to affect all
+    // revision translations, we always allow changes in default revisions.
+    if ($entity->isDefaultRevision() && !$entity->isDefaultTranslationAffectedOnly()) {
+      return;
+    }

Why not allow changes to default revisions regardless of isDefaultTranslationAffectedOnly() like EntityUntranslatableFieldsConstraintValidator does?

Interdiff review:

1. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -19,55 +20,57 @@ class FieldTranslationSynchronizer implements ExtendedFieldTranslationSynchroniz
   -      return $field_definition->getThirdPartySetting('content_translation', 'translation_sync') ?: NULL;
   +      return $field_definition->getThirdPartySetting('content_translation', 'translation_sync') ?: [];
   

   Nit: can't we change this to:

   return $this->…->getThirdPartySetting(…, …, []);
   

   i.e. use the third parameter, $default, to specify the default we want?

2. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraint.php
   @@ -17,6 +17,8 @@
   +  public $defaultRevisionMessage = 'Non-translatable field elements can only be changed when updating the current revision.';
   

   Can we add a comment here to document what "elements" means in this context? @plach explained this in #14.

3. +++ b/core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php
   @@ -102,20 +100,20 @@ public function validate($value, Constraint $constraint) {
            if ($has_changes) {
   -          $this->context->addViolation($constraint->message);
   +          $this->context->addViolation($constraint->defaultTranslationMessage);
            }
   
   @@ -134,49 +132,63 @@ public function validate($value, Constraint $constraint) {
        elseif ($has_changes) {
   -      $this->context->addViolation($constraint->message);
   +      $this->context->addViolation($constraint->defaultRevisionMessage);
        }
   

   Nice! 👌 Different execution branches, different messages, makes much more sense!

(patch review)

1. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -43,21 +87,39 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +      // default revision translation, we need to skip synchronization in
   

   s/default revision translation/default translation/, because that's what \Drupal\Core\Entity\TranslatableRevisionableInterface::isDefaultTranslationAffectedOnly() is about.

2. +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
   @@ -92,6 +154,10 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +            if ($force_sync) {
   +              $this->enforceSynchronization($values, $sync_langcode, $columns);
   +            }
   
   @@ -101,6 +167,49 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
   +   * Syncs the field values of all specified properties.
   +   *
   

   @plach just explained to me in a call why this is A) a separate method (i.e. not part of synchronizeItems()), B) why its code can be so much simpler (than synchronizeItems()).

   The short version of A is: because changing synchronizeItems() is relatively risky (despite lots of existing test coverage).

   The short version of B is: precisely because this runs after synchronizeItems(), a lot of the preparation work in terms of ordering and ensuring a clean data structure is already done.

   This should be documented, probably on the docblock for enforceSynchronization().

@plach asked me to review, so I did my best … but I fear that my lack of deep knowledge about the pre-existing architecture is getting in the way.

#24

• #19/#20 fix: I see you added explicit test coverage, excellent!
• #20: so you're referring to the fix, which is this change:
  

  +++ b/core/modules/content_translation/src/FieldTranslationSynchronizer.php
  @@ -99,10 +101,11 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
  -      // When saving a default revision for a non-default translation, we need
  -      // to make sure that we enforce synchronization of any previous changes in
  -      // the default translation.
  -      elseif (!$entity->isDefaultTranslation()) {
  +      // When saving a default revision, we need to make sure that we enforce
  +      // synchronization of any previous changes in the default translation,
  +      // since a pending revision may have desynchronized field values, even if
  +      // there is no change between the current and the previous revision.
  +      else {
  

  I think the comment here could be clearer, but maybe that's just me not having enough familiarity with this area.

• #21: @plach also explained this to me in a call, thanks to which I was able to understand this mostly (intuitively rather than exactly), this is why I asked for an explanation in protected function enforceSynchronizatino()'s docblock, at the very end of my previous comment.
• #22: 👍
• #23: We probably need a follow-up to fix EntityUntranslatableFieldsConstraintValidator as well, although it's probably not super urgent. Can we get a follow-up for this?

Hopefully @plach's comment 24 (which addressed @effulgentsia's feedback) and my feedback in this comment and my previous comment will result in one more reroll by @plach that will allow @effulgentsia to RTBC!

#33 is looking good to me. I just want to add/change a few code comments. I'll post that within the next few hours, but since it'll just be comments, it won't affect any behavior. @matsbla: if you're around today to test #33, it would be great to get your feedback on it!

I made a quick check now and to me it looks like patch in #33 effectively solve this issue.

Just one thing that I think is a little bit strange. but maybe this is by design;
1. I have unchecked the box "Hide non translatable fields on translation forms"
2. I publish a node with image
3. start make a draft in a second language
4. update and publish a new image in the original language
5. publish the draft of the second language (with the old image)

By publishing that draft I now revert changes made in step 4. It means after a new image is published the same image must be uploaded to every pending revision / draft that was made before, if not publishing each of them will revert the new image.
In one way it makes sense as this mode allow all languages to modify those non-translatable properties. But in another way it would also make sense if new images that were published where synchronized directly to all pending revisions.

In the other mode, when you have that checkbox checked, the new image is persevered, but you only see it once you publish the draft. When you edit the draft or look at the preview, you see the old image. You see the new image first when you publish the draft. What if I were suppose to update the image text? It could have made sense to see the new image there before you publish the pending revision.

Anyway these are just some observations, and I think this patch solve the most important issues.
Great work!

@plach in #29: MUCH clearer!

@matsbla in #35: thanks for confirming that this solved the problem! I don't know how this is meant to work, so I can't add nuance to the new observation you posted. Thank you very much for your continued diligence! :) 👌

Looks like it is working fine now, nice!

I would still like to improve/add some code comments per #34, which I didn't get to yesterday due to looking more into #35 (thanks again for that great find, @matsbla) with @plach. However, I woke up today not feeling very well, so am not yet sure if I'll be able to work on those docs/comments today. Meanwhile, beta is approaching, and getting this,
#2860097: Ensure that content translations can be moderated independently, and #2936511: ContentEntityType::getRevisionMetadataKeys() never considers the BC logic because of the new "revision_default" key in by then would be super awesome, whereas docs improvements can happen after beta if necessary. Therefore, since I'm happy with all of the code itself in #37, to keep things moving, RTBC!

+++ b/core/modules/content_translation/src/FieldTranslationSynchronizerInterface.php
@@ -54,4 +55,15 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
+  public function getFieldSynchronizedProperties(FieldDefinitionInterface $field_definition);

This needs a change record.

Looks good to me too, committed to both 8.6 and 8.5. There was this little code style problems that I fixed on commit:

FILE: ...modules/content_translation_test/content_translation_test.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 33 | ERROR | [x] Concat operator must be surrounded by a single
    |       |     space
----------------------------------------------------------------------

Publishing change record.

This is failing on 8.5.x:
https://www.drupal.org/pift-ci-job/878041
https://www.drupal.org/pift-ci-job/878043
https://www.drupal.org/pift-ci-job/878038

Drupal\Tests\content_translation\Kernel\ContentTranslationFieldSyncRevisionTest::testFieldSynchronizationAndValidation
The following module is missing from the file system: simpletest

/var/www/html/vendor/symfony/phpunit-bridge/DeprecationErrorHandler.php:270
/var/www/html/core/includes/bootstrap.inc:268
/var/www/html/core/includes/bootstrap.inc:291
/var/www/html/core/tests/Drupal/Tests/TestFileCreationTrait.php:71
/var/www/html/core/modules/content_translation/tests/src/Kernel/ContentTranslationFieldSyncRevisionTest.php:100

@tacituseu: thanks for your diligence!

@plach's explanation in #48 makes sense.

ContentTranslationFieldSyncRevisionTest() passed on 8.5.x, moving back to RTBC!

Thanks everyone, committed again. We should really make the extension discovery behavior harmonized between the two branches because this will cause problems. This is the second such issue since we opened 8.6.

Some users are reporting that this validator is causing exceptions when changing the default language of an entity. Issue here: #2995655: Invalid translation language (und) specified. in Drupal\Core\Entity\ContentEntityBase when changing language of an entity.