check dimension attribute values on tag

[albell]

see inline comments.

[dmlap]

Do browsers misbehave if they're given invalid dimension attributes?

[albell]

Well, I'm pretty sure browsers just ignore them, so we should too, to prevent the default dimension options from being overwritten by sloppy authoring. (We map these values to CSS, which isn't quite what the browser does, so it's not quite apples-to-apples.) The values I'm mostly concerned about disallowing are: "auto", empty string (possibly generated by a template?), and percentage units.

Easier to handle this in the console than in bug reports. :)

[heff]

Check out #1449. It's a later PR where we discovered the browsers just change NaN and null to zero.

We can't really block 'auto' until #983 is implemented. Auto was a way to allow you to set the player dimensions with CSS, since internally were using inline styles and a default 300x150. It's not great but this PR would kill that option.

[albell]

I'm not as worried about NaN here, although that's generally probably good to check for too. NaN and null aren't what I'm checking for here. The spec's integer requirement on the attribute dimensions is stricter than regular CSS, which allows floats, and will actually render them in some cases with sub-CSS-pixel precision sometimes on retina devices.

See my comments on #983 for why I think hardcoding in a class might be the wrong approach. Let's not wait for that to deal with this?

Auto was a way to allow you to set the player dimensions with CSS, since internally were using inline styles and a default 300x150. It's not great but this PR would kill that option.

Yeah, that's the option I want to kill :) Hijacking the tag's dimension attribute with invalid values to fill a hole in the API feels yucky, and complicates writing tests. Let's just fill the hole!

First of all, authors can use CSS to override inline styles on any of these elements. You just have to use !important. I realize some folks are adamant about never using !important, and I don't want to argue that here.

As I understand it, the scenario you're trying to address (let's call it "PURECSS") by allowing auto as a dimension attribute is that of skilled, competent authors who don't like to use !important in their stylesheets, and therefore want to turn inline css off entirely so that they can style things exactly as they please, with nice light selectors. Am I correct?

But there is no way to deliver all of the responsive use cases, and the height/width API, and the browser 2:1 default aspect ratio without any inline css. It's just impossible. There's too much conditional logic. You can capture a lot of it, but not all.

The only way to address the PURECSS use case that I see is to expose a boolean option, call it 'inlineCss', defaulting to true, that PURECSS authors can set to false if they want to keep inline styles out completely, and thus !important out of their stylesheets. We can add a simple check in the dimension function. I realize some very smart folks are adamant about never using !important, and this would give them what they want, without holding this up. Thoughts?

[heff]

Any thoughts on this since my comment on #983?

Currently this feels like a small piece of a bigger dimension handling change that breaks how some users are currently using dimensions. I think I'd prefer to close this and wrap it into the bigger overhaul of dimensions handing.

[albell]

Yeah, I agree this isn't urgent. I am just trying to parcel the big dimension issues out, and it's tricky because they're so intertwined. This seemed like a simple check to add. FWIW with this PR, and the existing code, authors could still nuke inline css with:

<video data-setup="{'width': 'auto', 'height': 'auto'}>

They just can't do it with:

<video height="auto" width="auto" data-setup="{}>

which is technically invalid. If you like we can put this aside, but I think the point basically stands.

[heff]

Ok yeah, I think we should close this for now, but we'll be sure to kill the auto option and make sure valid numbers are passed in with the larger dimensions overhaul