Hi!

I have check a site with W3 validation, and I receive messages like:

  • Bad value 100% for attribute widthon element iframe
  • Bad value 100% for attribute height on element iframe

The solutions is using inline styles instead of attributes.

I have created a patch, to modify the iframe.

CommentFileSizeAuthor
#11 Selection_623.png20.86 KBimyaro
#5 youtube-standard-responsive-dimensions-2630376-5.patch1.38 KBguschilds
#4 youtube_field_2630376.patch629 bytesdtamajon
#2 youtube_field_2630376.patch628 bytesdtamajon
youtube_field.patch635 bytesdtamajon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dtamajon created an issue. See original summary.

dtamajon’s picture

dtamajon CreditAttribution: dtamajon commented
FileSize
youtube_field_2630376.patch628 bytes

Sorry... I was thinking in opening a new issue for frameborder, but maybe it is better to solve all HTML problems over iframe together.

So, I propose a new patch which includes frameborder as inline border style too.

dtamajon’s picture

dtamajon CreditAttribution: dtamajon commented
Title: Bad value 100% for attribute width on element iframe » Use inline css instead than attributes
dtamajon’s picture

dtamajon CreditAttribution: dtamajon commented
FileSize
youtube_field_2630376.patch629 bytes

Previous patch is from a previous wrong version... updated!

guschilds’s picture

guschilds CreditAttribution: guschilds at Chromatic commented
Status: Active » Needs review
FileSize
youtube-standard-responsive-dimensions-2630376-5.patch1.38 KB

Hi dtamajon,

Your patch did not apply. I suspect it may have been manually edited? Either way, I could see what you wanted to accomplish.

My main concern is that I'd rather only use the style attribute for the dimensions when they are % values. I altered the patch to make it work that way, but then I realized that those dimensions are already being set on the iframe of a responsive video in youtube.css. That means we don't need to have the % dimensions in the responsive iframe tag at all. We only need to add dimensions to the tag when they are pixel values, in which case the width and height attributes are still appropriate.

I've altered theme_youtube_video() to accomplish that in the attached patch. I've also kept your border:0; change in my new approach.

Please have a look and let me know what you think. Please also test it thoroughly to ensure everything works as needed for you and that it now passes validation.

Thanks!

For anyone else that comes across this, here's an article about altering YouTube embed tags to pass HTML5 validation: http://www.gtpdesigns.com/design-blog/view/w3c-valid-xthml-and-html5-you...

dtamajon’s picture

dtamajon CreditAttribution: dtamajon commented

Hi,

If you validate a page with a iframe using https://validator.w3.org/ you will see that width and heigth attributes should be avoided. Of course, the patch is working properly for a responsive behavior and validation is ok.

Not sure what happens when using pixel values on the inline iframe... accordingly to the article should pass, but in my cases didn't (maybe because of %). I will try to check with fixed pixels and report feedback.

dtamajon’s picture

dtamajon CreditAttribution: dtamajon commented

Hi,

I have checked and works properly, but with only one consideration. Next I paste the message from the validator:

Self-closing syntax (/>) used on a non-void HTML element. Ignoring the slash and treating as a start tag.

I don't know which is intended for the $alternative_content, but if replaced by ' ', then the check works perfectly (we need at least a black space to ensure Drupal generates the closing tag instead the auto-closing.

guschilds’s picture

guschilds CreditAttribution: guschilds at Chromatic commented

Thanks, dtamajon.

I'm not able to reproduce what you've experienced because the iframe tag always has a value and is never self-closing.

This code constructs the $alternative_content variable:

  $path = 'https://www.' . $domain . '/embed/' . $id;
  $src = url($path, array('query' => $query));

  $player_title = t('Embedded video');
  if (!empty($variables['entity_title'])) {
    $player_title .= t(' for @entity_title', array(
      '@entity_title' => $variables['entity_title'],
    ));
  }

  // Alternative content for browsers that don't understand iframes (WCAG).
  $alternative_content = l($player_title, $src);

And $alternative_content is set to the iframe's value. Any chance you made improper modifications to that code and/or applied the patch incorrectly? Could you try again? If there is a problem with $alternative_content, we'll need to figure out what it is. If there isn't, I believe that would mean this patch works as needed?

guschilds’s picture

guschilds CreditAttribution: guschilds at Chromatic commented
Title: Use inline css instead than attributes » Use inline CSS instead of attributes to pass W3 validation
Gold’s picture

Gold
they/them
Location 20 minutes in the future
CreditAttribution: Gold at Catalyst IT commented
Version: 7.x-1.6 » 7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

Looking good from the point of view of height/width vs the class when responsive.

W3C is reporting that the text inside the iframe shouldn't be there though.

However, from the specific points on this issue I think this one is RTBC.

imyaro’s picture

imyaro CreditAttribution: imyaro at DrupalJedi commented
Status: Reviewed & tested by the community » Needs work
FileSize
Selection_623.png20.86 KB

Hello
I have checked the w3c validator with the content

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
  <head>
    <meta http-equiv="Content-Type" content="text/html;charset=utf-8" />
    <title> Test </title>
</head>
<body>
<div>
<iframe src="https://google.com">Some Text</iframe>
</div>
</body>
</html>

And got error https://www.drupal.org/files/issues/2018-08-10/Selection_623.png

Seems alt text have to be removed from the tag.
Also tag is not selected as self-closing tag (regarging the same validator)

Guess we have to remove value $alternative_content from the tag.