-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible incorrect mappings for positionAlign values #51
Comments
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. |
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. |
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. |
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. |
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. |
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.
The text was updated successfully, but these errors were encountered: