Remove margin from sandbox iframe to fix size calculation

[westonruter]

The sandboxed iframe was not getting resized enough to account for the margins of the body inside the iframed window. The result, as I'm working on adding sandboxing to the Custom HTML block (#1625), is the content being cut off:

With the fix, it looks like:

Amends #1392

[notnownikki]

I'm not sure this calculates correctly. I'm not sure the original does either, I did notice some cut-off even on twitter embeds...

This is how a twitter embed should look:

but changing to use scrollWidth and scrollHeight results in this:

The problem here is that it appears that the extra space can be clicked in to edit the caption, but it can't be. You have to click in a very specific area that is not visually delimited, as it clearly is in the first picture (the end of the tweet is the end of the embedded html, below it immediately is the caption).

The list of embeds we've been using to test is:

Short tweet:
https://twitter.com/notnownikki/status/876229494465581056

Tall tweet:
https://twitter.com/PattyJenks/status/874034832430424065

Youtube:
https://www.youtube.com/watch?v=PfKUdmTq2MI

Photo:
https://cloudup.com/cl3Oq5MU8Rm

Long tumblr post:
http://doctorwho.tumblr.com/post/162052108791

Perhaps we can add the margin sizes? Or in the document we create for the iframe, reset all margins?

[aduth]

In an alternate implementation of this behavior, we'd switched from offsetWidth / offsetHeight to getBoundingClientRect, which is documented as being more accurate than offset dimensions. At a glance it seems it could only be inaccurate by as much as a single pixel, but might be worth exploring as better accounting for margins.

[westonruter]

@aduth @notnownikki Using getBoundingClientRect to compute the width and height, combined with the computed style to obtain the margin dimensions, seems to do the trick.

[westonruter]

A couple other fixes related to the sandbox component:

• The width and height HTML attributes are integers, so 0b21a93 switches to specifying the dimensions using CSS so that the decimal parts won't be truncated.
• I noticed that the top window was being used instead of the parent window. While this isn't causing a problem right now, it could eventually cause problems if the editor is embedded inside of a window that is itself inside of another frame. See 2f7f4cc.

[westonruter]

@notnownikki how's this look for you?

[notnownikki]

Looking better, but seems to have broken photo embeds from cloudup. The long tweet, short tweet, youtube, and long tumblr embed are all pretty much pixel perfect now, but the test embed from cloudup ( https://cloudup.com/cl3Oq5MU8Rm ) doesn't show anything.

[notnownikki]

Ah, I think I see the problem with photo embeds.

Because some sites return the full size photo for a thumbnail, we set the width of photos to 100%, so they just fill the block and get the right height automatically. But, this change adds style, which sets the width and height to whatever's in the state, which by default is 0. That means the image ends up 0px wide and 0px tall, and gets hidden completely.

[notnownikki]

Ok, I've just tested with removing the style and going back to width and height in the iframe, and using the following for the dimensions calculation in sendResize:

						width: document.body.offsetWidth + parseFloat( computedStyle.marginLeft ) + parseFloat( computedStyle.marginRight ),
						height: document.body.offsetHeight + parseFloat( computedStyle.marginTop ) + parseFloat( computedStyle.marginBottom )

... and things look good for all the embeds and the Custom HTML block.

[westonruter]

@notnownikki Very strange. I don't see why using style on the iframe as opposed to using width/height attributes would be any different. Nevertheless, I can see switching back to HTML attributes does fix the problem. I did retain the getClientBoundingRect part, however, so that we can obtain the fractional dimensions and then ceil that value for the width/height to ensure nothing is cropped, even by 1px.

[notnownikki]

Works with everything I threw at it, including the custom html!

[westonruter]

@notnownikki is there a reason why margin:0 is not being applied to the iframe's body? This also came up in #1658. If we added it, then there would be no need to get the computed style of the body in the first place to obtain the margins.

[westonruter]

Also, I found a core bug with the oEmbed proxy endpoint in core where the maxwidth param is not getting passed along properly. This is partly why the YouTube video appears cut off on the right.

[notnownikki]

@westonruter Looking at #1658, yeah, we should zero the margins. We generate the HTML document for the iframe in trySandbox so just adding the CSS to the <head> seems ok to me.

[westonruter]

Cool, I added it to body[style] since inline styles are also being set with JS.

[notnownikki]

Works great with everything I threw at it again! Happy to see this 🚢