Support for VTTCue.positionAlign and lineAlign regressed cue positioning involving align:start |
||
Issue descriptionhttps://www.youtube.com/watch?v=0seWLzMHY7g has (auto-generated) subtitles that can be enabled, at least on Android. A minified sample of this is attach and pasted here: WEBVTT 00:00:00.460 --> 00:00:13.559 align:start position:19% cinema The positioning of this has changed, bisected on Mac down to: https://chromium.googlesource.com/chromium/src/+log/b8c94b96e65efba286cf232b7610369ae3818baa..f6310ca186ac7c5ff7737412a4e1649b3698f2d8 A regression range from Android was wider but also included the same commit: https://chromium.googlesource.com/chromium/src/+/2c1d466d78db6df8b4c9fbe37ac5af1cb135bf34 This cue, before the changes, would have some margin on the left, and now it doesn't. Attaching screenshots from the test clip I used.
,
Feb 22 2018
I've done some testing in other browsers. In Firefox I can't see any cues at all with this input, maybe the end up off screen or something. Safari has the same behavior as we did. Edge not tested.
,
Feb 22 2018
I'd imagine https://chromium-review.googlesource.com/c/chromium/src/+/845462 would make a difference in this case. (I suspect right now we'll just fall through a lot and not actually compute positions correctly... =/)
,
Feb 22 2018
I've prepared a revert in https://chromium-review.googlesource.com/c/chromium/src/+/931481 in case we need it, running it through a dry run. I haven't looked very closely at the changes yet, but given position:19% this is a non-snap-to-lines cue, so do you think the brokenness of that was exposed? Would it make sense to revert and try to reland together with https://chromium-review.googlesource.com/c/chromium/src/+/845462, or is there a smaller piece that could be reverted?
,
Feb 22 2018
It's position:...% - not line:...%, so it'll be a snap-to-lines cue. I think the problem is that the 'start' value of 'align' doesn't end up mapping to 'line-left' (which the CL in c#3 changes/fixes), so we end up with some form of initial values (0) for x-position. I don't think it would make sense to revert anything smaller, but if reverting, then relanding with said CL folded in would be the reasonable (only?) thing to do.
,
Feb 22 2018
+1 for landing it together and i assume that we will do it only after concluding https://github.com/w3c/webvtt/issues/425 which should make things even more clear.
,
Feb 22 2018
Thanks both, I'm submitting the revert CL now.
,
Feb 22 2018
https://chromium-review.googlesource.com/c/chromium/src/+/931481 has landed, will reopen the original bug.
,
Feb 23 2018
verified the issue by applying the https://chromium-review.googlesource.com/c/chromium/src/+/845462 patch, it is working properly. Once https://github.com/w3c/webvtt/issues/425 is resolved from webvtt spec, i will submit one patch by combining the two patchs.
,
Feb 23 2018
Excellent, thank you! |
||
►
Sign in to add a comment |
||
Comment 1 by foolip@chromium.org
, Feb 22 2018902 KB
902 KB View Download
903 KB
903 KB View Download
70 bytes
70 bytes Download