iframes right now have no indication of what the content is. For screen readers, this can be problematic, as the only information they get is that it's an iframe. Simple patch below, just adds title="Embedded video" to the youtube and vimeo handlers.

CommentFileSizeAuthor
#7 a11y-video-title.patch1.06 KBWebbeh
#5 video_embed_field-1888488-5.patch549 byteskarenann
#2 accessibility_screen_readers-1888488-2.patch1.73 KBAnonymous (not verified)
#1 video_embed_field-title-tag-1888488-1.patch1.41 KBwbobeirne

Issue fork video_embed_field-1888488

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

wbobeirne’s picture

wbobeirne commented
Status: Active » Needs review
StatusFileSize
new video_embed_field-title-tag-1888488-1.patch1.41 KB
Anonymous’s picture

Anonymous (not verified) commented
Issue summary: View changes
StatusFileSize
new accessibility_screen_readers-1888488-2.patch1.73 KB

re-rolled patch against latest 7.x-2.x code

sheldonreed3’s picture

sheldonreed3 commented

The patch seems to work as designed. While it addresses the issue, I think we should have an option to make this title more descriptive/definable, or grab it from the video. If there is interest I can roll some options around in my spare time. Any thoughts?

kleve’s picture

kleve commented

Agree with @sheldonreed3, it should be more descriptive. At least we should give the editor the ability to do so. The title should describe the contents of the embedded content, not that it is an embedded content.

A separate title field on the edit form with the fallback text "Embedded video" if left empty maybe?

karenann’s picture

karenann commented
StatusFileSize
new video_embed_field-1888488-5.patch549 bytes

This is by no means a "fix" but I'm attaching a stop gap for Drupal 8.

Status: Needs review » Needs work

The last submitted patch, 5: video_embed_field-1888488-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Webbeh’s picture

Webbeh
he/him/his
Location Georgia, USA
commented
StatusFileSize
new a11y-video-title.patch1.06 KB

I ended up pursuing the idea pitched in #4, which was to add a quick 'more descriptive' label for YouTube.

It's not a slam-dunk, but since the YouTube scraper already has the mechanism to collect and incorporate this metadata. This patch extends the YouTube handler to add in the title, array[0]['snippet']['title'], as output from video_embed_field_handle_youtube_data($url).

lias’s picture

lias commented

Adding #7 patch worked and removed failing on iframe in audit for Drupal 7.61

Thanks

Webbeh’s picture

Webbeh
he/him/his
Location Georgia, USA
commented
Status: Needs work » Needs review

Realizing this needs a slide for review given #7: https://www.drupal.org/project/video_embed_field/issues/1888488#comment-...

streever’s picture

streever commented

#7 doesn't seem to be valid; it applies to a folder & file that doesn't exist in the latest version of the module?

micahw156’s picture

micahw156
he/him/his
commented

Thanks for the patches!

The patch in #7 basically worked for me, although I made some customizations specific to our environment.

This patch should probably verify $data[0]['snippet']['title'] is set before using it, too. I discovered the hard way that video_embed_field_handle_youtube_data() needs an API key, which we hadn't been using prior to this.

There's another iframe in this file around line 524 to embed Vimeo player. That should probably be addressed, too.

Webbeh’s picture

Webbeh
he/him/his
Location Georgia, USA
commented
Status: Needs review » Needs work

Pushing back to Needs Work for next steps as outlined in #11:

This patch should probably verify $data[0]['snippet']['title'] is set before using it, too. I discovered the hard way that video_embed_field_handle_youtube_data() needs an API key, which we hadn't been using prior to this.

I wonder if we should encourage an agnostic backup if the API key does not exist.

There's another iframe in this file around line 524 to embed Vimeo player. That should probably be addressed, too.

micahw156’s picture

micahw156
he/him/his
commented

@webbeh, that's a good question, and possibly a new issue to update video_embed_field_handle_youtube_data(), which does not verify the key exists before making the API call.

wranvaud made their first commit to this issue’s fork.

wranvaud opened merge request !2

wranvaud’s picture

wranvaud commented
Status: Needs work » Needs review

Changing status to needs review. Please see merge request https://git.drupalcode.org/project/video_embed_field/-/merge_requests/2.

micahw156’s picture

micahw156
he/him/his
commented

+1 for the merge request from wranvaud. I was (finally) able to test these changes, and everything worked as expected.

wranvaud’s picture

wranvaud commented
Status: Needs review » Reviewed & tested by the community

Thank you for testing! Moving this to RTBC.

robloach’s picture

robloach
he/him
commented

Those following along for Drupal 8, the solution is over at https://www.drupal.org/project/video_embed_field/issues/3200253#comment-... .