Wrong language handling in ImageFormatter when linking to entity

Hi @bucefal91,
this patch should fix the problem, so let me know.

@bucefal91: Judging from the issue summary, you are pretty D8-savvy, I'd say! This is absolutely a legitimate bug.

I'm pretty sure there's either already an issue about this somewhere or at least a very similar one, but I can't find it right now.

@aronne: I think that's the right spot to put the language. However, we shouldn't use the interface language there. Instead the passed-in $langcode variable should be used. You can use the language manager's getLanguage() method to get the respective language object.

@tstoeckler: ok i've updated the patch by replacing the getCurrentLanguage() method with the one you've suggested getLanguage($langcode).

Let me know if that's what you want or if you would replace, for some reason, even the $this->languageManager call with a \Drupal::languageManager().

No, the patch looks perfect. Thanks! I'm afraid we need some tests, though, before this can get in.

Also, not sure why the testbot isn't picking up the existing patches. @aronne can you try uploading the file in #4 with a different filename, maybe without the dots (".") in the filename?

@tstoeckler tests are not running because the patch i've made is for 8.1 version, but during file upload the starting testable version was 8.2 so i didn't enable tests for these patches.

Ahh, I see. Unfortunately this won't go into 8.1.x anymore, as that is basically frozen (barring security issues). So we'll have to go for 8.2.x for this. I presume there's not that much of a difference in the code, though, so maybe you can just re-upload the patch and enable 8.2.x testing, to see.

Ok here it is the new patch for 8.2.x version.
Let's see what tests say :)

Awesome, so we know it doesn't break anything. Now we just need some additional tests for the bug. Marking needs work for that.

@bucefal91 awesome work :)

@bucefal91: Wow, very impressive work. Also thank you a lot for documenting the steps that you took and the things you struggled with. Not only can it help to inform future DX decisions, but it also encourages other contributors who might be struggling with the same issues. Not many people do this, I really appreciate it!

Some notes on the patch. Only the last part actually warrants a "needs work" from my point of view, but pointing out a couple of other minor things anyway.

   /**
+   * @var \Drupal\language\Entity\ConfigurableLanguage
+   */
+  protected $configurableLanguage;

1. It would be nice to add a little comment like "The language used in the test." above the @var declaration.
2. Minor, but I think it's fine for the variable to be named $language.

-    $this->display = entity_get_display($this->entityType, $this->bundle, 'default')
+    $this->display = $this->container->get('entity_type.manager')

Yay!!

+  /**
+   * Tests the link to entity from image formatters.
+   */

I think that sentence is a little bit unclear. Maybe "Tests the entity link of the image formatter."

+    $build = $this->display->build($entity);
+    $this->assertEquals($entity->language()->getId(), $build[$this->fieldName][0]['#url']->getOption('language')->getId(), 'Image is linked to the entity in the appropriate language.');

I think this test is a little bit too close to the actual code that it tests. I think it would be nicer to render the formatter, for example using $output = $this->container->get('renderer)->renderRoot($build[$this->fieldName][0]); and then assert that the correct URL is contained in the output.

Re-titling and adding related issue.

That issue has a patch which solves the same issue (for a different formatter) but in a slightly different way. I think we should make that consistent, although I'm not sure which approach is better. I'm leaning slightly to the approach there, but not sure.

Patch needed a re-roll. I also created a version of this patch for Responsive image here: #2889892: Responsive image field formatter links to the wrong entity translation

Sorry, ignore that last patch. Here's a working one.

Thanks for the patches, I had also noticed this issue in ResponsiveImageFormatter.

Since ResponsiveImageFormatter is using the exact same code, shouldn't we be using a private helper to get the translation to avoid code duplication?

The patch at https://www.drupal.org/project/drupal/issues/2883450#comment-12116998 fixes the problem for me, but takes a better approach in my opinion.

The processOutbound method in the LanguageNegotiationUrl plugin can add the current language when the content has language not specified or not applicable.
That means the rewriting is correct, and not every formatter needs to be rewritten.

I like the solution provided in #22 and it works well with Drupal 8.5.1. And I would like to see this fix or an other solution in one of the next releases too.

I adapt this solution to the MediaThumbnailFormatter too. May there is a more generic approach to handle all kind of similar issues.

I'm still not sure whether this patch is the right approach or whether the solution from #2648288: StringFormatter generates links in wrong language when linking to entity is better.

Maybe we can get the opinion of another entity API maintainer.

To recap, the question is:

If a langcode is passed explicitly into FieldWidgetInterface::view() what should be the behavior when generating links to entities?
A) Just pass the language as a URL option. This has the downside that even if there is a translation in that language the untranslated label will be used, not the translated one.

