Problem/Motivation

When embedding an image entity, the wrapper <div> from Entity Embed contains an alt and title tag, both of whic duplicate the same attributes from the inner <img> tag.

The title attribute is considered harmful to accessibility, and should be removed. See #3083583: [META] Discourage non-inclusive use of the HTML title attribute.

The alt attribute is not allowed on <div> elements in HTML5. This causes errors in HTML validation. If you provide it to Google Search, AMP pages will not appear in search results.

<div data-embed-button="media_browser"
     data-entity-embed-display="media_image"
     data-entity-embed-display-settings="small"
     data-entity-type="media"
     data-entity-uuid="XXXXXXXXXXXX"
     alt="myImageAlt" //This causes errors
     title="myImageTitle"
     data-langcode="ja"
     class="embedded-entity align-center">
    <amp-img src="/sites/default/files/styles/small/public/images/myImage.jpg?itok=XXXXXXXX"
             width="700"
             height="510"
             alt="myImageAlt"
             title="myImageTitle"
             layout="responsive">
    </amp-img>
</div>

Proposed resolution

Remove alt attribute from div element that rendered from <drupal-entity> .

<div data-embed-button="media_browser"
     data-entity-embed-display="media_image"
     data-entity-embed-display-settings="small"
     data-entity-type="media"
     data-entity-uuid="XXXXXXXXXXXX"
     // just removed alt with `attributes.removeAttribute('alt')` in twig.
     title="myImageTitle"
     data-langcode="ja"
     class="embedded-entity align-center">
    <amp-img src="/sites/default/files/styles/small/public/images/myImage.jpg?itok=XXXXXXXX"
             width="700"
             height="510"
             alt="myImageAlt"
             title="myImageTitle"
             layout="responsive">
    </amp-img>
</div>

Remaining tasks

reviews needed

CommentFileSizeAuthor
#28 entity_embed-remove_image_attributes_from_div-3086162-28-D8.patch1.17 KBjwilson3
#21 entity_embed-remove_image_attributes_from_div-3086162-21.patch1.41 KBspadxiii
#14 entity_embed-remove_image_attributes_from_div-3086162-13.patch1.19 KBgdaw
#13 entity_embed-remove_image_attributes_from_div-3086162-13.patch1.18 KBgdaw
#9 entity_embed-remove_alt_image_style_from_div-3086162-7-D8.patch395 bytesgdaw
#3 entity_embed-remove_alt_from_div_3086162-1-D8.patch1.01 KBTomotsugu Kaneko

Issue fork entity_embed-3086162

Command icon 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:

About issue forks

Comments

Tomotsugu Kaneko created an issue. See original summary.

Tomotsugu Kaneko’s picture

Tomotsugu Kaneko commented
Issue summary: View changes
Tomotsugu Kaneko’s picture

Tomotsugu Kaneko commented
StatusFileSize
new entity_embed-remove_alt_from_div_3086162-1-D8.patch1.01 KB
oknate’s picture

oknate
Location Greater New York City Area
commented

The <drupal-entity> tag shouldn't display on the front-end. It should be converted to an entity by EntityEmbedFilter. So this issue doesn't really make sense.

Tomotsugu Kaneko’s picture

Tomotsugu Kaneko commented

Yes. <drupal-entity> render to <div> with the alt attribute.
I can display <drupal-entity> with ckeditor's source mode in admin area.

gdaw’s picture

gdaw commented

This patch fixes the issue for me, RTBC +1.

gdaw’s picture

gdaw commented

Tomotsugu I found this ticket before I submitted my own patch and ticket, but I used a slightly different approach

Instead of ... attributes.removeAttribute('alt') ... I used this ... attributes|without('alt')

Both methods will fix the validation error, but perhaps it is more efficient to display|without as opposed to display.adding/removing stuff?

I also have a second validation error for the image_style attribute, so I will provide a patch that takes care of both using |without()

This new patch will fix both W3C validation Erros ...

Attribute alt not allowed on element div at this point.
Attribute image_style not allowed on element div at this point.

gdaw’s picture

gdaw commented
Status: Active » Reviewed & tested by the community
gdaw’s picture

gdaw commented
StatusFileSize
new entity_embed-remove_alt_image_style_from_div-3086162-7-D8.patch395 bytes
gdaw’s picture

gdaw commented
Title: Attribute alt not allowed on element div in HTML5 » Attribute alt (and image_style) not allowed on element div in HTML5
Status: Reviewed & tested by the community » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: entity_embed-remove_alt_image_style_from_div-3086162-7-D8.patch, failed testing. View results

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

@gdaw you need to also update the tests as was done in the previous patch that passed testing.

gdaw’s picture

gdaw commented
StatusFileSize
new entity_embed-remove_image_attributes_from_div-3086162-13.patch1.18 KB

Thanks for the reminder @joseph.olstad ... and while updating the test I spotted a third image attribute (title) we don't want to see on the div. Uploading patch 13

gdaw’s picture

gdaw commented
StatusFileSize
new entity_embed-remove_image_attributes_from_div-3086162-13.patch1.19 KB
gdaw’s picture

gdaw commented
gdaw’s picture

gdaw commented
joseph.olstad’s picture

joseph.olstad
Primary language French
commented

WCAG was also complaining about the title attribute?

Hmm:
w3schools says any element can have a title attribute
https://www.w3schools.com/tags/att_global_title.asp

but another article warns against using it:
https://developer.paciellogroup.com/blog/2012/01/html5-accessibility-cho...

hmm, ya some people say the title attribute is problematic with tools like jaws. (should we all change because of Jaws? apparently?)
https://www.24a11y.com/2017/the-trials-and-tribulations-of-the-title-att...

gdaw’s picture

gdaw commented

In this case the title would be on the div as well as on the image itself, so a screen reader would 'stutter' and repeat the same title twice. This fix would prevent that from happening, plus resolve the other invalid attributes being added to the div.

gdaw’s picture

gdaw commented
Status: Needs work » Needs review
joseph.olstad’s picture

joseph.olstad
Primary language French
commented

3 years later and we're still using this for WCAG compliance

spadxiii’s picture

spadxiii commented
StatusFileSize
new entity_embed-remove_image_attributes_from_div-3086162-21.patch1.41 KB

did a quick re-roll on the latest 8.x-1.x

tostinni’s picture

tostinni commented

@spadxiii I don't think your 8.x-1.x reference is accurate as your patch contains code that has been removed in #2881745: Wrapping embedded entities in <article> is bad for accessibility, use <div> instead, so for me the correct patch is still #14

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Status: Needs review » Needs work

Setting back to NW per #22. This would also benefit from being moved to an MR.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Title: Attribute alt (and image_style) not allowed on element div in HTML5 » Attribute alt not allowed on element div in HTML5
Version: 8.x-1.0 » 8.x-1.x-dev

image_style attribute no longer appears in the latest 8.x-1.x version.

jwilson3 opened merge request !60

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Title: Attribute alt not allowed on element div in HTML5 » Remove alt and title attributes from Entity Embed container div

We should also remove the title attribute, which has known accessibility issues.

See #3083583: [META] Discourage non-inclusive use of the HTML title attribute.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Status: Needs work » Needs review
jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Issue summary: View changes
StatusFileSize
new entity_embed-remove_image_attributes_from_div-3086162-28-D8.patch1.17 KB

Here is an updated and versioned patch from the current state of the MR.

peri22’s picture

peri22 commented
Status: Needs review » Reviewed & tested by the community

Hello, reviewed and works correctly. Thanks.

macsim’s picture

macsim commented

RTBC +1
Thanks