Accessibility enhancements for html output of iframe tag (title, id, name, inner A link)

Reading through the patch for the tests, it looks on the surface that the changes are applied to each attributes section, adding the title for each set.

Weirdly, however, the patch as applied by composer changes the file to add all the title attributes at line 363. See attached screenshot. Could this be the source of the failures?

I'll try to work on this.

Here I have implemented the Vimeo implementation that gets the Video title into the Youtube class and updated the test cases to be expecting the title attribute in the iframe already.

Hope everything is in order.

Thanks!

Fixed the test errors to the best of my knowledge.

Sorry for the wrong interdiff. This is the right one.

Additional things done:

• Fixed the additional errors.
• Changed the Absolute Beginner's Guide to Drupal(XgYu7-DQjDQ) Youtube video sample to another one Drupal 8 Day: Drupal Commerce in Drupal 8(BmFFPozHKFU) due to missing video. I just replaced the Youtube key directly and didn't change any of embed data i.e. a:6:{s:20:"emvideo_data_version";i:5;s:23:"emvideo_youtube_version";i:5;s:8:"duration";i:0;s:8:"playlist";i:0;s:9:"thumbnail";a:1:{s:3:"url";s:43:"http://img.youtube.com/vi/BmFFPozHKFU/0.jpg";}s:5:"flash";a:3:{s:3:"url";s:32:"http://youtube.com/v/BmFFPozHKFU";s:4:"size";s:1:"0";s:4:"mime";s:29:"application/x-shockwave-flash";}}
• Updated the test migration content for the zip files.

Test results

Fixed the error:

error: cannot apply binary patch to 'tests/fixtures/drupal6-emfield-2-x.php.gz' without full index line
error: tests/fixtures/drupal6-emfield-2-x.php.gz: patch does not apply
error: cannot apply binary patch to 'tests/fixtures/drupal7-vef-2-x.php.gz' without full index line
error: tests/fixtures/drupal7-vef-2-x.php.gz: patch does not apply

The patch for this comment is just about the same as in comment #17 but instead of using:
git diff 8.x-1.x > video_embed_field-title_attribute-2913598-19.patch
I now used:
git diff-index 8.x-1.x --binary > video_embed_field-title_attribute-2913598-19.patch

Additional issues

I'm trying to apply the patch to 8.x-1.5 but it doesn't cleanly apply. I get the following errors:

patching file modules/video_embed_media/tests/src/Functional/BundleTest.php
patching file modules/video_embed_media/tests/src/Kernel/DefaultNameTest.php
patching file modules/video_embed_wysiwyg/tests/src/Kernel/FilterTest.php
Hunk #1 FAILED at 47.
Hunk #2 FAILED at 79.
2 out of 2 hunks FAILED -- saving rejects to file modules/video_embed_wysiwyg/tests/src/Kernel/FilterTest.php.rej
patching file src/Plugin/video_embed_field/Provider/Vimeo.php
patching file src/Plugin/video_embed_field/Provider/YouTube.php
patching file src/Plugin/video_embed_field/Provider/YouTubePlaylist.php
patching file src/ProviderPluginBase.php
patching file tests/src/Functional/FieldConfigurationTest.php
patching file tests/src/Kernel/Drupal6EmfieldMigrationTest.php
Hunk #1 succeeded at 35 (offset -5 lines).
patching file tests/src/Kernel/Drupal7MigrationTest.php
Hunk #1 succeeded at 64 (offset -5 lines).
patching file tests/src/Kernel/FieldOutputTest.php
Hunk #1 succeeded at 360 (offset 284 lines).
Hunk #2 succeeded at 361 (offset 243 lines).
Hunk #3 succeeded at 362 (offset 201 lines).
Hunk #4 succeeded at 363 (offset 151 lines).
Hunk #5 succeeded at 364 (offset 111 lines).
Hunk #6 FAILED at 421.
Hunk #7 FAILED at 461.
Hunk #8 FAILED at 503.
Hunk #9 succeeded at 365 (offset -95 lines).
Hunk #10 succeeded at 366 (offset -134 lines).
3 out of 10 hunks FAILED -- saving rejects to file tests/src/Kernel/FieldOutputTest.php.rej
patching file tests/src/Unit/ProviderUrlParseTest.php

Any ideas on how to fix this?

Thanks!

If anyone is interested, I have created a patch as well directed to 8.1.5 because as seen in the previous comment, the 8.x-1.x patch conflicts when applied to 8.1.5 due to other changes already in 8.x-1.x.

This patch contains:

• The fixes from issue https://www.drupal.org/project/video_embed_field/issues/2888653 for the test cases to pass.
• Manual fixing of some patch conflicts due to class changes in https://www.drupal.org/project/video_embed_field/issues/2870151 which aren't in 8.1.5