B) If there is a translation in that language, just fetch that and use it to build the URL. This has the upside of using the translated language and not having to muck with the language manager manually, but the downside is that if there isn't a translation in that language but people might still want to have the language in the URL object.

Having written this, maybe the correct solution is to do to both? I.e. fetch a translation if it exists, but unconditionally add the language to the URL?

Re-rolled for 8.6

I've noticed this for multiple formatters, including ImageFormatter, ResponsiveImageFormatter and StringFormatter. Its quite tedious to update all formatters and the root of the issue seems to come from FieldItemList "forgetting" that its parent entity is a translation for untranslatable fields.

As a completely different approach, how about we ensure that the FielditemList passed to formatters always retains its translation? That way we won't have to update various (core/contrib/custom) formatters, and the existing behavior for FieldItemList and ::toUrl etc. can remain unchanged. I've attached a tiny patch that does this, looking forwards to your feedback. (Still needs tests, though, if the apporach is okay'ed)

#33 breaks search_api on the following condition:

• A Rendered HTML output is added to the index.
• The target view mode has at least one entity reference field formatter.

$items->getParent()->setValue($translation); will force all translations for a given entity to be indexed on the same language.

Regarding #36 I'll set it to "Needs work". Can still reproduce that image links to content do not respect language in latest D9 stable test enviroment for content rendered via manage display view modes, while linked titles do. But I am not sure if we need more reproducing tests in the new Drupal 9 eco system regarding HOW the image has been implemented. Is is a media reference image?(EDIT: cannot link to content) Or an image field. Are they rendered via Layout Builder or directly or via Display Suite? I actually would consider this even as a "Major" prio but leave it as is for now.

Rerolled patch against 9.4x. please review it

Thanks for working on this. But again, I recommend strongly to ask for more user tests to reproduce this issue. Because it is not clear enough now in D9 cycle IF it still exist, and IF from where it is caused. We need more reproducing reports on this here, imho. Would love to read from more users how they run into this issue in details.

This has also happened to me in a Drupal 10.4.4.
Tested it at https://simplytest.me/

Steps to reproduce the error:

1. Add a language to the web, in this case it will be English (default) and Spanish.
2. In the language configuration, indicate that in English the prefix will be “en” and in Spanish it will be “es” (admin/config/regional/language/detection/url)
3. Using the content type “basic page”, mark it as translatable and add an image media type field (not translatable)
4. Mark as translatable also the media type “image”.
5. Create 2 nodes in English, add an image to them and translate them into Spanish.

Now comes the problem:

1. Create a view that shows the content type “basic page”.
2. Add the image field with the format “thumbnail (important, with the other formats it works fine)”.
3. Check the “link to content” option in the image field.
4. When checking the English view, both images point to the corresponding nodes in English:
1. https://master-teaibnpl96czl1tjspp5qwvhrsw7quru.tugboatqa.com/en/node/1
2. https://master-teaibnpl96czl1tjspp5qwvhrsw7quru.tugboatqa.com/en/node/2
5. But if we switch to Spanish, the 2 images point to the English node.
1. https://master-teaibnpl96czl1tjspp5qwvhrsw7quru.tugboatqa.com/en/node/1 (should be /es/node/1)
2. https://master-teaibnpl96czl1tjspp5qwvhrsw7quru.tugboatqa.com/en/node/2 (should be /es/node/2)



So the problem seems to be that when selecting the thumbnail formatter, it is not picking up the correct language of the content it has to link to.

Improved Entity Translation Handling

In Drupal 8.1, entity translation handling was still evolving. The language negotiation and rendering system had some inconsistencies in cases where an untranslatable field (image) was being used in translated content.
In Drupal 11.x, the entity rendering system has improved, ensuring that links generated in untranslated fields respect the active page language.
Changes in ImageFormatter::viewElements()

The issue in Drupal 8.1 was that viewElements() used LANGCODE_DEFAULT, which caused links to always point to the original entity language.
In Drupal 11.x, the rendering correctly respects the current entity translation.
Field Translation System Fixes

Earlier Drupal versions required making the image field translatable as a workaround.
In Drupal 11.x, this is no longer required because entity rendering now properly resolves translation context for non-translatable fields.