Mapped media fields are overridden with metadata on translation save

Created on 13 September 2018, over 5 years ago
Updated 6 February 2024, 4 months ago

A media title is overridden with filename after saving of media translation with different image for target language

To reproduce:

  • Install Drupal 8.6.0 in English
  • Enable these modules: Core: Media, Multilingual: Content Translation
  • Add a language ( /admin/config/regional/language )
  • Check the 'Media' custom language settings in /admin/config/regional/content-language and make image media type translatable (translation for Image field should be enabled too).
  • Add an image (/media/add/image) and translate it: upload different image and change title

Actual result: media title is overridden with file name in translation.

I guess there is mistake in Media::prepareSave() method:

        foreach ($translation->bundle->entity->getFieldMap() as $metadata_attribute_name => $entity_field_name) {
          // Only save value in entity field if empty. Do not overwrite existing
          // data.
          if ($translation->hasField($entity_field_name) && ($translation->get($entity_field_name)->isEmpty() || $translation->hasSourceFieldChanged())) {
            $translation->set($entity_field_name, $media_source->getMetadata($translation, $metadata_attribute_name));
          }
        }

Looks like current condtition is not OK, and OR condition should be replaced with AND condition:

if ($translation->hasField($entity_field_name) && ($translation->get($entity_field_name)->isEmpty() && $translation->hasSourceFieldChanged())) {

In this way title will be overridden only in case when it is empty.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Media 

Last updated 1 day ago

Created by

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇨🇭Switzerland ayalon

    I have tested patch #62 with Drupal 9.5.3 and it successfully resolves the issue, that you cannot change the name ov a previously saved media entity translation.

  • Status changed to Needs work 12 months ago
  • 🇺🇸United States ndewhurst USA

    The patch in #62 prevents the specific problem outlined in the description, but it introduces a regression. Specifically, metadata will never be mapped to entity fields, even if those fields are empty, unless the source field changes. This does not appear to match the original intent here.
    It might not be a noticeable issue for most people because most metadata won't change unless the media source changes - e.g. if you edit an Image-sourced media entity and replace the image file. However, it's perfectly valid for there to be all sorts of other metadata that might be updated somewhere without the media source changing - e.g. a transcript on an externally-hosted video, a caption for an image stored in an external DAM, etc.
    I wonder if we could restate the problem by defining the expected behavior as:

    Available metadata should always be used to populate mapped fields when media entities are saved/resaved and the target fields are empty. When changing a media source field, we should expect any mapped fields to be updated with metadata-derived values, unless we are simultaneously providing alternative values for some of those fields.

    I know it doesn't avoid the "I previously customized the media name, and I was hoping my custom name would persist after replacing the source file" scenario...but it seems to balance the concerns in this thread with the original intent.
    Here's a patch that tries to put the aforementioned behavior into practice. If someone is able to look at tests for it or otherwise weigh in, that would be cool.

  • 🇺🇸United States ndewhurst USA

    Updating patch to work with new media entity creation, improve empty value handling, and revise comments.

  • Status changed to Needs review 11 months ago
  • last update 11 months ago
    29,787 pass, 6 fail
  • 🇮🇳India Ajeet Tiwari

    Please review.

  • Status changed to Needs work 11 months ago
  • 🇺🇸United States ndewhurst USA
    +++ b/core/modules/media/src/Entity/Media.php
    @@ -433,7 +432,7 @@ public function prepareSave() {
    -          if ($translation->hasField($entity_field_name) && ($translation->get($entity_field_name)->isEmpty() || $translation->hasSourceFieldChanged())) {
    +          if ($translation->hasField($entity_field_name) && ($translation->get($entity_field_name)->isEmpty() && $translation->hasSourceFieldChanged())) {
    

    @Ajeet Tiwari - this proposal is the same as in #2 🐛 Mapped media fields are overridden with metadata on translation save Needs work . I don't believe this is good, because mapped fields that are empty should be eligible for population regardless of whether the source field is changing.

  • 🇭🇺Hungary HAL 9000

    Hi, 62's patch has been re-rolled to work with 10.2.

  • last update about 2 months ago
    Patch Failed to Apply
Production build 0.69.0 2024