I believe there is a regression with Entity Translation (Alt + Title) Fields in 2.0-beta7.

#2062721: Add a white list of file fields that can be overwritten when the file is added in the wysiwyg

This patch referenced above is causing my Behat tests checking for the alt attribute on the French Side of the page to no longer work. I have traced it to the following commit, as one commit prior (2.0-beta6) is working as expected.

http://cgit.drupalcode.org/media/commit/?id=7296e287cf3a5900c157743e29c2...

Here is my Behat output showing english alt works but the french no longer does:

    Given I am on "en/content/drupal-wxt"                                                                # Drupal\DrupalExtension\Context\MinkContext::visit()
    Then I should see the image alt "Web Experience Toolkit Logo" in the "Content Well" region           # TestSubContext::assertAltRegion()
    And I am on "fr/contenu/wxt-drupal"                                                                  # Drupal\DrupalExtension\Context\MinkContext::visit()
    Then I should see the image alt "Boîte à outils de expérience Web Logo" in the "Content Well" region # TestSubContext::assertAltRegion()
      No alt text matching "Boîte à outils de expérience Web Logo" in the "Content Well" region on the page http://127.0.0.1:8888/fr/contenu/wxt-drupal (Exception)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sylus created an issue. See original summary.

sylus’s picture

sylus’s picture

I will take a look at this further over the weekend and try to locate the exact line / cause of issue related to commit mentioned.

joseph.olstad’s picture

after looking at that commit you mentioned:
Suggest zero'ing in on this function:

function media_wysiwyg_get_file_without_label($file, $view_mode, $settings = array(), $langcode = NULL) {

of this file modules/media_wysiwyg/includes/media_wysiwyg.pages.inc

and these two lines of code inside that function

if (isset($settings['fields'][$field_name][$langcode]['value'])) {

and:

if (isset($settings['fields'][$field_name][$langcode][0]['value'])) {

curious to see what the '$settings' variable looks like in a dpm() when you're in french mode

hope that helps, really want this module to get to stable soon.

joseph.olstad’s picture

Status: Active » Needs work
sylus’s picture

Ok I think I found the problem, below is an example of one of my file entities:

In the commit reference the following logic was added to media_wysiwyg_get_file_without_label()

  $alt = variable_get('file_entity_alt', '[file:field_file_image_alt_text]');
  $title = variable_get('file_entity_title', '[file:field_file_image_title_text]');
  $overrides = array(
    'alt' => $alt,
    'title' => $title,
  );
  foreach ($overrides as $field_type => $field_name) {
    preg_match($pattern, $field_name, $matches);
    if (!empty($matches[2])) {
      $field_name = $matches[2];
      $langcode = field_language('file', $file, $field_name);
      if (isset($settings['fields'][$field_name][$langcode]['value'])) {
        if (empty($settings['attributes'][$field_type])) {
          $settings['attributes'][$field_type] = $settings['fields'][$field_name][$langcode]['value'];
        }
      }
      if (isset($settings['fields'][$field_name][$langcode][0]['value'])) {
        if (empty($settings['attributes'][$field_type])) {
          $settings['attributes'][$field_type] = $settings['fields'][$field_name][$langcode][0]['value'];
        }
      }
    }
  }

The $langcode = field_language('file', $file, $field_name); line in particular is causing the issue since title_text gets called last in the for loop and doesn't have any data entered so the langcode will be set to 'und' for both alt + title text once the for loop is completed.

This is why I am losing my french alt text when viewing a french page as it defaults to 'en' when given 'und'

sylus’s picture

Status: Needs work » Needs review
FileSize
1.85 KB

I kept the langcode assignment local to the for loop so only the $settings array gets adjusted and the default lang_code from the function param persists.

I think this is the correct way but will need some review ^_^

joseph.olstad’s picture

@sylus, right on good work , so can you confirm if this also passes your behat tests?

joseph.olstad’s picture

Looked into creating a test case for this, it would be easiest to leverage a lot of the test functions already in 'entity_translation' module tests, add them into media_wysiwyg.test to be able to create a file with field translation turned on.

However I'd like to defer writing the tests right now and just get the patch in for expediency sake, as it looks to be the correct solution and its needed.

So, I'm going to commit the patch, but create a followup ticket to create the test.

here's the test code that could be leveraged:
http://cgit.drupalcode.org/entity_translation/tree/tests/entity_translat...

joseph.olstad’s picture

  • joseph.olstad committed 874cb08 on 7.x-2.x authored by sylus
    Issue #2828856 by sylus: Regression with Entity Translation (Alt + Title...
joseph.olstad’s picture

Status: Needs review » Fixed

fixed and thanks again @sylus

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.