Add media, insert from URL and alt attribute

[afercia]

When inserting an image using "Insert from URL" in the media modal, an alt attribute should be always inserted in the HTML. When users fill out the "Alt Text" field, the alt attribute is correctly inserted.
But when they leave the field empty, no alt attribute is inserted, not even an empty one. The resulting markup would be something like:

<img class="alignnone" src="https://cldup.com/YeRLlgnhxd.png" width="1195" height="249" />

and in this case an alt="" should be automatically added.

[ambrosey]

I looked into this and realized that the problem was that the filters set up on lines 57-61 of media-editor.js were not triggering,

				if ( 'image' === props.type && ! props.alt ) {
					props.alt = props.caption || props.title || '';
					props.alt = props.alt.replace( /<\/?[^>]+>/g, '' );
					props.alt = props.alt.replace( /[\r\n]+/g, ' ' );
				}

because props.type wasn't being assigned. This patch assigns props.type properly within the use case where the type is image, and enters this function properly.

However, now the alt tag that comes through is picking up props.title which is defaulting to the URL being pulled -- i.e.

<img class="alignnone" src="https://cldup.com/YeRLlgnhxd.png" width="1195" height="249" alt="https://cldup.com/YeRLlgnhxd.png" />

I don't think that is what the behavior should be, but I don't understand what the intent is for the alt to pick up in this case.

[cgrymala]

Replying to ambrosey:

However, now the alt tag that comes through is picking up props.title which is defaulting to the URL being pulled

I don't think that is what the behavior should be, but I don't understand what the intent is for the alt to pick up in this case.

It's definitely not ideal, from an accessibility standpoint, but, it does appear to be consistent with the way other "Insert Media" events occur. Currently, WordPress is set up to look at the alt field first, then fallback to the title or caption to use as the alt attribute for the image if the alt field is blank.

[joemcgill]

Thanks for digging into this, @ambrosey, and good catch. @cgrymala is correct that the fallback to caption and then to title mimics what happens in wp_get_attachment_image() ​see docs. It may be worth discussing a better alternative.

[afercia]

For a similar discussion about the alt fallback see also #34635.

[dabnpits]

I've attempted to take this a little further in terms of accessibility by preventing the alt tag using the title as a fallback if the title is the URL.

[joemcgill]

Hi @dabnpits that's a nice improvement, and maybe one that could be applied in wp_get_attachment_image() as well but that might be better handled as a part of adding a new wp_get_attachment_image_alt() helper function in #30180.

[afercia]

Related: #34635

[joemcgill]

36735.2.patch​ is similar to 36735.1.patch​ except that it sets the type property during the callback on the workflow.state for embeds, instead of waiting for wp.media.string.image() and also cleans up a few coding style issues.

Not sure which is the better place in the chain to explicitly set the type property since usually it is taken from the passed attachment, which doesn't exist in this case.

[joemcgill]

After looking at this a bit more thoroughly, I slightly prefer setting props.type in wp.media.string.image() as suggested by @dabnpits in 36735.1.patch​, though there isn't much functional difference from what I can tell.

[joemcgill]

In 38065:

Media: Always add alt attributes to images inserted from URLs

Previously, when inserting an image from a URL, leaving the alt
field blank in the media modal would result in an image being
inserted into the editor without an alt attribute, rather than
an empty alt. This happened because the props.type would not
get set in wp.media.string.props() — because attachment is
undefined in this case — causing the image fallbacks to get
skipped.

This fixes the issue by explicitly setting props.type to 'image'
in wp.media.string.image() before filling out the rest of the
properties.

Props ambrosey, dabnpits.
Fixes #36735.

[afineman]

Currently

alt

is unspecified.

Users should be able to specify

alt=""

