Youtube embeds lack title attribute

[morriscountynj]

I use a service called Siteimprove to monitor for accessibility issues. They pointed out an issue -- when you embed a Youtube video via a link, the code comes out something like this

<iframe width="960" height="540" src="https://www.youtube.com/embed/iGddWyoWkyg?feature=oembed"
frameborder="0" allowfullscreen=""></iframe>

However, Siteimprove gives that an A-level error:

The iFrame is missing a description
The iFrame has no 'title' attribute or the 'title' attribute is empty.
Provide the frame with the attribute title=”” and add a description of the content in the title.

Is there any way to add a field to enter a title for the iframe?

See this page for an example: ​Example

[swissspidy]

Hey there,

Welcome to WordPress Trac and thanks for your report!

WordPress embeds have a title attribute for exact these reasons. Unfortunately, YouTube doesn't add a title attribute themselves. oEmbed fetches the <iframe> code directly from YouTube. See ​https://www.youtube.com/oembed?url=http%3A//youtube.com/watch%3Fv%3DM3r2XDceM6A&format=json for an example.

It seems like Drupal fixed this on their own by adding a custom title attribute, see ​https://www.drupal.org/node/2085749. Apparently they also added a name attribute: ​https://www.drupal.org/node/2129317. We could certainly do the same, although I'd love to see YouTube fixing this on their end.

[afercia]

Adding the accessibility focus, since iframe elements should really have a title attribute (and it's the only valid case of title attribute usage WordPress currently recommends, see all the work done so far about title attributes).

[afercia]

We should try a workaround for 4.9.

[joedolson]

Note: ticket filed with Google to try and get this changed.

​https://groups.google.com/forum/#!searchin/accessible/oembed%7Csort:relevance/accessible/d-5z87lOWeY/MW1zLZiOCAAJ

[tfrommen]

Even though noone responded to the Google ticket, this seems to have been fixed already, see the example embed link @joedolson posted: ​https://www.youtube.com/oembed?url=http%3A//youtube.com/watch%3Fv%3DM3r2XDceM6A&format=json

However, when I paste the URL (i.e., ​https://www.youtube.com/watch?v=M3r2XDceM6A) into the editor, the generated output still lacks the title attribute:
<iframe src="https://www.youtube.com/embed/M3r2XDceM6A?feature=oembed" allowfullscreen="" height="473" frameborder="0" width="840"></iframe>

Am I doing anything wrong, or did I misunderstand what the example link was about? Can you help, @swissspidy?

[SergeyBiryukov]

Replying to tfrommen:

this seems to have been fixed already, see the example embed link @joedolson posted: ​https://www.youtube.com/oembed?url=http%3A//youtube.com/watch%3Fv%3DM3r2XDceM6A&format=json

There's still no title on the <iframe> element in that response, only as a separate value in the array.

[afercia]

Seems a fix from upstream isn't going to happen. @swissspidy any chance to make a patch for this?

[swissspidy]

It would be great if we could first check other oEmbed providers to see whether this is a broader problem. If so, we could apply a more general solution, not just one tied to YouTube.

[afercia]

It would be great if we could first check other oEmbed providers to see whether this is a broader problem

I've made my part and checked Vimeo :) it adds a title attribute on the iframe with the video title. Just the title of the video. It doesn't say the iframe content is about a video though. So, for example, when embedding a video that ironically has a title like ​The Bigger Picture, the iframe title doesn't let me actually understand what the iframe content is about.

[afercia]

Beta 1 is in 2 days. Punting as per today's a11y bug scrub.

[bamadesigner]

I wrote some code to solve this issue for my websites. I don't mind submitting a patch to get things started, if it's still needed.

Here's how I solved with a filter. Basically it looks for the title in the embed data array and as a possible existing title attribute. If anything, even if the title does exist in the iframe markup, it allows for it to be filtered. If it doesn't exist, it allows for it to be created from the data array or via filter.

​https://github.com/wpcampus/wpcampus-network-plugin/commit/a48e4c81b1233a7051139046a91b3a19777ea902

[afercia]

@bamadesigner please do feel free to go ahead :)

[bamadesigner]

On it!

Replying to afercia:

@bamadesigner please do feel free to go ahead :)

[bamadesigner]

Here you go @afercia! I apologize for the delay. #life

for40245.diff​

[afercia]

@bamadesigner thanks for the patch! Seems a very good start to me. Couple minor things: the keyword public should be removed and the title attribute value should be escaped with esc_attr().

Moving to 5.0 for consideration. Review and feedback from the Embeds component team would be greatly appreciated.

[bamadesigner]

Updated patch for 40245

[bamadesigner]

Ah! Great catch. I've uploaded the new patch. Thanks!

for40245updated.diff​

[swissspidy]

Why is an existing title attribute being replaced in that patch? Shouldn't this only add missing attributes instead of overriding the provided ones?

[bamadesigner]

It's being replaced because this patch also introduces a "oembed_title" filter.

[swissspidy]

If we‘re going to add that, I think that filter should be renamed to oembed_iframe_title_attribute or something. Also, it needs to be documented.

[ocean90]

#43647 was marked as a duplicate.

[TomHarrigan]

patch refresh

[TomHarrigan]

patch refresh - comment fix

[TomHarrigan]

patch refresh - proper diff

[TomHarrigan]

patch refresh - proper diff for real this time...

[afercia]

@swissspidy mind owning this ticket? As I know you have some expertise with embeds :) Would be great to make it land in 5.1.

[pento]

This could do with a bunch of unit tests.

[swissspidy]

Added a new patch with unit tests.

I think $pattern needs to be improved so that only iframes will be touched.

[afercia]

@swissspidy when you have a chance: what's left here? Ten days left to 5.2 beta 1. I'd really appreciate some feedback to not punt this ticket again to next release. Thank you :)

[swissspidy]

I think wp_filter_oembed_title_attribute needs some adjustments to make sure it only targets iframes. Otherwise anyone filtering oembed_title_attribute would add title attributes to any element.

Maybe using a regex like this: <iframe[^>]*?title=["']([^"']*)["'][^>]*?>

I'm not a regex expert though :-)

[afercia]

@swissspidy thanks. Who's the best person who can help you moving this ticket on? :)

[jrf]

Did I hear regex expert ? Want me to take a look ?

What are the specs and have we got test data for things which should and shouldn't match ?

[afercia]

@jrf thanks! I think @swissspidy has more context but I'll try to describe what I see in the patch the best I can.
We need to add a title attribute with the video title to the HTML returned from an oembed provider response. The patch already checks if the oembed is a video.
After that, it uses the following regex to search for an existing title attribute in the returned HTML:
$pattern = '/title\=[\"|\\\']{1}([^\"\\\']*)[\"|\\\']{1}/i';

that could match any title attribute in any element in the returned HTML. Instead, we need to search for a match only within the <iframe ...> HTML tag, if any.

[jrf]

@afercia Ok, I'll have a look. I presume you'd be happy to see other improvements to the regex ? It currently looks very unstable with regards to the quote matching to me.

[afercia]

@jrf please do feel free to make this regex yours :) I totally trust your judgment.

[jrf]

Improved regex line:

<?php
$pattern        = '`<iframe[^>]*?title=(\\\\\'|\\\\"|[\'"])([^>]*?)\1`i';

Important: $matches[1] on line 802 and 803 will need to be changed to $matches[2] when using this regex.

Test data used:

<img src="..." title="title"/>
'<iframe src="..." title="title" width="324" height="auto"/>'
"<iframe src='...' width='324' height='auto' title='title' ></iframe>"
"<iframe src=\"...\" title=\"title\" width=\"324\" height=\"auto\"/>"
'<iframe src=\'...\' width=\'324\' height=\'auto\' title=\'title\' ></iframe>'
"<iframe src=\"...\" title=\"title's with apostrophe\" width=\"324\" height=\"auto\"/>"
"<iframe src='...' width='324' height='auto' title='title \"quoting\" something' ></iframe>"

Aside from the first one which shouldn't match, all the others now match the title correctly.
Just for fun, try using the old regex against this data set.

Let me know if there are any peculiarities in how the oembed data is received which should be taken into account additionally.

[jrf]

FYI: Regex tests ​https://regex101.com/r/M8epXh/2

[afercia]

Thanks @jrf! Then, I guess there's the need for a second (simpler) regex to use for the replacement at line 823. Would something like the following be OK or can we use something simpler?

<?php
'`title=(\\\\\'|\\\\"|[\'"]).*?\1`i'

[afercia]

@swissspidy I wonder if we should prepend something to the title attribute string. With the current patch the title attribute would be, for example:

