theme_feed_icon currently sets the alt attribute for the feed icon image to "Syndicate content" and allows for an alternate string to be passed for the title attribute for the image.
1. It is not best accessibility practice to have different strings for the alt and title attributes of an image, see #520704: How should theme_image() deal with screen readers not reading the image title attribute?
2. The alt text is not very meaningful to most screen-reader users. What does "syndicate content" mean?
For 2 I recommend changing the alt text to something more meaningful. Possibly:
1. Syndicate RSS feed of page content
2. RSS Feed
3. Syndicate content: RSS Feed
Any other suggestions?
theme_feed_icon http://api.drupal.org/api/function/theme_feed_icon/7
Comment | File | Size | Author |
---|---|---|---|
#22 | 520734-Revisions.patch | 913 bytes | webernet |
#20 | 520734-Revisions.patch | 809 bytes | webernet |
#17 | issue-520734-feed-icon-3.patch | 743 bytes | Everett Zufelt |
#10 | issue-520734-feed-icon-2.patch | 900 bytes | Everett Zufelt |
#3 | issue-520734-feed-icon-1.patch | 854 bytes | Everett Zufelt |
Comments
Comment #1
Everett Zufelt CreditAttribution: Everett Zufelt commentedtagging
Comment #2
annmcmeekin CreditAttribution: annmcmeekin commentedHow about dropping the jargon of Syndicate (because nobody really uses feeds to syndicate content these days, and hardly anyone in the real world understands what that means in relation to RSS feeds) and rewrite the alt text to something along the lines of:
"Subscribe to [page/site] RSS feed"
or even just "Subscribe to [context]"
Comment #3
Everett Zufelt CreditAttribution: Everett Zufelt commentedRolled a patch re: Ann's suggestion above.
This patch
1. Replaces the $title param with $description in the function definition.
2. Removes the title attribute from the feed icon img element.
3. Replace the alt attribute of the feed icon img element, which was "Syndicate content", with "Subscribe to " + $description.
Comment #4
webchickI like the text changes very much!
Hm. Either we need both $title and $description into this function, or we need to repeat the description as the 4th function argument. The old code passed the $title along as the title attribute of the image. The new code removes the title attribute of the image. Or is that by design?
Also, t('Subscribe to ') . $description) is not translatable. Need to use placeholders like t('Subscribe to @description')
17 days to code freeze. Better review yourself.
Comment #5
webchickComment #6
Brigadier CreditAttribution: Brigadier commentedNot to be picky but could you do
There are modules that do subscription via email and I think there can be confusion.
And I'm not a translation expert so I don't know if the form you used is preferred or if
is easier to translate (though my instinct says the first form is probably better).
Comment #7
Everett Zufelt CreditAttribution: Everett Zufelt commented@webchick
Removing the title attribute is by design, as title is often used to convey information that is not accessible to screen-reader and keyboard only users. We can add it back in using the exact same text as the alt attribute to preserve the tooltip
As far as translation goes, it is not possible to translate a variable, only a literal string.
The calling method would have to translate $description before calling theme_feed_icon(), at which point we are left with:
1. Leave T('Subscribe to ') . $description
2. Get rid of the 'Subscribe to' string altogether, and provide docs to explain how to write meaningful $description's for the RSS icon.
Comment #8
catch@Everett - you can't translate variables, but you need the variable substitution in t() strings to give context -
So this is correct - the translator can tackle everything including the placement of %desc if subject/object/verb order is different in their language since they have access to the entire string in one place.
<?php
Comment #9
Everett Zufelt CreditAttribution: Everett Zufelt commentedRerolled this patch.
1. T() now contains a placeholder for $description to provide context for translaters.
2. Leaving RSS Feed off the end of the alt attribute string, to allow developers more freedom to name these icons the way they prefer.
3. I notice that on the default d7 frontpage that the string being passed as $description is wrapped in em elements, not sure where this is being generated, but it seems like a small tweak for after code freeze.
Comment #10
Everett Zufelt CreditAttribution: Everett Zufelt commentedPatch
Comment #11
mgifford+1
Comment #12
annmcmeekin CreditAttribution: annmcmeekin commented+1 from me too.
Comment #13
Brigadier CreditAttribution: Brigadier commentedPatch looks good to me.
Comment #14
mgiffordthis is great!
Comment #15
Dries CreditAttribution: Dries commentedWell, the patch is a bit confusing because we always pass in the _title_ when we call theme('feed_icon'). Calling it 'description' is confusion because RSS feeds have also a description. So, we're effectively mixing up titles and descriptions. I'd suggest we reroll renaming the variable back to $title.
Comment #16
Dries CreditAttribution: Dries commentedLet's do a quick re-roll.
Comment #17
Everett Zufelt CreditAttribution: Everett Zufelt commented@Dries
Changed variable to $title as recommended in comment #15
Comment #18
Dries CreditAttribution: Dries commentedLooks nice and simple now. Committed to CVS HEAD. Thanks!
Comment #19
webernet CreditAttribution: webernet commentedThe reason the title attribute was added in the first place was to give the image a tooltip description. The alt doesn't do that. There isn't any way to tell what the image links to without inspecting the HTML anymore. This is a regression.
Comment #20
webernet CreditAttribution: webernet commentedRestores the title attribute.
Comment #21
Dries CreditAttribution: Dries commentedFrom an accessibility point of view, if we add the alt-tag, can we remove the title-tag?
Comment #22
webernet CreditAttribution: webernet commentedAlt is required for the image, how about the title on the link? Also fixed % to @.
Comment #23
Everett Zufelt CreditAttribution: Everett Zufelt commented@Dries
Title is not required for accessibility. The title attribute was never intended to be used for important information / tooltip. If the argument is that people won't know what this icon does without using the title attribute then a new approach needs to be found, as the title attribute isn't intended to be used for that purpose.
@webernet
I like the patch in #22, adding alt to the image and title to the link seems like the best approach.
Wondering if $title needs to be check_plain'd / checked to see if it is not empty. If empty then we could use a default string of text.
Comment #25
mgiffordThis patch works as promised and should be committed!
The spam above this comment #24 should be deleted and sballer banned from the site.
Comment #26
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!
Comment #27
Everett Zufelt CreditAttribution: Everett Zufelt commentedSetting to needs work to add documentation.
Comment #28
Cliff CreditAttribution: Cliff commented@Everett, let me know if I can help.
Comment #29
Everett Zufelt CreditAttribution: Everett Zufelt commentedUpdated docs at http://drupal.org/update/theme/6/7#theme_feed_icon-improved-alt-text
Comment #31
Liam Morlandtagging