Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I believe there is a regression with Entity Translation (Alt + Title) Fields in 2.0-beta7.
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)
Comment | File | Size | Author |
---|---|---|---|
#7 | regression_with_entity-2828856-7.patch | 1.85 KB | sylus |
| |||
#6 | regression_file_entity_et.png | 72.6 KB | sylus |
Comments
Comment #2
sylus CreditAttribution: sylus commentedComment #3
sylus CreditAttribution: sylus commentedI will take a look at this further over the weekend and try to locate the exact line / cause of issue related to commit mentioned.
Comment #4
joseph.olstadafter 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 modehope that helps, really want this module to get to stable soon.
Comment #5
joseph.olstadComment #6
sylus CreditAttribution: sylus commentedOk 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()
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'
Comment #7
sylus CreditAttribution: sylus commentedI 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 ^_^
Comment #8
joseph.olstad@sylus, right on good work , so can you confirm if this also passes your behat tests?
Comment #9
joseph.olstadLooked 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...
Comment #10
joseph.olstadComment #12
joseph.olstadfixed and thanks again @sylus