Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
As discussed here #821672: Forum icons not accessible to screen-reader users CSS background images should not be used instead of real images as they pose accessibility problems. You cannot assign alt text to a CSS background image to display the meaning that it conveys.
Comment | File | Size | Author |
---|---|---|---|
#23 | 821672-forum-icon-remove-old-d7.patch | 4.51 KB | andypost |
#18 | 821672-forum-icon-d7.patch | 3.3 KB | andypost |
#17 | 821672-forum-icon.patch | 3.32 KB | andypost |
#16 | text-image-replace-topic-icons_851496_v3.patch | 2.95 KB | Jeff Burnz |
#12 | text-image-replace-topic-icons_851496_v2.patch | 3.01 KB | Jeff Burnz |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedNot really. It's not a question of using the
<img>
tag or not, but of the presence or absence of a tag that has a text alternative.Generally, we should use *more* CSS background images and *less*
<img>
tags, because the former can be easily overridden in the theme while the latter cannot. Plus:is much more elegant and semantically powerful then:
The markup should make minimal assumptions about how the elements will be displayed to the user (sighted or not).
In the case of the forum containers, the only meaning the icon convey is "I'm a forum container". Not sure there is a need to pass this information to the non-sighted users. But it is needed, we should add an invisible label before the name of the forum container.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedBy invisible label, I mean a:
Comment #3
Everett Zufelt CreditAttribution: Everett Zufelt commented@damien
Isn't what you are proposing the opposite of semantic markup?
Comment #4
mgifford@damien with so much of Drupal core being accessible via theme over-rides I don't know that having the images in CSS makes it that much easier.
In the case of the forums there is inconsistency that would be really annoying to have to deal with. There is an actual image or a css background in the forum to express the same image. Inconsistency is a problem.
I also think that going through and adding
<span class="element-invisible">Hot topic</span>
everywhere that images/hot.png is going to be a real hassle too.I do think it's much more consistent to just add an image with proper alt tags. It's much more common for elements like this.
Comment #5
Jeff Burnz CreditAttribution: Jeff Burnz commentedWell, hmmm, from my perspective as themer I like damien's proposal, sort of, I would use text-image-replacement rather than hiding it with element-invisible and probably use the strong or em element and get rid of the image element altogether. This means we could have a sprite to replace the 6 or so images used by Forum module which is better performance also.
This is basically as simple as
<strong class="<?php print $icon ?>"><?php print t($icon) ?></strong>
in forum-icon.tpl.php and a bit more CSS and combine the seperate images into a sprite - 5 minutes work guys.Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commented@Everett Zufelt: no, on the contrary. The
<img>
tag should only be used for actually displaying an image or photography in a page (like in a catalog or gallery). It should not be used for decorative elements, nor for elements that convey meaning by themselves (like our forum icons).For those elements, we need to proper (non-
<img>
) markup associated with a form of text-to-image replacement.Comment #7
lotyrin CreditAttribution: lotyrin commentedI agree with #6 completely.
We have the idea of something being a "Hot" topic. This should be displayed to both sighted and non-sighted users in the most semantic and simple way, which would be a text label.
If we want to style this text by replacing it with an image, that's great. Those styles belong in CSS.
Comment #8
mgiffordOk.. Let's look at a specific example here. Here's a demo forum http://drupal7.dev.openconcept.ca/forum/3
Forums contain no images:
But Topics do:
Visually they contain the same information... however, there is no indication of the image or any meaning it may represent in the Forum example posted above. It's fine in the topics.
I don't think that the use of:
Would produce any disadvantages for accessibility. However, if we've got examples of the text-image-replacement technique we can test it out and determine if there is a problem.
Consistency is the first goal however.
Comment #9
JacineI agree with Damien. I also think a
<span>
is better than<strong>
in this case.Comment #10
andypost+1 to span with a bit of css, also do not forget to set title attribute make hints visible
Comment #11
Jeff Burnz CreditAttribution: Jeff Burnz commentedOK, so heres a rudimentary patch to get the ball rolling and allow for testing.
This patch:
The TIR technique is basically brand new and leverages our existing element-invisible class, I suspect its based (very) loosely on the old Gilder/Levin method, but doesn't attempt to cover the text (well it does, except the text is now totally hidden). This method will survive the "CSS Off" test but not the "Images Off" test (visually in any case, of course it will still be accessible).
There's some pretty big advantages to using text-image replacement - for one users can override template_preprocess_forum_icon() and set whatever natural language phrases they like - we could push this one better and extend the function to separate the class from the phrase, in which case we could remove the need for drupal_html_class(). Obviously theres a performance saving here also - less http requests and overall 8kb saved in image weight.
The new forum-icons.png image goes in /misc (note the patch does not attempt to remove the original images).
Comment #12
Jeff Burnz CreditAttribution: Jeff Burnz commentedOps, attached the wrong patch - this is the right one!
Comment #13
Jeff Burnz CreditAttribution: Jeff Burnz commentedJust to clarify heres a couple of samples of the HTML output:
Comment #15
MichelleI normally never "subscribe" to anything but I'm making an exception since Advanced Forum uses CSS background images for this and does it differently and I want to see what goes into core and update AF accordingly. So my apologies to everyone I just did a useless bump on. :(
Michelle
Comment #16
Jeff Burnz CreditAttribution: Jeff Burnz commentedre-roll, project space hiccup... give me a break bot :)
Comment #17
andypostJeff Burnz, great work! This looks exactly as #821672: Forum icons not accessible to screen-reader users
Except this one cares about css.
I think we need TITLE attribute because icons could tell nothing to user but hovering above icon user see a hint.
I incorporate descriptive/translatable titles into your patch from #821672
Once icon is not image and never was...
I changed $icon => $icon_class as it is and add $icon_title seems more descriptive as $icon_alt
Comment #18
andypostFixed css rules - no reason in TD.ICON
Comment #19
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis looks pretty good to me.
Comment #20
Damien Tournoud CreditAttribution: Damien Tournoud commentedJust one thing: the quality of the images in #11 seems very poor.
Comment #21
Jeff Burnz CreditAttribution: Jeff Burnz commented@20, yeah, the images are very poor - the sprite needs improvement, not sure what the hell I was doing with Photoshop but that particular sprite came out real bad, be good if someone has the original PSD for those icons.
Comment #22
andypostSuppose patch should include removal of old icons
Also I think we need mark as duplicate #821672: Forum icons not accessible to screen-reader users
Comment #23
andypostPatch with removal of old icons
As I see http://drupalcode.org/viewvc/drupal/drupal/misc/forum-closed.png?view=log
Icons was created by yoroy in #102743-200: new forum icons
And modified in
#377204: new forum icons display in IE6
#552000: Smush core images
Comment #24
Jeff Burnz CreditAttribution: Jeff Burnz commentedThis is looking good, the only suggestion I might make remove some use of the word "icon", for example, its not really the icon_title - nor is it the topic title, but more the "topic status", so I would probably go with something more like
Otherwise the changes are great, good stuff.
Comment #25
aspilicious CreditAttribution: aspilicious commentedIf this ever gets committed the icons have to be re-rendered, they are kinda bad on the outside
Comment #26
NaheemSays CreditAttribution: NaheemSays commentedwhy not do this but without the tiled icons?
That is just a space optimisation (that may or may not exist once the tiles are rerendered to look good in all situations).
Comment #27
Jeff Burnz CreditAttribution: Jeff Burnz commentedGuys, I did not realise this is being discussed over in this issue #821672: Forum icons not accessible to screen-reader users, so given this has become mostly about forum icons I'm thinking we can close this or mark as duplicate and move the patch and discussion over there.
Comment #28
andypostLet's continue in original issue #821672: Forum icons not accessible to screen-reader users
I'm going to re-roll patch but we still need a hq icons sprite
I've ask yoroy via contact form to post a source file of icons to aggregate them into sprites. So let's wait when he does this.
Comment #29
mgiffordI'm re-opening this general issue as the forum isn't the only place that uses this.
Noting #1982236: Markup for: theme_feed_icon()
Comment #30
mgiffordtagging
Comment #31
mgifford#23: 821672-forum-icon-remove-old-d7.patch queued for re-testing.
Comment #33
mgiffordNever mind the patch retest here.. Sorry..
Comment #34
BarisW CreditAttribution: BarisW commentedThis seems to be already fixed in D8, right?
Comment #35
mgiffordThis would work, it's a matter of whether it is consistently applied or not. I re-opened this based on theme_feed_icon(). It was fixed in the forum, but we don't have a generalized (and documented) solution for dealing with non-background issues in an accessible way just yet.
Comment #36
mgiffordI'm going to close this. It's just adding confusion and I should have just opened a new issue instead.