WordPress.org

Make WordPress Core

Opened 10 months ago

Last modified 6 weeks ago

#46124 accepted defect (bug)

Allow setting custom image alt text in custom header image

Reported by: webmandesign Owned by: audrasjb
Milestone: Future Release Priority: normal
Severity: normal Version: 5.0.3
Component: Media Keywords: needs-patch
Focuses: accessibility Cc:
PR Number:

Description

Currently when using custom header feature and displaying an image in custom header, the get_header_image_tag() function sets the custom header image alt text to get_bloginfo( 'name' ) only.

It is not possible to override the alt text by setting a custom alt text for the image in Media Library.

To improve accessibility, this should be possible.

Attachments (2)

ticket46124.diff (2.0 KB) - added by webmandesign 10 months ago.
Patch fixing ticket #46124
46124.patch (596 bytes) - added by mukesh27 10 months ago.
Updated patch.

Download all attachments as: .zip

Change History (40)

@webmandesign
10 months ago

Patch fixing ticket #46124

#1 @webmandesign
10 months ago

  • Keywords has-patch added

@mukesh27
10 months ago

Updated patch.

#2 @webmandesign
10 months ago

Hi @mukesh27,

Thank you for the patch. However, please note that $header->attachment_id is not always set and you are actually now removing the default get_bloginfo( 'name' ) alt text.

Please note that I have already added my own patch for the issue which is backwards compatible with get_bloginfo( 'name' ) alt text.

#3 @mukesh27
10 months ago

Hi @webmandesign,

If attached image don't have it's alt value than no need to add get_bloginfo( 'name' ) means it's not necessary to have bloginfo when alt it empty.

#4 @webmandesign
10 months ago

Hi @mukesh27,

Thank you. In that case, if there is no need for this backwards compatibility, your solution is much better. I think you don't even have to set $alt variable, you could just use 'alt' => (string) get_post_meta( $header->attachment_id, '_wp_attachment_image_alt', true );

However, please note that as it is possible to filter the image URL via get_header_image(), in my solution I also check if the image is actually the one that should have the alt text - see lines 1096 to 1101 in my patch.

Could someone from core team provide feedback if this is OK please?

#5 @webmandesign
10 months ago

By the way, if it is content image, which custom header in most cases is, the alt should be set to some value. So, it makes sense there is get_bloginfo( 'name' ) used as fallback now.

#6 @webmandesign
10 months ago

  • Keywords 2nd-opinion added

#7 @audrasjb
10 months ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

#8 @audrasjb
10 months ago

  • Keywords needs-refresh added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to 5.2
  • Status changed from reviewing to accepted

Hi,

The idea looks good at a glance. I think we should also add a fallback. Moving this to 5.2.

#9 @webmandesign
10 months ago

Hi @audrasjb,

Thank you for feedback and for accepting the fallback patch.

Could you please also provide some feedback on ticket #46134 as that is somehow related? Thanks!

#10 @afercia
10 months ago

Actually, in most of the cases the header image is purely decorative and in that case it would need an empty alt="" so it gets ignored by assistive technologies.

Only if the image adds some relevant information, the alt attribute would need to have some meaningful content to communicate the image purpose.

Can't recall off the top of my head why it is always populated with the site title. Worth reconsidering because the choice should be in the hands of the site owner but it should be made very clear that in most of the cases the alt attribute should be empty.

Also noting, as in the case of the logo image in #46127, the cropped header image is not listed in the grid view.

#11 @webmandesign
10 months ago

Hi @afercia,

Yes, you're right, all cropped images WordPress creates are affected by #46127.

In the case of empty alt text the fix could be as simple as setting it directly in $attr array as mentioned above.

But there is still an issue here as the custom header image URL could be different from attachment ID in get_header_image_tag(). That way the alt text is pulled from the attachment ID image, yet a different image could be actually displayed... (Look at the first 2 lines of the function code.)
My patch takes care of this issue, but the possibility of different image URL indeed complicates things here, which also relates to other ticket I've reported (#46134).

I think the alt text for the custom header image should be pulled from the actual image if possible and not overwritten or set empty by default. That way user can decide what to do.

As an example, in my theme on single post I'm overriding the custom header image with post featured image. And there is not an easy way of setting correct image alt text for this image now for me if I am using native WordPress functionality only. (I will solve this in my themes with a custom code eventually, but dealing with the issue I've spotted this discrepancy in WP core code.)

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


8 months ago

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


8 months ago

#14 @audrasjb
8 months ago

  • Keywords 2nd-opinion added
  • Milestone changed from 5.2 to 5.3

Hi,

The ticket is still relevant but will need some further work and/or feedback.

That would be great to get some feedback from Media component maintainers and from the Theme Team as well.

