An accessibility review for a site I am working on identified some issues with the iframe tags output by the Media YouTube module.
To improve accessibility, there should be a title attribute to help screen reader users, and alternative content (inside the iframe tags) for browsers that don't render iframes.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | media-youtube-accessibility-followup-1823096-5.patch | 975 bytes | David_Rothstein |
| #3 | media-youtube-accessibility-1823096-3.patch | 2.79 KB | RobW |
| #1 | media-youtube-accessibility-1823096-1.patch | 1.94 KB | David_Rothstein |
Comments
Comment #1
David_Rothstein commentedHere is a patch.
Comment #2
RobW commentedThis sounds good. I think we can do one better by pulling in the YouTube title and maybe the description for the title and alternate content respectively.
Comment #3
RobW commentedIt looks like we'd need to parse the YouTube video page to get the description from the meta tag, so that's out for now. I revised your patch a little to use the YouTube file entity's name for the title, and "Video of {filename}" for the alternate content. Committed!
http://drupalcode.org/project/media_youtube.git/commit/c3ad415
Comment #4
RobW commentedComment #5
David_Rothstein commentedCool, thanks for the improvement!
However, that patch looks like it contains a security hole (and isn't good for translatability either). So, here's a followup to fix that...
The first line I changed here might actually be better off as strip_tags() rather than check_plain() (since it's a title attribute); but in any case it definitely needs one of them.
Comment #6
RobW commentedGood catch and good improvements. But, after some testing I see a major problem with my version of the patch.
$variables['id']isn't the file id (fid). On my test installation it's permanently '1'.I'm pretty sure it's supposed to be the file id. If we can fix whatever's malfunctioning in a day or two I'll commit it all together, otherwise I'll roll back and commit the first patch with a static "YouTube video content" placeholder for the title and alternative content.
Comment #7
RobW commentedPulled the file object in during preprocessing with
file_uri_to_object(). Still not sure what's up with$variables['id'], but this works for now. Committed: http://drupalcode.org/project/media_youtube.git/commit/f8b2488Comment #8
RobW commentedFollowup information from Aaronwinborn on IRC: "
$variables['id']will auto increment according to how many times the theme function is called on a page."Not actually related to the fid at all. Good to know.