search

videojs / vtt.js

A JavaScript implementation of the WebVTT specification, forked from vtt.js for use with Video.js
http://dev.w3.org/html5/webvtt/
Other
71 stars 43 forks source link

Possible incorrect mappings for positionAlign values #51

Open jonknowles opened 4 years ago

jonknowles commented 4 years ago

In cue-style-box.js, there is a switch statement based on the value of cue.positionAlign (https://github.com/videojs/vtt.js/blob/master/lib/process/cue-style-box.js#L54). However, the values in the switch appear to be possible values for the VttCue lineAlign property, not the positionAlign property.

See https://www.w3.org/TR/webvtt1/#the-vttcue-interface:

enum LineAlignSetting { "start", "center", "end" }; enum PositionAlignSetting { "line-left", "center", "line-right", "auto" };

I'm new to this, so I wonder if these values have perhaps changed over time. If there's another explanation, I'd be interested to hear it, since I'm trying to debug some positioning issues I'm seeing.

gkatsev commented 4 years ago

This is likely because the values in the spec changed but vtt.js hasn't been updated with for that. Hadn't noticed that positionAlign here was still using the old values.

gkatsev commented 4 years ago

We definitely should fix this but my focus is currently on things like #50 because the webvtt spec currently has position and line alignment options as at-risk https://htmlpreview.github.io/?https://github.com/gkatsev/webvtt/blob/at-risk-june/archives/2019-06-19/Overview.html#status.

jonknowles commented 4 years ago

Thanks for the explanation, that's what I had suspected. I hadn't seen that about these being at-risk in the spec, so that is good to know. I got confusing positioning results when I tried an initial refactor, so I probably won't be able to try to fix it at the moment, but if I ever figure it out, I'll try to do a PR.

gkatsev commented 4 years ago

Actually, looking at the implementation report of webvtt again, it looks like Firefox may have an implementation and if we fix it here, we can claim it as another implementation and then remove it from being at risk. That actually sounds likely might be more important than region support. @jonknowles what types of things are you seeing and what have you tried to do?

jonknowles commented 4 years ago

I think the odd behavior was due to most cues not having a positionAlign property at all, so I added logic to default it according to the algorithm laid out here: https://www.w3.org/TR/webvtt1/#webvtt-cue-position-alignment

This was in Chome, using the vtt.js parser, not the native one. I'm kinda struggling to comprehend the interaction of the positionAlign and align values in the WebVTT spec right now, so I might not be understanding what the proper behavior actually is.