I'm moving the ticket to the next release because it's too late for 5.2, but I'll try to ask for feedbacks with the concerned people in the next couple of weeks to make sure the ticket can be handled in 5.3.

#15 @webmandesign
8 months ago

Thank you @audrasjb!

This ticket was mentioned in Slack in #core-media by anevins. View the logs.


4 months ago

#17 @anevins
4 months ago

Mentioned in today's Media meeting and Mike's thoughts:

1) It’d be ideal if it core longer filled alt if it shouldn’t have an alt.
2) I wonder where the ideal place for this to be exposed to users, if at all, would be — whether alt of the parent image would be appropriate, or not at all, for instance.
3) I wonder if anyone is already working around this and how.

My thoughts:

Although @afercia mentions in most use cases the purpose of the header image is entirely decorative, I've found that most users in the wild actually put information inside header images - from my experience on the support forums.

I would like to see the alt text blank and make it the content editor's responsibility, but I'm happy with it prepopulated as long as it can be left empty too. Which ever is easier.

#18 @mikeschroder
4 months ago

I very poorly worded 2) in the chat. A bit of clarification:

  • I think users should be able to set the header alt somehow, but am uncertain of the optimal UX for that.
  • We could search for/retrieve the alt of the parent image, but that alt might not be correct for the cropped header.

#19 @mikeschroder
4 months ago

  • Keywords needs-design-feedback added

This ticket was mentioned in Slack in #core-media by mike. View the logs.


4 months ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


3 months ago

#22 @karmatosed
3 months ago

This was discussed in this week's design triage. Just to get a few confirmations, you can right now edit alt text on custom headers. This is why having a UI for this seemed to already exist. In this ticket is that being recommended to go?

#23 @afercia
3 months ago

Not sure there's an UI to edit the custom header image alt text.

Testing with Twenty Seventeen

  • customiser > Header Media > Add new image
  • the media modal dialog "Choose Image" opens
  • select an image
  • I can edit the alt text in the right sidebar > Attachment details
  • however, once inserted, the header image has always the site name as alt text, in my case: alt="WordPress develop"

@nrqsnchz checking the discussion on Slack > design I see you mentioned you successfully set the alt text, can you please share more details?

Worth also mentioning that even if this worked, editing the alt text from the media modal dialog isn't ideal because it changes the alt text "globally". There should be a UI to edit the alt text only in for the Header context.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


3 months ago

#25 @nrqsnchz
3 months ago

I think you're right @afercia and I was not thorough when testing. While I was able to set an alt text to the image:

https://cldup.com/05a3ZVyEkA.png

the alt text does not appear on the actual HTML:

https://cldup.com/Vlc3MD6c0N.png

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


2 months ago

#27 @afercia
2 months ago

  • Keywords 2nd-opinion needs-design-feedback removed

Discussed during today's accessibility bug-scrub. Agreed the UI is misleading users because:

  • when inserting the header image, the UI shows the alt text field with the text set by the user
  • but then, the alt text that is actually printed out in the markup is the site name

Considered two options:

  • just let the header image take the alt text from the media library: no need for a new UI
  • the above + a new UI to override the alt text from the Media Library so that users have the ability to set a custom alt text without changing the “global” one

The first option would be a good first step and doesn't need design.

The second option could be implemented at a later stage and would need design for a new UI. Will split in a separate ticket so going to remove the needs-design-feedback keyword.

Also, users should be allowed to set an empty alt text alt="". Not sure there should be a fallback to the site name.

Last edited 2 months ago by afercia (previous) (diff)

#28 @audrasjb
2 months ago

The above sound nice to me. I'm all in favor of using the alt text of the existing image in the media library.
However, I tested the last patch from @mukesh27 and it's not working for cropped images since once cropped, the alt text doesn't exists anymore.

#29 @webmandesign
2 months ago

@audrasjb As far as I know, you should be able to set the cropped image alt text (and other options) in list view of media library. Cropped images are available only there, but I actually logged a ticket for this behavior too.

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


2 months ago

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


8 weeks ago

#32 @afercia
8 weeks ago

  • Type changed from enhancement to defect (bug)

Discussed during today's accessibility bug scrub. Agreed to change this ticket from "enhancement" to "bug" as anything related to the image alt attribute is of fundamental importance for accessibility.

#33 @audrasjb
7 weeks ago

  • Keywords needs-patch added; has-patch needs-refresh removed

This ticket was mentioned in Slack in #core-media by mike. View the logs.


7 weeks ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


7 weeks ago

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


7 weeks ago

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


6 weeks ago

#38 @audrasjb
6 weeks ago

  • Milestone changed from 5.3 to Future Release

This ticket still needs some work and improvements. Moving it to the future release.

Note: See TracTickets for help on using tickets.