I recently played with a design idea which would show several lists of content each with its own feed.
I quickly realized that the theme_feed_icon() function only accepted the feed url giving me a page with several feed icons each saying "Syndicate content".
On a page with only one feed icon the text "Syndicate content" might be informative enough for users using screen readers given the context.
But on a page featuring several feed icons the user might loose the context due to other aspects the page layout.
I did a quick mod for the theme making the alt and title optionally modifiable. I later added the possibility to also change the path for the icon.
In the patch I have done the same for theme_xml_icon().
The patch only changes theme_feed_Icon() and theme_xml_icon() in includes/theme.inc.
The patch will not break current use of theme_feed_Icon() and theme_xml_icon() since behavior is the same as before when only given an url.
The patch is based on head:
// $Id: theme.inc,v 1.313 2006/08/30 07:37:13 drumm Exp $
Before:
function theme_feed_icon($url) {
if ($image = theme('image', 'misc/feed.png', t('Syndicate content'), t('Syndicate content'))) {
return '<a href="'. check_url($url) .'" class="feed-icon">'. $image. '</a>';
}
}
After:
function theme_feed_icon($url, $alt = t('Syndicate content'), $title = t('Syndicate content'), $img = 'misc/feed.png') {
if ($image = theme('image', check_url($img), check_plain($alt), check_plain($title))) {
return '<a href="'. check_url($url) .'" class="feed-icon">'. $image. '</a>';
}
}
Similar change for theme_xml_icon().
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | theme.inc_.rss_.patch | 1.9 KB | mgifford |
| theme_feed_icon.patch | 1.15 KB | Tiburón |
Comments
Comment #1
Tiburón commentedGetting some sleep before posting might have helped but there is an obvious syntax problem in the code sample which is also in the patch :-(
I did a quick test of the patch and it seemed to work alas I forgot about cache so when the cache had timed out the site broke with a syntax error.
I will fix the code and resubmit a working patch.
Comment #2
drummComment #3
lilou commentedThis feature request is it still valid ?
Comment #4
lilou commentedSee : #172571: Remove theme_xml_icon() and misc/xml.png
Comment #5
mgiffordpossible accessibility issue.
Although from the /blog page there are these two instances:
From the Blog's RSS
From the site's RSS
I think I'd need another instance where it just says "Syndicate content". I do think this could benefit with a more in-depth description though.
Mike
Comment #6
mohammed76hello. subscribing, I think this should be pushed ahead.
I have just checked with three screen readers, one of them read the title attribute while the other 2 read the alt atribute "syndicate content", which just doesn't describe the link enough. would be nice if the alt attribute be context sensitive.
thanks.
Comment #7
mgiffordWould be so nice if screen readers had a standard that they all complied with. So the problem here is that some screen readers read the alt tag of the image and others use the title tag, but it isn't consistent, so we need to push both forward with meaningful content.
Makes more sense now. +1
I think if governments invested a portion of what they do now into producing a cross platform, top of the line, open source screen reader that was based on open standards and had evaluation metrics for developers that we'd all be saving a lot of time & $$. That being said, don't see that happening any time soon.
Unfortunately, looks like the patch needs to be updated, 1 out of 1 hunk FAILED -- saving rejects to file includes/theme.inc.rej
Comment #8
mohammed76hello again.
I asked the Mozilla accessibility
QA, Marco Zehe about what is prefered alt or title, or both, and he gave the following answer:
Mike, I definitely agree with you on the idea of an open source, top of the line screen reader. NVDA is an attempt to do this, and it already has come along way into a mature screen reader. By the way, developers can use this screen reader to have a feel of how their work will be presented by screen readers, and marco has written this excellent article to illustrate how.
hope that helps!
Comment #9
mgiffordThanks mohammed76,
You found an expert, so I'm rolling a patch to fix this.
Not sure about references to - http://drupal.org/node/172571
Other than that there's like a lot of duplicate code and we might be able to just pass one function to the other with an alternate image.
Didn't really want to make too many changes in one patch though. Testable here - http://drupal7.dev.openconcept.ca/blog
I also brought in the logic that you mentioned to hide a duplicate title.
Mike
Comment #10
mgiffordThis is in core now - http://drupal.org/node/172571
MIke