<iframe title="House of Commons debate on extension of Article 50" ...

Users will be informed about the topic but still they don't know it's a video :) How about prepending Video:

<iframe title="Video: House of Commons debate on extension of Article 50" ...

[jrf]

@afercia Good question. To be fair, I would just simplify that code block like so:

Original code:

<?php
if ( $has_title_attr ) {
        $result = preg_replace( $pattern, 'title="' . esc_attr( $title ) . '"', $result );
} else {
        return str_ireplace( '<iframe ', sprintf( '<iframe title="%s" ', esc_attr( $title ) ), $result );
}

return $result;

Suggested alternative:

<?php
if ( $has_title_attr ) {
        // Remove the old title.
        $result = preg_replace( $pattern, '', $result );
}

return str_ireplace( '<iframe ', sprintf( '<iframe title="%s" ', esc_attr( $title ) ), $result );

[afercia]

@jrf to my understanding:

<?php
        // Remove the old title.
        $result = preg_replace( $pattern, '', $result );

would remove also the <iframe start tag :) the full match of $pattern includes anything starting with <iframe (which we need for the first check) and a title attribute.

[jrf]

@afercia Shoot, yes, you're right. I shouldn't be doing twenty things at the same time.

Ok, what about this: (though you run the risk of replacing the title in other HTML elements too if it would be exactly the same)

<?php
if ( $has_title_attr ) {
        // Remove the old title.
        $result = str_replace( ' title=' . $matches[1] . $matches[2] . $matches[1], '', $result );
}

return str_ireplace( '<iframe ', sprintf( '<iframe title="%s" ', esc_attr( $title ) ), $result );

[joedolson]

Re: preprending "Video" - what about prepending "YouTube"? That would give users more definite context, and possibly tell users of YouTube how the controls will work, where 'Video' would still require them to discover that.

[afercia]

I shouldn't be doing twenty things at the same time.

😄 Thanks for your help @jrf ! Yep, I'd tend to think improving the conditional instead of using one more regex would be best.

[swissspidy]

Re: preprending "Video" - what about prepending "YouTube"?

Well, the patch currently applies to all iframe embeds, regardless of their source. It could be a YouTube video, Vimeo video, tweet, poll, etc. We could use the provider_name field in the oEmbed data if it's available and prepend that to the title. Would that work?

[afercia]

the patch currently applies to all iframe embeds, regardless of their source

The patch only tries to set a title if the embed type is of type rich or video: it returns early if ! in_array( $data->type, array( 'rich', 'video' )

Looks like Twitter is rich but doesn't use an iframe (any longer?) and uses the shadow DOM instead.

Regardless, this doesn't help us. We could certainly prepend Video: when the type is video, then the regex searches for an iframe but it's not possible to predict what a rich embed with iframe actually is: it could be a Tweet, a WordPress post, whatever.

Anyways, having an iframe title is better than not having it :)

[afercia]

40245.2.diff​ refreshes the patch:

• uses the regex and final replacement suggested by @jrf
• renames all the things to clarify it's about the iframe title attribute
• running grunt precommit updated also the wp-api-generated.js file, adding the title attribute to "Una cancion para programadores". Didn't know about that song :)

Some testing and review would be appreciated. Reminder: the oembed responses get saved as post meta. When testing, either use a new video to embed each time or delete the _oembed ... entries from the database.

[jrf]

@afercia Looks like your new patch is missing the unit tests which were included in @swissspidy's patch....

[afercia]

Uh thanks @jrf. I even spent some time to rename stuff in the tests... and forgot to svn add. #blamesvn.
Patch updated.

[swissspidy]

Added a new test_filter_oembed_iframe_title_attribute_does_not_modify_other_tags test in 40245.3.diff​.

[afercia]

In 44942:

Accessibility: Ensure embed iframes have a title attribute.

Screen reader users rely on the iframe title attribute to describe the contents of iframes. A meaningful title attribute allows to quickly identify the iframe content, so users can determine which iframe to enter and explore in detail or skip if desired.
Note: this is the only case where a title attribute is required for compliance with the W3C Web Content Accessibility Guidelines (WCAG).

• checks for oEmbed response of type video or rich
• checks if they use an iframe
• fetches the title (if any) from the oEmbed response
• adds the title to the embed iframe

Props bamadesigner, TomHarrigan, swissspidy, jrf, afercia.
Fixes #40245.