ALT text and TITLe text not showing on images

I too would like to see this fixed. It seems there might be some complexity UI-wise? Should the site administrator be able to choose which field to use as the alt, title, description, etc? What is the plan for this?

There already token based configuration that makes the title/alt fields available when using the normal image formatter.

We need to find a way to inject that back into the special image formatter that we use to display the image.

Could we add two configuration variables to the formatter that would define which fields to use as a source for this information (and default to auto-created alt and title fields)?

Another related problem is if image field is being used. Content creators can add alt and title as part of it, which will be completely ignored if image is displayed as a rendered entity. This will be even trickier to resolve. Out of this issue's scope I think.

See file_entity_file_load(), we used to have settings for this, we probably just need to make those configurable again on a global settings page and make sure they are also applied in \Drupal\file_entity\Plugin\Field\FieldFormatter\FileImageFormatter somehow.

Pseudo-fields that are added in file_entity_file_load() doesn't seem to be used at all. I think that making this configurable on the formatter level is the simplest and most self-documenting.

Maybe we should even consider removing code from file_entity_file_load()?

We still need the part that loads metadata I think, although we could also consider to change that to do lazy-loading as we have methods for it now. But then we'd need to change how we store it as well.

The alt/title stuff can possibly go away yes. But as I wrote, formatter level isn't that easy, actually, because it is not our formatter. We'd have to replace the core image formatter or provide our own and then people have to use that one.

The one that we have is only for displaying ourself, not for image fields. There it is indeed pretty easy.

Handling metadata through methods would be nice. Separate issue?

It seems that we have few possible situations:

1. Image field, image formatter: Formatter will use alt/title form the field and ignore the ones that are set on the file entity.
2. Image field, rendered entity formatter: alt/title from the entity will be used (assuming we fix Drupal\file_entity\Plugin\Field\FieldFormatter\FileImageFormatter to handle that correctly), alt/title from the field will be ignored
3. File or ER field, rendered entity formatter: alt/title form the entity will be used (assuming we fix the formatter)
4. File or ER field, image formatter (with help of file_image_formatters or similar module): alt/title is ignored entirely?

It seems that we could make cases 2. and 3. work by fixing our image formatter. 1. is a bit more tricky as we're dealing with the core formatter. I am inclined to override it to prevent confusion with many similar formatters. But we have another problem there, which is a big UX mess in my opinion. How do we sync alt/title between image field and file entity? Do we try to sync? What happens if data was changed on both sides after sync? Should we override the field widget too and pass alt/title from the field to the entity? I am not sure....

I'd propose that we fix 2. and 3. as part of this issue and open separate issue(s) for the other problem.

Here the initial patch ready for review.

This should fix the tests.

Looks ok, just few small notices:

1. +++ b/config/schema/file_entity.schema.yml
   @@ -48,7 +48,21 @@ file_entity.settings:
   +  type: mapping
   +  label: 'Image formatter for file entity settings'
   +  mapping:
   +    title_text:
   +      type: string
   +      label: 'Title text'
   +    alt_text:
   +      type: string
   +      label: 'Alt text'
   +    image_style:
   +      type: string
   +      label: 'Image style'
   +    image_link:
   +      type: string
   +      label: 'Link to image'
   

   This is ok, but check if we can just extend this default image settings with new fields?

2. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -75,6 +99,30 @@ class FileImageFormatter extends ImageFormatter {
   +    $available_fields = \Drupal::service('entity_field.manager')->getFieldDefinitions(
   +      $form['#entity_type'],
   +      $form['#bundle']
   +    );
   +    $options = [];
   +    foreach ($available_fields as $field_name => $field_definition) {
   +      if ($field_definition instanceof FieldConfigInterface && $field_definition->getType() == 'string') {
   +        $options[$field_name] = $field_definition->label();
   +      }
   +    }
   +    $element['title_text'] = [
   +      '#title' => $this->t('Image title text'),
   +      '#description' => $this->t('The field that is used as source for the image title text.'),
   +      '#type' => 'select',
   +      '#options' => $options,
   +      '#default_value' => $this->getSetting('title_text'),
   +    ];
   +    $element['alt_text'] = [
   +      '#title' => $this->t('Image alt text'),
   +      '#description' => $this->t('The field that is used as source for the image alt text.'),
   +      '#type' => 'select',
   +      '#options' => $options,
   +      '#default_value' => $this->getSetting('alt_text'),
   +    ];
   

   We can cover this with some simple test too.

Is this something to do with the problem discovered here:

Image Alt and Title displayed in the wrong language IF field format is not "rendered file"

Addressed feedback from #15

@hefterbrumi i don't think this is related but i will check.

Test should be green now.

Looks good to me.

1. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -29,6 +24,26 @@ class FileImageFormatter extends ImageFormatter {
   +      'title_text' => 'field_image_title_text',
   +      'alt_text' => 'field_image_alt_text',
   

   This is not the actual text. Maybe rename variable names to reflect the fact that this is a field.

2. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -75,6 +99,30 @@ class FileImageFormatter extends ImageFormatter {
   +    $available_fields = \Drupal::service('entity_field.manager')->getFieldDefinitions(
   

   Let's inject this.

1. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -60,6 +75,15 @@ class FileImageFormatter extends ImageFormatter {
   +    // Don't display empty alt and title.
   +    $title_field = $this->getSetting('title_text');
   +    if (isset($file->$title_field) && $file->$title_field->value) {
   +      $elements[0]['#title'] = $file->$title_field->value;
   +    }
   +    $alt_field = $this->getSetting('alt_text');
   +    if (isset($file->$alt_field) && $file->$alt_field->value) {
   +      $elements[0]['#alt'] = $file->$alt_field->value;
   +    }
   

   Wondering if we should use token for this instead of hardcoding that it is a field. Since that actually requires the token contrib module, maybe not.

   However, if not, we should use ->hasField() instead of isset()

2. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -75,6 +99,30 @@ class FileImageFormatter extends ImageFormatter {
   +    $options = [];
   +    foreach ($available_fields as $field_name => $field_definition) {
   +      if ($field_definition instanceof FieldConfigInterface && $field_definition->getType() == 'string') {
   +        $options[$field_name] = $field_definition->label();
   +      }
   

   Ah, you actually show a list of options. Fine, that's easier to use. Might want to set a propert empty option, so it says "No alt text" or something like that.

   Also, I agree with naming the fields then title/alt_field, not .. text.

And on 1., we should alos check if we actually have a value before hasField(). no value should be a valid option.

Addressed the feedback also expanded the tests a bit.

1. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -27,6 +27,88 @@ use Drupal\image\Plugin\Field\FieldFormatter\ImageFormatter;
   +   * @param \Drupal\Core\Session\AccountInterface $current_user
   +   *   The current user.
   +   * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager
   +   *   The entity field manager.
   

   Image style storage is missing in @params.

2. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -27,6 +27,88 @@ use Drupal\image\Plugin\Field\FieldFormatter\ImageFormatter;
   +      $summary[] = $this->t('Title value is hidden.');
   ...
   +      $summary[] = $this->t('Alt value is hidden.');
   

   This reads strange. Maybe replace "value" with "attribute" or "text"?

3. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -60,7 +142,19 @@ class FileImageFormatter extends ImageFormatter {
   +    if ($title_field !== 'hide_value') {
   +      if ($file->hasField($title_field)) {
   ...
   +    if ($alt_field !== 'hide_value') {
   +      if ($file->hasField($alt_field)) {
   

   You can combine both conditions into one.

4. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -75,6 +169,30 @@ class FileImageFormatter extends ImageFormatter {
   +      '#options' => ['hide_value' => 'No title text'] + $options,
   ...
   +      '#options' =>  ['hide_value' => 'No alt text'] + $options,
   

   You could use #empty_option.

5. +++ b/src/Tests/FileEntityCreationTest.php
   @@ -130,6 +125,10 @@ class FileEntityCreationTest extends FileEntityTestBase {
   +    $this->assertRaw('alt="A test image"', 'Alt tag is shown and has the correct value.');
   +    $this->assertRaw('title="My image"', 'Title tag is shown and has the correct value.');
   

   Alt/Title attribute.

Should cover the above. Made some changes to element name to combine better for 3.

Used empty option and empty value in element.

1. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -27,6 +27,90 @@ use Drupal\image\Plugin\Field\FieldFormatter\ImageFormatter;
   +      $summary[] = $this->t('Alt attribute is hidden.');
   +    } else {
   +      $summary[] = $this->t('Field used for the image alt attribute: @alt', ['@alt' => $this->getSetting('alt')]);
   +
   +    }
   

   Extra newline.

2. +++ b/src/Plugin/Field/FieldFormatter/FileImageFormatter.php
   @@ -60,7 +144,14 @@ class FileImageFormatter extends ImageFormatter {
   +      $field_name = $this->getSetting($element_name);
   +      if ($field_name !== '#empty_option') {
   +        if ($file->hasField($field_name)) {
   

   Conditions can be merged.

fixed above. also a few other stuff.

Committed.

Follow-up: #2735111: Decide how to handle alt/title on images when core formatter is used

FYI: the Follow-up issue has a D7 core fix that needs porting to D8 core.
#2735111: Decide how to handle alt/title on images when core formatter is used