See existing source code from media-editor.js line 54-70 below:

	// Final fallbacks run after all processing has been completed.
			fallbacks = function( props ) {
				// Generate alt fallbacks and strip tags.
				if ( 'image' === props.type && ! props.alt ) {
					if ( props.caption ) {
						props.alt = props.caption;
					} else if ( props.title !== props.url ) {
						props.alt = props.title;
					} else {
						props.alt = '';
					}

					props.alt = props.alt.replace( /<\/?[^>]+>/g, '' );
					props.alt = props.alt.replace( /[\r\n]+/g, ' ' );
				}

				return props;

[joemcgill]

Good catch @afineman. This was easy to miss because the visual editor automatically converted this to the proper alt="" notation, but the text editor does not. 36735.3.patch​ addresses the issue by making an exception in wp.html.string() so empty attribute notation isn't used when the attribute is alt.

[adamsilverstein]

@joemcgill thanks for the quick fix here, I was looking at this ticket today with @afineman and wondering if in wp.html.string we should be inserting as the comment calls it 'empty attribute notation' for any attribute?

Googling the alt tag specifically, I clearly see that it should be a blank string and not just alt with no value... my question is: is it ever proper HTML syntax to insert an attribute without a value, ie. attribute vs. attribute=""?

[adamsilverstein]

36735.4.patch​ is an alternate approach completely removing the early exit for blank attributes in wp.html.string - this result in empty attributes always getting the ="" appended indicating their blank value. not certain this is proper/better, offering it here pending more research/feedback from the accessibility team, etc.

[afineman]

Thanks @joemcgill and @adamsilverstein
I'm new to this and not sure how to apply the patch to see. But one other idea I had that seemed to work — albeit a bit sloppy — was changing line 63 media-editor.js

props.alt = '""';

It results in 4 double quotes first, but resolved to 2 double quotes when the post saved.

[joemcgill]

Replying to adamsilverstein:

@joemcgill thanks for the quick fix here, I was looking at this ticket today with @afineman and wondering if in wp.html.string we should be inserting as the comment calls it 'empty attribute notation' for any attribute?

This is a good question. Looking back through the history here, it seems like the current behavior was introduced in [22567], but it's not clear why @koop added this "empty attribute notation" check. Looking at the W3C draft spec, the only valid use case for that style that I could come up with is this section on ​boolean attributes. Ex:

<label><input type=checkbox checked name=cheese disabled> Cheese</label>

However, even for those cases I can't see how something like prop.checkbox = '' would be expected to be true, so 36735.4.patch​ seems like the right approach here. All of our QUnit tests still pass with 36735.4.patch​, but I'm not sure we have adequate (or any) coverage for something that would break by changing that check.

[afineman]

Was able to get the patch installed thanks to @rfair404 help
@joemcgill FYI 36735.3.patch didn't seem to work.
However, patch 36734.4 from @adamsilverstein seems to be working — at least with images inserted by URL.

[joemcgill]

Thanks for testing @afineman. 36735.3.patch​ seems to be working for me, so maybe you were still getting a cached version of the shortcode.js file. At any rate, I think 36735.4.patch​ is probably the way to go, unless someone has a use case for the current behavior.

Changing the behavior of images inserted from the media library is related, but a bit outside the scope of this ticket. The changes you're proposing for the title would be better discussed as a part of #34635. Feel free to follow along and leave feedback there. Cheers!

[joemcgill]

In 38116:

Media: Ensure empty alt attributes are set to blank strings.

This is a follow up to [38065] to ensure that wp.html.string() always
converts empty alt attributes to alt="" rather than returning HTML using
empty attribute notation, like alt. This allows screen readers to ignore
images with empty alt attributes, rather than reading out the URL string.

Additionally this completely removes the logic in wp.html.string() for
converting blank attributes to empty attribute notation since empty attribute
notation is generally meant to denote a boolean value, e.g.,
checked == checked="checked", which doesn't apply in this context because
boolean attributes must be omitted in order to represent a false value.
See: ​https://www.w3.org/TR/html5/infrastructure.html#boolean-attributes

Props adamsilverstein, afineman, joemcgill.
Fixes #36735.