Remove errant .css('height') && .css('width') checks

[jcdarwin]

iFrames have a default width (300px) and height (150px):
http://stackoverflow.com/questions/5871668/default-width-height-of-an-iframe
Becuase of this, .css('height') && .css('width') will
return these default values, meaning that our video ends up with an
aspect ratio of 2:1 rather than 16:9

[davatron5000]

Do have a CodePen of this failing? I'm fairly certain we added both the attr and css() height/width checks on purpose. Also most major video vendors supply a height/width.

[jcdarwin]

Yep, you can see this behaviour at:

http://codepen.io/Tuscan/pen/EVNrOr

The net result is that video embeds without both height and width end up
with an aspect ratio of 2:1 instead of 16:9.

On 29 September 2015 at 04:08, Dave Rupert [email protected] wrote:

Do have a CodePen of this failing? I'm fairly certain we added both the
attr and css() height/width checks on purpose. Also most major video
vendors supply a height/width.

—
Reply to this email directly or view it on GitHub
#223 (comment)
.

[kenhowardpdx]

@jcdarwin @davatron5000 Here's the convo around why we added the check: 8054992

[jcdarwin]

Thanks for that -- for any iFrame (at least in Chrome), the condition:

if ((!$this.css('height') && !$this.css('width')) &&
(isNaN($this.attr('height')) || isNaN($this.attr('width'))))

will never evaluate true, as iFrames have a default style for width (300px)
and height (150px), which is retrieved by $this.css('height') /
$this.css('width').

Refer:

http://stackoverflow.com/questions/5871668/default-width-height-of-an-iframe

http://codepen.io/Tuscan/pen/EVNrOr

What we really need is a way to ignore these default values -- will
investigate further.

Jason

On 30 September 2015 at 06:32, Ken Howard [email protected] wrote:

@jcdarwin https://github.com/jcdarwin @davatron5000
https://github.com/davatron5000 Here's the convo around why we added
the check: 8054992
8054992

—
Reply to this email directly or view it on GitHub
#223 (comment)
.

[jcdarwin]

Had further look/think about this -- as per the amended codepen (first example in http://codepen.io/Tuscan/pen/EVNrOr) we still think there's a problem with having (!$this.css('height') && !$this.css('width'), as this means that embedded videos with no height/width end up being sized as 2:1 rather than 16:9 (because of the defaults applied to an iframe: http://stackoverflow.com/questions/5871668/default-width-height-of-an-iframe).

We've also made changes to ensure that videos with height / width specified in the style attribute are also handled correctly.

At the moment, fitvids will create a responsive container (with padding), but the style attribute values wil constrain the size of the video, meaning that there's a lot of whitespace (see the second example in http://codepen.io/Tuscan/pen/EVNrOr).

Our change honours the aspect ratio implied by the height / width in the style attribute, and once made responsive, removes the height / width from the style attribute to ensure the video fills the container properly.

[kenhowardpdx]

@jcdarwin I'm ok with this latest change. Detecting that the style attribute has both a height and width is better than detecting if the element has a style and width considering the base style of an iframe.

@davatron5000 You can make the final decision.

[kenhowardpdx]

@jcdarwin Are you missing an open paren on that if statement?

[kenhowardpdx]

NM. My eyes are playing tricks on me 😬