Gallery: Remove caption fallback for alt attribute

[ockham]

Fixes #26549.
Alternative to #26574, which is somewhat controversial (see discussion).
Effectively reverts #26082. Might require a follow-up for improved a11y.

Description

When a gallery image's alt attribute value is empty, we use what's in the caption prop to fill it out. The issue is that caption can contain links, whereas the alt attribute should not contain any HTML. This PR removes the fallback -- if the image has no alt text set, the alt attribute will be empty.

How has this been tested?

Manually:

1. Add the Image Gallery block.
2. Add some images.
3. Fill the caption with some text and a link.
4. Preview the site and inspect the image element.
5. See that the alt attribute is empty.

Types of changes

Bugfix

Checklist:

• My code is tested.
• My code follows the WordPress code style.

[github-actions]

Size Change: -14 B (0%)

Total Size: 1.21 MB

Filename	Size	Change
build/block-library/index.js	146 kB	-14 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
build/a11y/index.js	1.14 kB	0 B
build/annotations/index.js	3.78 kB	0 B
build/api-fetch/index.js	3.45 kB	0 B
build/autop/index.js	2.84 kB	0 B
build/blob/index.js	665 B	0 B
build/block-directory/index.js	8.72 kB	0 B
build/block-directory/style-rtl.css	943 B	0 B
build/block-directory/style.css	942 B	0 B
build/block-editor/index.js	130 kB	0 B
build/block-editor/style-rtl.css	11.1 kB	0 B
build/block-editor/style.css	11.1 kB	0 B
build/block-library/editor-rtl.css	9.06 kB	0 B
build/block-library/editor.css	9.06 kB	0 B
build/block-library/style-rtl.css	7.9 kB	0 B
build/block-library/style.css	7.9 kB	0 B
build/block-library/theme-rtl.css	837 B	0 B
build/block-library/theme.css	838 B	0 B
build/block-serialization-default-parser/index.js	1.88 kB	0 B
build/block-serialization-spec-parser/index.js	3.1 kB	0 B
build/blocks/index.js	48.1 kB	0 B
build/components/index.js	172 kB	0 B
build/components/style-rtl.css	15.2 kB	0 B
build/components/style.css	15.2 kB	0 B
build/compose/index.js	9.81 kB	0 B
build/core-data/index.js	12.5 kB	0 B
build/data-controls/index.js	772 B	0 B
build/data/index.js	8.77 kB	0 B
build/date/index.js	31.8 kB	0 B
build/deprecated/index.js	768 B	0 B
build/dom-ready/index.js	571 B	0 B
build/dom/index.js	4.46 kB	0 B
build/edit-navigation/index.js	11.2 kB	0 B
build/edit-navigation/style-rtl.css	881 B	0 B
build/edit-navigation/style.css	885 B	0 B
build/edit-post/index.js	306 kB	0 B
build/edit-post/style-rtl.css	6.41 kB	0 B
build/edit-post/style.css	6.39 kB	0 B
build/edit-site/index.js	22.4 kB	0 B
build/edit-site/style-rtl.css	3.88 kB	0 B
build/edit-site/style.css	3.88 kB	0 B
build/edit-widgets/index.js	26.4 kB	0 B
build/edit-widgets/style-rtl.css	3.13 kB	0 B
build/edit-widgets/style.css	3.13 kB	0 B
build/editor/editor-styles-rtl.css	480 B	0 B
build/editor/editor-styles.css	482 B	0 B
build/editor/index.js	43.1 kB	0 B
build/editor/style-rtl.css	3.85 kB	0 B
build/editor/style.css	3.85 kB	0 B
build/element/index.js	4.65 kB	0 B
build/escape-html/index.js	735 B	0 B
build/format-library/index.js	7.7 kB	0 B
build/format-library/style-rtl.css	547 B	0 B
build/format-library/style.css	548 B	0 B
build/hooks/index.js	2.16 kB	0 B
build/html-entities/index.js	623 B	0 B
build/i18n/index.js	3.57 kB	0 B
build/is-shallow-equal/index.js	712 B	0 B
build/keyboard-shortcuts/index.js	2.52 kB	0 B
build/keycodes/index.js	1.94 kB	0 B
build/list-reusable-blocks/index.js	3.11 kB	0 B
build/list-reusable-blocks/style-rtl.css	476 B	0 B
build/list-reusable-blocks/style.css	476 B	0 B
build/media-utils/index.js	5.34 kB	0 B
build/notices/index.js	1.79 kB	0 B
build/nux/index.js	3.42 kB	0 B
build/nux/style-rtl.css	671 B	0 B
build/nux/style.css	668 B	0 B
build/plugins/index.js	2.56 kB	0 B
build/primitives/index.js	1.43 kB	0 B
build/priority-queue/index.js	791 B	0 B
build/redux-routine/index.js	2.85 kB	0 B
build/reusable-blocks/index.js	3.06 kB	0 B
build/rich-text/index.js	13.2 kB	0 B
build/server-side-render/index.js	2.77 kB	0 B
build/shortcode/index.js	1.69 kB	0 B
build/token-list/index.js	1.27 kB	0 B
build/url/index.js	4.06 kB	0 B
build/viewport/index.js	1.84 kB	0 B
build/warning/index.js	1.14 kB	0 B
build/wordcount/index.js	1.22 kB	0 B

