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.

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wbobeirne’s picture

Status: Active » Needs review
FileSize
1.41 KB
Anonymous’s picture

Issue summary: View changes
FileSize
1.73 KB

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

sheldonreed3’s picture

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

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

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

FileSize
1.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

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

Thanks

Webbeh’s picture

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

#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

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

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

@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’s picture

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

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

wranvaud’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for testing! Moving this to RTBC.

RobLoach’s picture

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