Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#39912 closed defect (bug) (fixed)

Image empty alt attributes get stripped out when editing the image details from a post

Reported by: afercia Owned by: iseulde
Priority: high Milestone: 4.7.3
Component: TinyMCE Version:
Severity: normal Keywords: has-patch fixed-major
Cc: Focuses: accessibility

Description

When an image has a decorative purpose, it's extremely important for accessibility that the image has an empty alt attribute alt="". In absence of an empty alt attribute, assistive technologies won't assume the image is just decorative and will try to read out "something". In most of the cases they read out the image file path or part of it, i.e. the filename.

Turns out, when editing the details of an image inside a post, empty alt attributes get stripped out when updating the details.

Seems to me this is not specific to 4.7 but happens also in some of the previous versions, under different conditions. Worth reminding the alt attribute fallback to the title was recently removed, and this is one of the reasons for the different behaviour across different versions.

To reproduce in WordPress 4.7:

  • edit a post
  • insert an image with an empty alt attribute

The markup inserted in the post will be something like this:
<img class="alignnone size-full wp-image-3020" src="http://.../wp-content/uploads/2017/01/image.jpg" alt="" width="1200" height="800" />

  • then click on the image and click on the "Edit" pencil icon in the inline image toolbar
  • the "Image Details" modal opens
  • change some image property, for example set the alignment to "right"
  • click the "Update" button in the modal

The updated markup in the post will be:
<img class="alignright wp-image-3020 size-full" src="http://.../wp-content/uploads/2017/01/image.jpg" width="1200" height="800" />

To reproduce in WordPress 4.6 and 4.5:

  • repeat all the steps above but make sure that also the "Title" of the image is empty. When both the title and the alt text fields are empty, the empty alt="" gets stripped out

WordPress 4.4:
Seems to me the empty alt attribute is always preserved, which is the correct behaviour.

Attachments (1)

39912.diff (814 bytes ) - added by afercia 10 years ago.

Download all attachments as: .zip

Change History (18)

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


10 years ago

#2 @sami.keijonen
10 years ago

I confirm this and can reproduce in WP 4.7.2.

#3 @afercia
10 years ago

  • Component MediaTinyMCE

I think it's because the wpeditimage TinyMCE plugin uses tinymce.dom.DOMUtils.setAttribs which just recursively calls tinymce.dom.DOMUtils.setAttrib which accepts 3 params, and about the value one:

Value to set on the attribute - if this value is falsy like null, 0 or "" it will remove the attribute instead.

So when wp.media passes an empty alt attribute, TinyMCE just strips it out. It makes sense to remove empty HTML attributes, with the notable exception of the alt attribute. While WordPress should ensure this doesn't happen and try to find a way to re-add the empty alt attribute when appropriate, maybe this should be reported upstream too.

Last edited 10 years ago by afercia (previous) (diff)

#4 @joemcgill
10 years ago

Agreed. We should add an exception that allows alt attributes to remain with empty values since, otherwise, screen readers would read out the filename, which is often not preferred.

#5 @afercia
10 years ago

If I revert to [36984], that is before TinyMCE was updated to 4.3.8 this doesn't happen. Quickly looking at the TinyMCE changelog, the following change looks a bit suspect to me:

Version 4.3.8 (2016-03-15)
    ...
    Fixed so the alt attribute doesn't get padded with an empty value by default.

which, in combination with setAttrib() used in the wpeditimage plugin, maybe could explain the issue.

Last edited 10 years ago by afercia (previous) (diff)

@afercia
10 years ago

#6 @afercia
10 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone Awaiting Review4.7.3

39912.diff could be a fix from the WordPress side. There are probably better ways to do this, not really a TinyMCE expert here :) Moving to 4.7.3 for consideration, since this is a pretty serious issue for accessibility.

#7 @afercia
10 years ago

  • Owner set to iseulde
  • Status newassigned

Assigning to @iseulde for review, as agreed :)

#8 @afercia
10 years ago

Worth checking if getAttribs() and getAttrib() are used also in other places, e.g. captioned images.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


10 years ago

This ticket was mentioned in Slack in #core-tinymce by afercia. View the logs.


10 years ago

This ticket was mentioned in Slack in #core-editor by afercia. View the logs.


10 years ago

This ticket was mentioned in Slack in #core by jnylen. View the logs.


10 years ago

#13 @azaozz
10 years ago

  • Resolutionfixed
  • Status assignedclosed

In 40110:

TinyMCE: preserve empty image alt attributes.

Props afercia.
Fixes #39912 for trunk.

#14 @azaozz
10 years ago

  • Resolution fixed
  • Status closedreopened

Reopen for 4.7.

#15 @jnylen0
10 years ago

  • Keywords fixed-major added

#16 @azaozz
10 years ago

  • Resolutionfixed
  • Status reopenedclosed

In 40112:

TinyMCE: preserve empty image alt attributes.

Props afercia.
Merges [40110] to the 4.7 branch.
Fixes #39912.

This ticket was mentioned in Slack in #accessibility by sami.keijonen. View the logs.


10 years ago

Note: See TracTickets for help on using tickets.