Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 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's profile afercia Owned by: iseulde's profile iseulde
Milestone: 4.7.3 Priority: high
Severity: normal Version:
Component: TinyMCE Keywords: has-patch fixed-major
Focuses: accessibility Cc:

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 8 years ago.

Download all attachments as: .zip

Change History (18)

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


8 years ago

#2 @sami.keijonen
8 years ago

I confirm this and can reproduce in WP 4.7.2.

#3 @afercia
8 years ago

  • Component changed from Media to TinyMCE

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 8 years ago by afercia (previous) (diff)

#4 @joemcgill
8 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
8 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 8 years ago by afercia (previous) (diff)

@afercia
8 years ago

#6 @afercia
8 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 4.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
8 years ago

  • Owner set to iseulde
  • Status changed from new to assigned

Assigning to @iseulde for review, as agreed :)

#8 @afercia
8 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.


8 years ago

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


8 years ago

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


8 years ago

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


8 years ago

#13 @azaozz
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 40110:

TinyMCE: preserve empty image alt attributes.

Props afercia.
Fixes #39912 for trunk.

#14 @azaozz
8 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 4.7.

#15 @jnylen0
8 years ago

  • Keywords fixed-major added

#16 @azaozz
8 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

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.


8 years ago

Note: See TracTickets for help on using tickets.