VTTCue: Simplify computation of direction-aware text position

VTTCue: Simplify computation of direction-aware text position

Add a helper resolveCueAlignment to resolve 'start' and 'end' to the
corresponding "visual" value, and also compute the visual text position
once.

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

[fs, 2015-01-15 18:11:04]

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

[philipj_slow, 2015-01-16 13:37:42]

Would this not be made obsolete by implementing
http://dev.w3.org/html5/webvtt/#dfn-text-track-cue-computed-text-position per
spec?

The spec was changed to make the position and size of the cue box not depend on
text directionality, now it's only the alignment of the text within the box that
is affected.

[fs, 2015-01-16 13:57:15]

On 2015/01/16 13:37:42, philipj wrote:
> Would this not be made obsolete by implementing
> http://dev.w3.org/html5/webvtt/#dfn-text-track-cue-computed-text-position per
> spec?

That alone doesn't help, since
http://dev.w3.org/html5/webvtt/#dfn-text-track-cue-computed-text-position-ali...
will be needed to actually get there. This change is fairly decent step on the
way though IMO (compare step 7 of
http://dev.w3.org/html5/webvtt/#dfn-apply-webvtt-cue-settings to the resulting
code.)

[philipj_slow, 2015-01-16 14:08:34]

Hmm, the "Determine the value of maximum size for cue as per the appropriate
rules from the following list" steps use the "text track cue computed text
position" does it not?

[fs, 2015-01-16 14:19:53]

On 2015/01/16 14:08:34, philipj wrote:
> Hmm, the "Determine the value of maximum size for cue as per the appropriate
> rules from the following list" steps use the "text track cue computed text
> position" does it not?

Yes, but it still switches on 'text track cue computed text position alignment'
- which is also "close" to what it's doing right now. I have patch locally
(somewhere...) that - as a next step - unifies these two. The next step would
then be to switch to use the 'computed text position alignment' and drop the
directionality check.

[philipj_slow, 2015-01-16 14:43:23]

Right, it's positionAlign that gives rise to "text track cue computed text
position alignment". I'm hoping to not implement positionAlign/lineAlign, and
http://foolip.github.io/webvtt/#dfn-apply-webvtt-cue-settings is the shape of
things that I am hoping for.

There eventually won't be a function which takes VTTCue.align and the computed
text directionality, as ignoring positionAlign "text track cue computed text
position" and the CSS mapping table to CSS text-align values ought to be the
only bits of code using VTTCue.align...

LGTM since it actually improves on the current state of the code, and I guess it
doesn't matter that it now looks less like the spec revision it corresponds to.

[fs, 2015-01-16 15:03:55]

On 2015/01/16 14:43:23, philipj wrote:
> Right, it's positionAlign that gives rise to "text track cue computed text
> position alignment". I'm hoping to not implement positionAlign/lineAlign, and
> http://foolip.github.io/webvtt/#dfn-apply-webvtt-cue-settings is the shape of
> things that I am hoping for.
> 
> There eventually won't be a function which takes VTTCue.align and the computed
> text directionality, as ignoring positionAlign "text track cue computed text
> position" and the CSS mapping table to CSS text-align values ought to be the
> only bits of code using VTTCue.align...

I filed crbug.com/449521 for this.

[fs, 2015-01-16 15:04:00]

The CQ bit was checked by fs@opera.com

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

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

[philipj_slow, 2015-01-16 15:07:38]

On 2015/01/16 15:03:55, fs wrote:
> On 2015/01/16 14:43:23, philipj wrote:
> > Right, it's positionAlign that gives rise to "text track cue computed text
> > position alignment". I'm hoping to not implement positionAlign/lineAlign,
and
> > http://foolip.github.io/webvtt/#dfn-apply-webvtt-cue-settings is the shape
of
> > things that I am hoping for.
> > 
> > There eventually won't be a function which takes VTTCue.align and the
computed
> > text directionality, as ignoring positionAlign "text track cue computed text
> > position" and the CSS mapping table to CSS text-align values ought to be the
> > only bits of code using VTTCue.align...
> 
> I filed crbug.com/449521 for this.

Starred, thanks!

[commit-bot: I haz the power, 2015-01-16 21:51:53]

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