_{compressed-size-action}

[youknowriad]

Why didn't you consider to apply the fallback in edit?

[gziolo]

I performed some checks and this is what I found out:

• the original issue that this problematic alt attribute is trying to solve is Linked gallery images are not accessible #13164
• initial try that was merged and reverted: Gallery: Add labels to img, figure and figcaption elements for accessibility #25560
• second attempt that this PR in fact reverts is: Gallery block: use image caption as fallback for alt text. #26082

If we merge this change, we should reopen #13164.

I personally prefer this PR over #26574 only because it buys us more time to rethink how to improve the accessibility of the linked images in Gallery block without creating too much technical debt in the implementation and ensuring that we don't have to create a deprecation to cover something that is very rare:

• an image needs to be linked
• the image should have no alt defined
• a caption needs to contain HTML tag(s)

Aside: The first result on Google has a quite interesting take on this (https://stackoverflow.com/a/58468470):

Copying the text of the figcaption into the alt attribute, or any shortened version, is almost always useless: the screen reader will read twice the same or almost the same information, and it's worth absolutely nothing

It might not be the case here, but it only makes the initial recommendation to use aria-describedby more appealing:
#13164 (comment)

Unfortunately, introducing this new attribute would trigger the need for deprecation 😞

[ockham]

Why didn't you consider to apply the fallback in edit?

I considered it, but I'm similarly skeptical about it as you're about the save-based approach:

• It's fairly convoluted for a purportedly simple fallback. As we've discussed, we'd need to conditionally update alt on every caption change. To recap: The condition can't simply be if ( alt === '') (as it would be in the save case), since that would only update the alt when the caption is first changed (e.g. first keystroke), but would stop updating it as the user continues typing. To compensate for that, we'd also need to update if ( alt === __unstableStripHTML( previousCaption ) ). There's already one unwanted (albeit unlikely) edge case: If the alt and caption are initially set to the same value, and the user wants to change caption only.
• It has potential for confusion, as it doesn't update the image's alt_text attribute (as stored in the media library). alt_text is what's normally used to populate the img's alt attribute; however, the caption fallbacks (both save and edit based approaches) don't update that, so if the user clicks on the 'edit image' button within the gallery, and then opens the media library, the alt attribute there will be empty, rather than contain the caption fallback. We might be able to change this, but it's going to make things even more complicated.
• It doesn't solve the issue with existing alt attributes that contain HTML -- in that regard, it's not superior to this PR.

Considering these, and what @gziolo said, I think we should start with a simpler solution for 9.3, and reconsider how we'd like to solve this eventually when we're less pressed for time.

[ockham]

Thanks a lot @gziolo for bringing a fresh perspective to this ❤️ The SO quote was particularly eye-opening; I agree that given that information, the aria-describedby approach seems to make the most sense (and would allow us to avoid some of the other issues we've encountered while attempting to fix this). But as you said, since this would require a deprecation, let's tackle this problem separately (after 9.3 has shipped).

[youknowriad]

Let's do this. Thanks all for weighing in

[WunderBart]

Something to remember when we're back at the alt fallback issue - Image block already has a certain logic applied for that:

gutenberg/packages/block-library/src/image/image.js

Lines 349 to 363 in 029b1d3

	const filename = getFilename( url );
	let defaultedAlt;
	if ( alt ) {
	defaultedAlt = alt;
	} else if ( filename ) {
	defaultedAlt = sprintf(
	/* translators: %s: file name */
	__( 'This image has an empty alt attribute; its file name is %s' ),
	filename
	);
	} else {
	defaultedAlt = __( 'This image has an empty alt attribute' );
	}

Also, the alt attribute can be controlled via the block settings panel there:

IMO it would be good to stay consistent and, whatever we end up with regarding the alt fallback, make sure it's the same for both GalleryImage and Image blocks. Unless of course, we can use the Image block as the inner block for Gallery?

[glendaviesnz]

Unless of course, we can use the Image block as the inner block for Gallery?

Working on this here - #25940

[tellthemachines]

• initial try that was merged and reverted: Gallery: Add labels to img, figure and figcaption elements for accessibility #25560

Unfortunately, introducing this new attribute would trigger the need for deprecation 😞

@gziolo that PR was not reverted; #26082 only changed where the caption content was added: alt instead of aria-label. The deprecation added by the original PR is still in place, so if the intent is to revert both PRs we should remove that too.

Copying the text of the figcaption into the alt attribute, or any shortened version, is almost always useless: the screen reader will read twice the same or almost the same information, and it's worth absolutely nothing

It might not be the case here, but it only makes the initial recommendation to use aria-describedby more appealing:
#13164 (comment)

The aria-describedby approach was tested and didn't work correctly with screen readers. See here and below for the discussion that ended with the decision to fill in the alt attribute instead. Note that this should only happen if the image is inside a link. #26082 changed that behaviour and applied the caption content to the alt attribute for all images, which was a mistake.

From an accessibility point of view, I maintain that the best solution is something like #26574, except it should only apply to images that are inside links. In any case, further work on this issue must be tested with multiple screen readers, as that is the only way to be sure we get it right.

[gziolo]

@gziolo that PR was not reverted; #26082 only changed where the caption content was added: alt instead of aria-label. The deprecation added by the original PR is still in place, so if the intent is to revert both PRs we should remove that too.

I saw this change:
bd1ea5a

#25833 contains also a comment:

We should try to do this server side.

[gziolo]

Hmm, this deprecation is still in the file. I'm lost now:

gutenberg/packages/block-library/src/gallery/deprecated.js

Lines 22 to 175 in 3fd7a8e

	{
	attributes: {
	images: {
	type: 'array',
	default: [],
	source: 'query',
	selector: '.blocks-gallery-item',
	query: {
	url: {
	type: 'string',
	source: 'attribute',
	selector: 'img',
	attribute: 'src',
	},
	fullUrl: {
	type: 'string',
	source: 'attribute',
	selector: 'img',
	attribute: 'data-full-url',
	},
	link: {
	type: 'string',
	source: 'attribute',
	selector: 'img',
	attribute: 'data-link',
	},
	alt: {
	type: 'string',
	source: 'attribute',
	selector: 'img',
	attribute: 'alt',
	default: '',
	},
	id: {
	type: 'string',
	source: 'attribute',
	selector: 'img',
	attribute: 'data-id',
	},
	caption: {
	type: 'string',
	source: 'html',
	selector: '.blocks-gallery-item__caption',
	},
	},
	},
	ids: {
	type: 'array',
	items: {
	type: 'number',
	},
	default: [],
	},
	columns: {
	type: 'number',
	minimum: 1,
	maximum: 8,
	},
	caption: {
	type: 'string',
	source: 'html',
	selector: '.blocks-gallery-caption',
	},
	imageCrop: {
	type: 'boolean',
	default: true,
	},
	linkTo: {
	type: 'string',
	},
	sizeSlug: {
	type: 'string',
	default: 'large',
	},
	},
	save( { attributes } ) {
	const {
	images,
	columns = defaultColumnsNumber( attributes ),
	imageCrop,
	caption,
	linkTo,
	} = attributes;
	return (
	<figure
	className={ `columns-${ columns } ${
	imageCrop ? 'is-cropped' : ''
	}` }
	>
	<ul className="blocks-gallery-grid">
	{ images.map( ( image ) => {
	let href;
	switch ( linkTo ) {
	case LINK_DESTINATION_MEDIA:
	href = image.fullUrl || image.url;
	break;
	case LINK_DESTINATION_ATTACHMENT:
	href = image.link;
	break;
	}
	const img = (
	<img
	src={ image.url }
	alt={ image.alt }
	data-id={ image.id }
	data-full-url={ image.fullUrl }
	data-link={ image.link }
	className={
	image.id
	? `wp-image-${ image.id }`
	: null
	}
	/>
	);
	return (
	<li
	key={ image.id || image.url }
	className="blocks-gallery-item"
	>
	<figure>
	{ href ? (
	<a href={ href }>{ img }</a>
	) : (
	img
	) }
	{ ! RichText.isEmpty(
	image.caption
	) && (
	<RichText.Content
	tagName="figcaption"
	className="blocks-gallery-item__caption"
	value={ image.caption }
	/>
	) }
	</figure>
	</li>
	);
	} ) }
	</ul>
	{ ! RichText.isEmpty( caption ) && (
	<RichText.Content
	tagName="figcaption"
	className="blocks-gallery-caption"
	value={ caption }
	/>
	) }
	</figure>
	);
	},
	},

Should we remove it? save method doesn't differ at all anymore if I follow the whole chain of changes :)

[gziolo]

I opened #26736 in case we no longer need this deprecation added.