mdn / developer-portal

The code that generates the MDN Web Docs Developer Portal.
Mozilla Public License 2.0
61 stars 38 forks source link

iframe on video page #1741

Closed schalkneethling closed 3 years ago

schalkneethling commented 3 years ago

One video pages such as: https://developer-portal.stage.mdn.mozit.cloud/videos/coding-dark-mode-your-website/ the following problem has been identified

  1. The iframe that embed the Youtube video does not have a title attribute.

We have a couple of options here:

  1. Add a title attribute to the iframe element that matches the title of the page, for example:
<iframe width="480" height="270" src="https://www.youtube.com/embed/jmepqJ5UbuM?feature=oembed" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen="" title="Coding a Dark Mode for your Website"></iframe>
  1. Add an id attribute to the h1 and reference it as the label on the iframe, for example:
<h1 id="video-title">Coding a Dark Mode for your Website</h1>

...

<iframe width="480" height="270" src="https://www.youtube.com/embed/jmepqJ5UbuM?feature=oembed" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen="" aria-labelledby="video-title"></iframe>

Option one is most likely the easiest to maintain and avoids the problem of coming up with different id attribute values if there is more than one video on the page.

@valgrimm I believe this is something that will be addressed from the CMS side so, I am assigning it to you. Please reassign if my assumption is incorrect.

valgrimm commented 3 years ago

Yup, I'm cleaning up posts and videos to ensure all the metadata is there, not only on the first page (including descriptions) but social and other. After launch I'll create some tickets to autopopulate this stuff from the initial info added. I'll leave this open until I am done.

stevejalim commented 3 years ago

Stealing this because the iframe template is controlled by wagtail or our oembed provider or YouTube. Will see if I can override it to slot in the extra markup

valgrimm commented 3 years ago

leaving also assigned to me as a reminder if thats ok

stevejalim commented 3 years ago

:+1:

stevejalim commented 3 years ago

Note to self: This looks like we'll need to subclass the regular YouTube embed provider (and document that all others will need it adding too, or make it a generic wrapper for existing providers). Am a bit surprised that this isn't something that has been solved already, so will check to see, too.

stevejalim commented 3 years ago

Related ticket on the Wagtail side: https://github.com/wagtail/wagtail/issues/5982