Custom logo is missing meaningful alt attribute

[sami.keijonen]

In 38812 default alt fallbacks was removed to improve accessibility.

Therefore custom logo now have empty alt attribute. Should it have meaningful alt attribute by default for better accessibility?

There is ​background info in Slack archive.

[joedolson]

I'm inclined to think that the ideal default alt attribute for this case would be the site title. That's likely to be appropriate in most cases - even if it's not a formal match for the text on the alt attribute, it would probably convey the same general information.

[sami.keijonen]

Agreed that site title would be nice default alt.

Did some more testing.

1. Go to the Customizer >> Site Identity.
2. Click Select logo.
3. Pick image and write alt text.
4. Click Select.
5. Crop image.
6. Save and Publish.
7. Cropped image doesn't inherit the alt text that I just wrote in step 3.
8. If I go back to Media library and add alt text to cropped image, it does show in up in custom logo.

This is probably how it should work but step 7. still feels weird to me. I'd expect that alt text would be in logo image because I just wrote it.

I was testing in Twenty Seventeen.

Also note that if I use image that I don't have to crop, alt text is naturally there.

[afercia]

In 4.6. the alt attribute is just the image filename, and should be improved to have a meaningful value. In 4.7 is going to be an empty alt attribute because the fallback is going to be removed, see [38812]. Either ways, this should be fixed, hopefully during 4.8.

[sstoqnov]

Add logo alt attribute

[sstoqnov]

I agree with you that the site title is a meaningful value for alt attribute, but I think that the user should have control over that value. That's why I think it's better to check if the image has an alt attribute first and then we can add the site title if it's empty.
What do you think? @afercia @sami.keijonen

[nobremarcos]

Just tested @sstoqnov path and as he describes it right now there is no checking if the user has added an "Alt Text" to the logo image and it applies the site title to the alt even if the user entered a custom "Alt Text" to it.

[gma992]

@nobremarcos

The check in the patch is here:

		if ( empty( $image_alt ) ) {
			$image_alt = get_bloginfo( 'name' );
		}

I've just tested it and works fine when adding a new logo.

If the alt field is empty, is populated automatically with the site title, otherwise is populated with the custom alt text added into the field. Mind to clarify the steps to reproduce why is not working on your end? Maybe I misunderstood the issue you're seeing?

On the other hand, as @sami.keijonen pointed out, when we crop the image, it does not grab the customized alt text, but populates it by an empty alt text by default or with the site title after being patched.

This is the normal behavior thought (but not the expected one) as when we crop the logo, we're creating a new image on the media gallery, and that new image has no alt text yet.

[LiamMcArthur]

I agree. I personally think it's overkill to have another input box for the user to specify a custom alt tag for their logo. I think the site title (or even tagline, if a site title isn't set) should suffice.

Replying to joedolson:

I'm inclined to think that the ideal default alt attribute for this case would be the site title. That's likely to be appropriate in most cases - even if it's not a formal match for the text on the alt attribute, it would probably convey the same general information.

[jjcomack]

Tested this patch by going through the following steps:

1. Uploaded an image to use as a logo (700px x 700px) and applied an alt-tag.
2. Was prompted to crop said image, which I did, resulting in the alt-tag changing to the site title.
3. Uploaded a different image (250px x 250px) and applied an alt-tag.
4. Was given the option to skip cropping, which I did. Original alt-tag remains.

Clearly the issue where the alt-tag gets removed post-cropping (and thus replaced thanks to this patch) needs to be addressed as this leads to inconsistent behavior in the user experience. Will probably be best to create a new issue for this.

[sstoqnov]

@jjcomack Thanks for testing.
I agree with you that the cropped image should keep the main image attributes, because rarely you will have cropped images with different alt attributes than the main image. If you still need different attribute, you can change it from editor.

Actually there is a ticket for this issue already and I will try to provide a patch when I have some more free time #37750

[afercia]

Refreshed patch, escapes the alt attribute and makes it filterable. Since the issue with cropped images is already tracked in #37750, I'd say this could go. Some final testing would be nice.

[afercia]

Haven't had the chance to check what happens on multisite, some testing would be nice :)

[sami.keijonen]

I tested the patch also in multisite and default alt is the site title (for sub site and main site) as it should.

[afercia]

Related test Tests_General_Template::test_the_custom_logo needs to be updated.

[afercia]

Update related test.

[afercia]

Refreshed patch to update a related test.

[flixos90]

38768.4.diff​ adjusts the previous test fix, so that _set_custom_logo() is called before accessing the logo's alt text, to make sure it is set prior. It furthermore adds a similar fix to another multisite-specific test that was failing before.

This should be good to go.

[flixos90]

38768.5.diff​ removes the esc_attr() calls, as the attributes should be escaped late as it happens in wp_get_attachment_image().

[flixos90]

38768.6.diff​ makes the new code in get_custom_logo() and its unit tests more precise, by only passing the alt attribute if it is the site name. This ensures the trim( strip_tags( ... ) ) logic in wp_get_attachment_image() still fires correctly for the original alt attribute.

Furthermore it adds a new unit test for a custom logo that has an alt text provided already, to provide better coverage.

I added commit a bit early, but now I actually think it's ready. :)

[afercia]

38768.7.diff​ clarifies a bit the inline comments and cleans-up a test. However, this doesn't work well when updating and already existing custom logo in the customizer, i.e. press "chance logo", select the same image, update the alt attribute: the image get inserted with the previous alt attribute unchanged. /cc @westonruter

[westonruter]

Replying to afercia:

However, this doesn't work well when updating and already existing custom logo in the customizer, i.e. press "chance logo", select the same image, update the alt attribute: the image get inserted with the previous alt attribute unchanged.

This would be expected because the control only re-renders the image when the setting is changed. The setting just contains the attachment ID, so if you select the same image then there is no detected change, as the attachment props aren't being listened to for changes. One way to go about that would be to manually trigger a setting change event when the media library is closed after a selection has been made.

But the problem is deeper than that actually: the media control doesn't ever print out values for the alt attributes. See ​https://github.com/WordPress/wordpress-develop/blob/5ea3c91d57c17fdadbd0ef910b65c4ff8891ff87/src/wp-includes/customize/class-wp-customize-media-control.php#L160-L187

So a couple things would need to change here, out of scope from improving the Custom Logo image's alt on the frontend.

[flixos90]

So a couple things would need to change here, out of scope from improving the Custom Logo image's alt on the frontend.

I agree with @westonruter. I think this ticket can move forward without the Customizer handling it perfectly, this can happen in a separate ticket (although it's probably going to be rather low-priority).

[afercia]

In 40817:

Themes: Improve the theme Custom Logo accessibility.

Uses the Site title as fallback value for the Custom Logo alt attribute when the original alt attribute is empty.

Props sami.keijonen, joedolson, sstoqnov, nobremarcos, gma992, LiamMcArthur, jjcomack.
Fixes #38768.