Please feel free to correct me if I needed to create a separate issue for 8.1.5 and just relating it to this issue.

Thanks!

For the most part, this patch still applies to 8.x-2.x, failing only on various test files. I've stripped out the patches to test files that failed to apply, and the result appears to work.

Patch #21 worked for me. Here is a patch for 8.x-2.x with tests updated to pass.

Patch #22 doesn't apply to 2.0, so rerolling just for that.

Fixing tagging.

I provide a patch in another issue that do the trick https://www.drupal.org/project/video_embed_field/issues/2913925#comment-... (I do that because I work on the project where I also need the patch proposed in this issue

Patch #23 doesn't apply to 2.2, so rerolling just for that!

(disregard the second file, sorry!)

Rerolling, I had some issues applying this to 8.7.7

This issue seems to be addressed in https://www.drupal.org/project/video_embed_field/issues/2913925. I'd favor closing this issue as a duplicate in favor of that issue, which is currently RTBC, and is more feature complete, where as this issue does a basic job. Hopefully the maintainers can chime in to what they would prefer.

This same problem occurs for the youtube provider, here is the patch integrating the 2 changes.

#33 is giving a 403 Forbidden. SSL is required to perform this operation.

I've updated the patch to use ssl instead.

Changing to "won't fix," and copying/pasting the note left by the video_embed_field maintainer on other threads, for anyone else who lands here like I did :)

With the advent of Media in core, the Video Embed Field module has moved to being minimally maintained. Only issues which assist in the migration to Media in core will be committed. To read more about this decision, please see: #3089599: Maintenance status for Video Embed Field now that media is in core.

^dropped wrong file. oops.

this variant of #29 checks runtime for use of http/s by adding getProtocol() to ProviderPluginBase. let the project decide what it's using @mixed protocols.

A11y: add a unique id and name attributes to iframe per field item.

ignore this patch. has bad syntax. friday brain (:

post friday cleanup.
set field max-age 0, complete docblock note for new var.

The Video Embed Filter also needs attributes.

include A tag in iframe. change vimeo to use httpClient.

I am working on one of the 60,000 client sites still using this module.... it needs fixes regardless of suggestion to use core media.

44 patch issue.

i'm skipping lint. but fixing container name (oversight)

handle patch with dev head, not release.

Reroll for Drupal 10 spaceless tag

The YouTube API refuses non-HTTPS requests and it is a bad practice to call external services without HTTPS.

I see some overlap here with #3200253: Add "title" attribute to YouTube, Vimeo, and Playlist embeds which seems a bit better scoped and ahead in terms of implementation IMHO.
Hence, moving to needs work. I will focus on the other issue for our project needs.

Some observations:

1. +++ b/src/Plugin/Field/FieldFormatter/Video.php
   @@ -92,21 +92,22 @@ class Video extends FormatterBase implements ContainerFactoryPluginInterface {
   +   		  $element[$delta]['#cache']['max-age'] = 0;
   

   Why do we need to kill the page cache here?

2. +++ b/src/Plugin/video_embed_field/Provider/Vimeo.php
   @@ -78,7 +72,9 @@ class Vimeo extends ProviderPluginBase {
   +    $title = (($this->oEmbedData() !== FALSE) ? $this->filterCharaters($this->oEmbedData()->title) . ' | ' : '');
   

   This calls oEmbedData() twice, so will fetch twice which could be avoided by caching the response, similar to what is done in #3200253: Add "title" attribute to YouTube, Vimeo, and Playlist embeds

3. +++ b/src/ProviderPluginInterface.php
   @@ -72,11 +73,13 @@ interface ProviderPluginInterface extends PluginInspectionInterface {
   +  public function renderEmbedCode($width, $height, $autoplay, $instance_id);
   

   This changes the method signature, so breaks backwards compatibility which isn't ideal.

4. +++ b/src/Plugin/video_embed_field/Provider/Vimeo.php
   @@ -48,11 +37,16 @@ class Vimeo extends ProviderPluginBase {
   +    $data = \json_decode($requestOembed->getBody()->getContents());
   

   I think it would be best to use \Drupal\Component\Serialization\Json::decode

The Twig spaceless filter is deprecated: https://twig.symfony.com/doc/3.x/filters/spaceless.html

The spaceless filter is deprecated as of Twig 3.12. While not a full replacement, you can check the whitespace control features.

Why do we need to kill the page cache here?

Just noticed this as well, it bubbles and disables cache for the entire page.

I see some overlap here with #3200253: Add "title" attribute to YouTube, Vimeo, and Playlist embeds which seems a bit better scoped and ahead in terms of implementation IMHO.

I agree that #3200253: Add "title" attribute to YouTube, Vimeo, and Playlist embeds has the same goal and is more active, closing as duplicate.