Make videos responsive

[jasmussen]

Embeds are now sandboxed. This broke our responsive video trick. We should restore this in some way. This PR is a work in progress.

In order to finish the work here, we need to inject a few styles into the embed iframe. I could use help with this.

The following CSS should be injected in all embeds:

body {
	margin: 0;
}

The following CSS should additionally be injected into all video embeds:

body,
body > div,
body > div > iframe {
	width: 100%;
	height: 100%;
}

The CSS in the first commit assumes the above styles are present, and positions the caption accordingly:

[ellatrix]

Do all embeds need to be sandboxed? If the provider returns an iframe (no scripts), we can skip sandboxing?

[jasmussen]

Do all embeds need to be sandboxed? If the provider returns an iframe (no scripts), we can skip sandboxing?

¯_(ツ)_/¯

The key here should be code simplicity. The way the embeds work right now, seems to be ⭐️ ⭐️ ⭐️ ⭐️ ⭐️ right now. Outside of a little CSS to normalize the extra margins, it feels solid to me.

[notnownikki]

Right now, the iframe does not load any stylesheets, and it doesn't pick up the parent document's styles either (this is on purpose, as it messes with calculating sizes). We construct a complete html document for the iframe content, so we can have that load whatever stylesheets we need, or we could pass raw CSS to the sandbox component and have it render inline... Which would be preferred?

[jasmussen]

Whether it's passed raw as inline styles or loaded as a stylesheet doesn't matter much to me. Which ever is easier. Given that we want to ambiguate between video and not video, perhaps we could apply that as a classname, and scope the video CSS to the class. That way we could load a single stylesheet for all?

[notnownikki]

To me, the cleanest way would be a single embed stylesheet, and pass a class name to sandbox which gets applied to the body. Sound ok?

[jasmussen]

👍 👍

[westonruter]

See also #1688 which fixes the resizing logic of the iframe to account for any margins inside.

[westonruter]

Note the body margin has been zeroed out in #1688.

Note also a related PR to fix the oEmbed maxwidth: #1876.

[notnownikki]

Taking a look at this again... It's probably simpler to have this css inline in the document we generate, and set a class on body to show which type of embed it is, rather than including extra stylesheets, because of the stuff we have to do to calculate paths to CSS files. I'll start taking a look at this, it shouldn't be too much work.

[notnownikki]

This approach triggers a problem with the resizing sandbox... setting height and width to 100% means they get set to 0 wide and 0 tall, because that's how the sandbox starts out. And the sandbox never shows any content, because the content is set to 100% of the parent, and so never gets bigger and makes the sandbox grow. I'm looking into it more.

[notnownikki]

Further into this... the problem I'm running against is that video iframes have a width and height set. That makes the sandbox resize to those dimensions, which is great, but to get the video to be responsive, it has to be 100%, but the height is already set, so it gets squashed.

If we have height and width set to 100%, that takes on the dimensions of the container, which is 0x0 (because there's no content initially and it has to resize).

This all works fine with images because the image itself contains the size metadata, but videos do not.

I'm thinking we need some video-aware rules that scale for the width, and set the height based on a 16:9 ratio. This is what Vimeo advises, and they have example CSS to do it (which of course doesn't work for us because our container resizes itself!) but we can pass through the content type to the sandbox and have the height calculation work differently for videos.

[notnownikki]

I have this working now in #1921

[jasmussen]

Closing this in favor of #1921 as I've ported the fixes. Thanks Nicola!