Problem/Motivation
Images viewed with Media Thumbnail Formatter haven't alt and title attribute.
The problem occurs after update at Drupal 9.
Steps to reproduce
Steps to reproduce:
- Install Media module
- Add an Image media entity with any image file and specify alt text
- Edit the image media entity and update the alt text and save
- Use the Media Thumbnail Formatter
- Observe that the image haven't alt and title text
Control:
- Spin up a new Umami site (I used https://ddev.github.io/ddev-gitpod-launcher/)
- Observe the alt text of the borscht in the card on the homepage - "Traditional Ukrainian soup with beets, tomatoes, and pork ribs"
- Go to Content > Media > "borscht" and edit the "Borscht with pork ribs" image
- Change the alt text to "Yummy traditional Ukrainian soup with beets, tomatoes, and pork ribs"
- Save the image
- Return to the homepage and observe the alt has been properly updated.
Failure case:
- Go to Admin > Structure > Content Types > Recipe > Manage Display > Card common alt (/en/admin/structure/types/manage/recipe/display/card_common_alt)
- Change the Media Image Format from "Rendered entity" to "Thumbnail" (no other settings matter)
- Save
- Return to the homepage and observe the alt text has reverted to "Traditional Ukrainian soup with beets, tomatoes, and pork ribs"
- (optional) Try clearing caches, because that could be a thing. No change on cache clear.
- Apply patch, clear caches
- Observe alt text properly updates
- Change alt text on media again, observe it properly updates again
Proposed resolution
I created a path to solve the problem.
The patch gets the item attribute from the source field.
Issue fork drupal-3354876
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
marco.aresu commentedComment #3
aaronpinero commentedThis seems like a pretty basic and important accessibility problem. I just noticed it today when doing a WAVE report on my website. This issue was the first error reported. I thought it was a content author mistake. It's a surprising oversight for the Media system. Can we bump this up to critical?
Comment #4
aaronpinero commentedI tested the patch with Drupal 9.5.8 and it appears to solve the issue. I am seeing alt attribute information on a media image thumbnail where previously there was none.
Comment #5
cilefen commentedComment #6
aaronpinero commentedOne thing I noted about the site after the patch: if the image is missing alt text, an empty alt attribute is added to the image. Not sure if that's a desired outcome... would it be better to leave off the alt attribute entirely if there's no text available?
Comment #7
aaronpinero commentedI am uploading a new patch because the current one doesn't apply cleanly through composer with Drupal 9.5.9. There was an update in 9.5.9 to the media thumbnail formatter that need to be accounted for. That said, the patch is VERY similar.
Comment #8
froboy@aaronpinero as per the Web Accessibility Initiative guidelines:
I think it's the correct behavior to add an empty alt if none exists so that the images at least behave unobtrusively.
Comment #9
froboyI'm seeing this issue in 10. Bumping the version.
Comment #11
froboyI've opened a MR for Drupal 10.1 and added the diff here for folks to use with composer. I'm not great at tests, but if this needs them and someone could direct me to the right place I can make an attempt at adding them.
Comment #12
podarokLooks good
Here is a diff, no need to upload patches @froboy
https://git.drupalcode.org/project/drupal/-/merge_requests/4291.diff
Comment #13
froboy@podarok I'm aware, but I didn't want to introduce potential security risks. :)
Comment #14
podarokComment #15
podarokComment #16
lauriiiWhat are the specific steps needed to reproduce this? I tried to follow the steps from the issue summary and couldn't reproduce this
Comment #17
chris matthews commentedLike @lauriii, I also tried following what seem to be simple steps in the IS but I also could not reproduce this issue. Might be missing something simple but this issue is postponed until lauriii or another committer can reproduce.
Comment #18
froboyI believe I've been able to reproduce the issue on a clean environment:
Control:
- Spin up a new Umami site (I used https://ddev.github.io/ddev-gitpod-launcher/)
- Observe the alt text of the borscht in the card on the homepage - "Traditional Ukrainian soup with beets, tomatoes, and pork ribs"
- Go to Content > Media > "borscht" and edit the "Borscht with pork ribs" image
- Change the alt text to "Yummy traditional Ukrainian soup with beets, tomatoes, and pork ribs"
- Save the image
- Return to the homepage and observe the alt has properly updated.
Failure case:
- Go to Admin > Structure > Content Types > Recipe > Manage Display > Card common alt (/en/admin/structure/types/manage/recipe/display/card_common_alt)
- Change the Media Image Format from "Rendered entity" to "Thumbail" (no other settings matter)
- Save
- Return to the homepage and observe the alt text has reverted to "Traditional Ukrainian soup with beets, tomatoes, and pork ribs"
- (optional) Try clearing caches, because that could be a thing. No change on cache clear.
- Apply patch, clear caches
- Observe alt text properly updates
- Change alt text on media again, observe it properly updates again
I hope that helps. Thanks for your time.
Comment #19
smustgrave commentedThanks for providing steps!
This is NW though for the tests and upgrade paths.
Comment #20
podarokComment #21
aaronpinero commentedHi all. I would definitely like to move this issue along. It's been indicated that tests are needed to move this back to "needs review". I would like to help but I am a novice developer and the only tests I've ever written for a module were based heavily on provided examples of Unit and Kernel tests. I don't know a whole lot about selecting the appropriate tests and then writing them.
That said, I did a bit of digging into the media module code. For tests related to the thumbnail formatter (where the proposed patch is applied) there is a Kernel test (core/modules/media/tests/src/Kernel/MediaThumbnailFormatterTest.php) and a Functional test (core/modules/media/tests/src/Functional/FieldFormatter/MediaThumbnailFormatterTest.php). The Kernel test seems to provide only a test to verify settings of the field formatter. The Functional test appears to actually test the rendering of the formatted media field as it would be used in a node.
Based on what I'm reading in the code, the appropriate test addition for this change to the module would be to modify the Functional test that already exists to include some check of the rendered alt text vs the alt text stored in the reference media entity. Leaving aside (for the moment) the details of actually executing this change to the functional test, does this seem like a reasonable plan? Are there additional tests that would need to be included that I haven't considered?
Comment #22
mikegodin commentedHere's the patch from #7, adjusted for Drupal 10.1.5
Comment #23
neclimdulThis has a long history and a bunch of other issues trying to address is in different ways. This seems to currently be the most active, so linking them up.
Comment #24
neclimdulIt looks like the current patch remove the thumbnail entirely if there's an image. I guess that works, but does it really solve the problem? Should we get rid of the thumbnail entirely for all cases? Is there a use case where the thumbnail is still needed but you wouldn't be able to manage the thumbnail title and alt properties?
Comment #26
nathan573 commentedI have a View that displays media items of various types. This default generic "media-icon" icons show for each item that isn't an image - like documents or videos in the media library. Applying this patch makes this View render a blank space instead of these generic media icons.
Thanks
Comment #27
nmangold commentedI confirm this issue and also confirm the MR fixes the issue using Drupal 11.1.4.
Although, it does not appear grabbing the source field was needed in Drupal 8 and that part of the field formatter did not change. So, this solution feels like a hack to get the desired outcome instead of fixing the root of the problem. That said, I have not dug into figuring out the root problem, which seems to be the alt attribute is not included when the thumbnail field is returned. FWIW, the alt attribute IS being returned for the media icons, when the media entity is not an image type. Image type does require the alt text. Maybe that is helpful.
@aaronpinero, I agree with that testing approach.
I could not replicate any issue with the media type icons disappearing after applying the patch. I configured a view to list media entities, showing the Thumbnail field, and the media type icons are displayed both with and without the patch. So, @neclimdul and @nathan573, please give more details about how to reproduce your issue.
Comment #28
nmangold commentedI dug and dug, and dug some more. @neclimdul is right, this does have a long history. Here is my not so short summary.
The Thumbnail display mode was designed to be used for the entity type icons, which are decorative. Mainly for the admin media list page.
The alt text is empty so screen readers will interpret the image as decorative. See #2956368: MediaThumbnailFormatter produces unhelpful text alternative and title attributes for media thumbnails. The suggestion there was to use the rendered entity formatter and use a custom view mode and configure the real image field, where the alt text submitted by user is displayed. Since, the Thumbnail view mode was not intended for content images.
The media entity has a base field named thumbnail which stores the thumbnail metadata only when the media entity is first created. It is not updated when the source image field is updated. This is by design as there has been some disagreement about how to do handle the updates. See the updateThumbnail() method in the Media entity code docroot/core/modules/media/src/Entity/Media.php, and the issue #2878119: Whether queued or not, update the media thumbnail and metadata before beginning the entity save database transaction.
The MR for this issue proposes to get the thumbnail metadata from the underlying image entity, if one exists, and use it's alt text. Similar patches have been suggested at #2956368: MediaThumbnailFormatter produces unhelpful text alternative and title attributes for media thumbnails. However, that issue was closed before those patches were added. So, no reviews were made regarding the solution similar to the one in the MR for this issue. This may seem like a viable solution. Since, pulling the alt and title from the underlying image field is how the alt and title value were original populated when the media entity was initially created. However, that bypasses the alt and title values on the media entity, and begs the question for the purpose of the alt and title field on the media entity. At least for the image media types, because the other media types use those fields for the icons. Those fields would be NULL forever, and never be updated.
I am not sure what initially caused the existing media entity alt and title fields to be changed to NULL, but the damage has been done. I believe the proper solution is to actually update the media entity alt and title values using the values from the source image. Not just display them on-the-fly.
The problem of setting the value to NULL instead of an empty string was initially found and fixed in #3319601: Media image thumbnail incorrectly ends up as NULL when it should be an empty string. So, this issue probably could have been marked as a duplicate of that one. Unfortunately, an update hook was not created within that solution to update existing media entities.
An active issue exists to expose the ability to end users to trigger update of media metadata and thumbnail, #2983456: Expose triggering update of media metadata + thumbnail to end users. That seems like the correct approach at this point. Especially, given all the disagreement about how to handle updates on a regular basis.
Therefore, I suggest closing this issue in favor of 2983456. Content images that need to be rendered as a thumbnail version should be configured to use the rendered entity formatter and a custom view mode that utilizes the thumbnail image style.
Comment #29
quietone commentedUsing the steps in the issue summary and in #18 I was not able to reproduce the problem.
Why is this tagged for 2 different upgrade paths, D9 and D10 upgrade path. It isn't explicit in the IS or comments, is the upgrade actually removing data?