WebVTT: Update cue alignment to a later spec. version

WebVTT: Update cue alignment to a later spec. version

This implements the updated spec. behavior by removing the dependency
on the (resolved) direction when determining the alignment.

BUG=449521
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188817

[fs, 2015-01-21 17:44:50]

fs@opera.com changed reviewers:
+ philipj@opera.com

[philipj_slow, 2015-01-22 10:06:51]

Code LGTM, but it's a change in behavior so a test that would fail before this
change is needed, i.e. a test where the text directionality would previously
make a difference.

[fs, 2015-01-22 12:49:15]

On 2015/01/22 10:06:51, philipj wrote:
> Code LGTM, but it's a change in behavior so a test that would fail before this
> change is needed, i.e. a test where the text directionality would previously
> make a difference.

TBH I was a bit surprised that no test regressed with this (but less and less so
now when it comes to rendering tests... =/)

[philipj_slow, 2015-01-22 12:57:47]

Yeah, are our old LayoutTests/media/track/opera/track/webvtt/rendering/reftest/
tests aren't run at all, maybe getting those up and running would provide some
more coverage, although some of them are bound to be stale by now.

[fs, 2015-01-22 13:09:02]

On 2015/01/22 12:57:47, philipj wrote:
> Yeah, are our old
LayoutTests/media/track/opera/track/webvtt/rendering/reftest/
> tests aren't run at all, maybe getting those up and running would provide some
> more coverage, although some of them are bound to be stale by now.

Yes, I've considered that a few times by now, but I think they'd need to be
updated wrt. "position of controls" as well as spec.

[fs, 2015-01-22 14:32:59]

On 2015/01/22 13:09:02, fs wrote:
> On 2015/01/22 12:57:47, philipj wrote:
> > Yeah, are our old
> LayoutTests/media/track/opera/track/webvtt/rendering/reftest/
> > tests aren't run at all, maybe getting those up and running would provide
some
> > more coverage, although some of them are bound to be stale by now.
> 
> Yes, I've considered that a few times by now, but I think they'd need to be
> updated wrt. "position of controls" as well as spec.

Test added, please apply your best scrutiny.

[philipj_slow, 2015-01-22 15:27:59]

https://codereview.chromium.org/851333003/diff/20001/LayoutTests/media/track/...
File LayoutTests/media/track/track-cue-rendering-position-auto-rtl-expected.html
(right):

https://codereview.chromium.org/851333003/diff/20001/LayoutTests/media/track/...
LayoutTests/media/track/track-cue-rendering-position-auto-rtl-expected.html:21:
right: 50%;
The input here was { align: 'start', size: 50, line: 0 } but I'm unable to
convince myself that this is correct. position is auto and align is start, and
so the computed text position should be 0, but this and the expectations seem to
say 50, in line with the old way before position auto. I'm probably just
confused about where we are in this patch series, why does this make sense?

[philipj_slow, 2015-01-22 15:28:21]

I've verified that the test fails without your changes, FWIW.

[philipj_slow, 2015-01-22 15:44:25]

LGTM!

https://codereview.chromium.org/851333003/diff/20001/LayoutTests/media/track/...
File LayoutTests/media/track/track-cue-rendering-position-auto-rtl-expected.html
(right):

https://codereview.chromium.org/851333003/diff/20001/LayoutTests/media/track/...
LayoutTests/media/track/track-cue-rendering-position-auto-rtl-expected.html:21:
right: 50%;
On 2015/01/22 15:27:59, philipj wrote:
> The input here was { align: 'start', size: 50, line: 0 } but I'm unable to
> convince myself that this is correct. position is auto and align is start, and
> so the computed text position should be 0, but this and the expectations seem
to
> say 50, in line with the old way before position auto. I'm probably just
> confused about where we are in this patch series, why does this make sense?

I'm just confused. The resulting box in which to position the text has left:0
and width:50% so it's expected that RTL text will end up aligned against the
middle. This is exactly what we wanted when we changed the spec, so that text
directionality doesn't change the part of the screen which may be occupied.

[fs, 2015-01-22 15:54:04]

On 2015/01/22 15:44:25, philipj wrote:
> LGTM!
> 
>
https://codereview.chromium.org/851333003/diff/20001/LayoutTests/media/track/...
> File
LayoutTests/media/track/track-cue-rendering-position-auto-rtl-expected.html
> (right):
> 
>
https://codereview.chromium.org/851333003/diff/20001/LayoutTests/media/track/...
>
LayoutTests/media/track/track-cue-rendering-position-auto-rtl-expected.html:21:
> right: 50%;
> On 2015/01/22 15:27:59, philipj wrote:
> > The input here was { align: 'start', size: 50, line: 0 } but I'm unable to
> > convince myself that this is correct. position is auto and align is start,
and
> > so the computed text position should be 0, but this and the expectations
seem
> to
> > say 50, in line with the old way before position auto. I'm probably just
> > confused about where we are in this patch series, why does this make sense?
> 
> I'm just confused. The resulting box in which to position the text has left:0
> and width:50% so it's expected that RTL text will end up aligned against the
> middle. This is exactly what we wanted when we changed the spec, so that text
> directionality doesn't change the part of the screen which may be occupied.

Yes, this matches my understanding to.

[fs, 2015-01-22 15:54:07]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2015-01-22 15:54:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/851333003/20001

[commit-bot: I haz the power, 2015-01-22 15:58:15]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188817