Improve default width/height settings

[heff]

We have a challenge when trying to mimic the video element with the player div. The video element defaults to 300x150 if no width/height attributes or CSS is applied, but a div defaults to 100% width and zero height, and doesn't respond to width/height attributes. In order to make the div act like the video element and not require width and height, we have to apply a default using CSS.

Currently we're adding the default using the element styles if no width/height is supplied (and no auto is used either as in #359). The problem is still that these styles can't be overridden easily, and the auto setting isn't valid for width/height.

Talking with @mmcc we may have come up with a better solution: Add the 300x150 default to the video-js class. This would allow it to be overridden more easily, either by custom user styles that follow video-js.css in the dom, or through element styles like it currently does when you do supply a width/height.

So the solution would be to kill the default here:
https://github.com/videojs/video.js/blob/v4.3.0/src/js/core.js#L86

And add it here:
https://github.com/videojs/video.js/blob/v4.3.0/src/css/video-js.less#L672

This would need to be tested in different situations.

• No width/height provided at all
• No width/height provided but set by CSS

[albell]

I think hardcoding default dimensions in a class might be the wrong approach. Here's why:

http://jsfiddle.net/277ze2p5/

(I've tested Chrome, FF, Safari, and results are identical. IE, anyone?)

While it's true that a video with no dimension attributes and no metadata will default to 300x150px, that doesn't mean, programmatically speaking, that the width independently defaults to 300 and the height independently defaults to 150. It would be truer to say that the aspect ratio defaults to "2", (which CSS has a harder time expressing). Notice that if you supply a single attribute dimension, in the absence of metadata, the other dimension will scale to preserve an aspectRatio of "2". If you hardcode this in a class, the author will still need to override both height and width with inline CSS in order to get correct behavior. I think we're just stuck with inline css, period. There's no way around it that I can see, if we want to capture the complexity here.

There's no reason why 300 and 150 need to be exposed as options. They're purely internal, really. Maybe the default dimension should just be the string "default". That way we can tell whether either option has been touched as it merges, twice, first through the elements attributes and then through the data-setup config. We have to handle the "only one dimension supplied" case, and be able to tell if the number we're getting is author supplied, default, or generated from the other dimension by a (default) aspect ratio. And if both height and width are supplied, then aspectRatio gets ignored.

So I think this fiddle possibly refutes my earlier idea that we should use "16:9" as the default aspectRatio. Instead, it should be "2:1". Strange, but true. Or else we can preserve that option naming, and use "2" hardcoded somewhere in the logic. Also, we should maybe add simplified tests for the cases in the fiddle that go beyond just the "no dims" case. Sorry for another long post, but this stuff is surprisingly complicated.

[heff]

If you hardcode this in a class, the author will still need to override both height and width with inline CSS

Not true. They just need to use a stronger selector. i.e. if our default 300x150 is under .video-js in video-js.css, they would just need to use div.video-js or video-js.vjs-default-skin or #parentEl .video-js. Or they can just use .video-js in a stylesheet that's loaded after video-js.css. If they set any CSS dimensions they would have a hard time not overriding the defaults.

refutes my earlier idea that we should use "16:9" as the default aspectRatio. Instead, it should be "2:1".

Yeah, that does appear to be true. I wonder if there was any thought towards common ARs in that. It's a difference of 6% from 16:9. I'm having a hard time coming up with consequences for still defaulting to 16:9.

Here's how I would prioritize the specific use cases in all of this, since we probably can't satisfy every one exactly.

Top Priority Use Cases

• User provides no dimensions (attributes or CSS) - The player just needs to still be visible - which the div by default is not - when metadata loads it should update to match those dimensions
• User provides no attributes but uses CSS to set both dimensions - it should match the CSS
• User provides attribute dimensions (both of them) and no CSS - should match the attributes dimensions

I don't think the single dimension attribute use case is a strong one so I wouldn't want to get too complicated trying to solve it. The browser can do some things that we can't to make the 2:1 aspect ration work always. Specifically, a user can set a width of 400px using CSS (no attributes) on the video element and the browser will still make the height 200px. We can't provide that option in the player without also messing up the case where the user also provides a CSS height.

[gkatsev]

You could also use .video-js.video-js to increase the specificity 😸

I've been looking at the html spec but I can't find where it specifies the default dimensions for the video element. However, it seems like 300x150 is the default for a lot of other things in html-land, like canvas and some image things. Given that, I think defaulting to 300x150 and then also to an aspect ratio of 16:9 is reasonable.

[heff]

@gkatsev that's hilarious. Had no idea that worked. :-P

[gkatsev]

Yeah, it's useful because it increases the specificity without changing the semantic meaning of the selector. I used this in the contrib-ads project.

[albell]

I'm having a hard time coming up with consequences for still defaulting to 16:9.

Yeah. Definitely no one every uses "2" as an AR in the wild. So it's pretty weird. On the other hand, it is the spec, and how browsers handle the native element, which we're using everywhere else. So I'm torn. I kind of hate enshrining 16:9 as a standard. A lot of cinematographers hate it! But it is the de facto norm. I'm right on the fence on this. But I'm on board with moving 300x150 into the class.

Seeing the issues mix and mingle complexly, I think a good next step would be to write up an informal test bed, a gist in pseudo code, that explains what exactly we're going to test for. Input/output pairs, based on the discussion so far, that include all the dimensional cases we want to cover and support, and all the edge cases we're concerned about. Put markup, and the condition to test. I would ballpark around 20 cases, conservatively speaking? With this in hand, writing the code will be a lot easier. I can take a whack at this later this week, or maybe someone else can start the process?

[gkatsev]

I believe this has been taken care off with the 5.0 update and the defaults and styles classes.