Sync snap-to-lines VTT cue layout steps with spec

Sync snap-to-lines VTT cue layout steps with spec

 * Sync step documentation ('line position' -> 'line' and similar.)
 * Remove remnants of 'best position' and re-number steps.
 * Implement new cue dodging logic.

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

[fs, 2015-08-21 10:45:57]

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

[fs, 2015-08-21 10:45:58]

Found in an old pile of TODO...

[philipj_slow, 2015-08-21 10:50:39]

We just can't stop changing the wording, see
https://github.com/w3c/webvtt/pull/211 for more churn.

[fs, 2015-08-21 10:54:57]

On 2015/08/21 10:50:39, philipj wrote:
> We just can't stop changing the wording, see
> https://github.com/w3c/webvtt/pull/211 for more churn.

(╯°□°）╯︵ ┻━┻)

[philipj_slow, 2015-08-21 10:55:04]

https://codereview.chromium.org/1306833003/diff/1/LayoutTests/media/track/tra...
File LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html (right):

https://codereview.chromium.org/1306833003/diff/1/LayoutTests/media/track/tra...
LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html:24:
addCue(track, { line: 0, align: 'start' });
Perhaps comment here which of these is expected to be visible and which will be
pushed out, and even give them different texts so that one could tell just by
looking at the result?

https://codereview.chromium.org/1306833003/diff/1/Source/core/layout/LayoutVT...
File Source/core/layout/LayoutVTTCue.cpp (right):

https://codereview.chromium.org/1306833003/diff/1/Source/core/layout/LayoutVT...
Source/core/layout/LayoutVTTCue.cpp:214:
m_cueBox.setLogicalTop(m_cueBox.containingBlock()->logicalHeight());
Is there any chance in some zoom level that this perfect alignment and
anti-aliasing could lead to some visible artifact?

[fs, 2015-08-21 12:02:31]

On 2015/08/21 10:54:57, fs wrote:
> On 2015/08/21 10:50:39, philipj wrote:
> > We just can't stop changing the wording, see
> > https://github.com/w3c/webvtt/pull/211 for more churn.
> 
> (╯°□°）╯︵ ┻━┻)

A tried to approach that a bit (and did some other random syncing). Syncing
diff->code is even more tedious than spec->code though, so it's highly likely I
missed something (a lot)...

https://codereview.chromium.org/1306833003/diff/1/LayoutTests/media/track/tra...
File LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html (right):

https://codereview.chromium.org/1306833003/diff/1/LayoutTests/media/track/tra...
LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html:24:
addCue(track, { line: 0, align: 'start' });
On 2015/08/21 10:55:03, philipj wrote:
> Perhaps comment here which of these is expected to be visible and which will
be
> pushed out, and even give them different texts so that one could tell just by
> looking at the result?

Sure. Hope you don't mind that the textContent will be 'PAS' and 'FAI' =) (the
font-size is pretty "well" tweaked for the purpose, and since 'PASS' and 'FAIL'
run a risk of being different width, etc...)

https://codereview.chromium.org/1306833003/diff/1/Source/core/layout/LayoutVT...
File Source/core/layout/LayoutVTTCue.cpp (right):

https://codereview.chromium.org/1306833003/diff/1/Source/core/layout/LayoutVT...
Source/core/layout/LayoutVTTCue.cpp:214:
m_cueBox.setLogicalTop(m_cueBox.containingBlock()->logicalHeight());
On 2015/08/21 10:55:03, philipj wrote:
> Is there any chance in some zoom level that this perfect alignment and
> anti-aliasing could lead to some visible artifact?

I can nudge it one more (CSS) pixel away, then it should definitely be safe I
think.

[philipj_slow, 2015-08-21 12:17:53]

LGTM, I haven't actually checked the comment updates against the spec, but they
look very similar to the diffs of the spec itself, so I'm sure this is now
*more* like the spec, although as you say there's probably lots more not in
sync. Some day? Probably not :)

https://codereview.chromium.org/1306833003/diff/40001/Source/core/html/track/...
File Source/core/html/track/vtt/VTTCue.cpp (right):

https://codereview.chromium.org/1306833003/diff/40001/Source/core/html/track/...
Source/core/html/track/vtt/VTTCue.cpp:960: // When the user agent is to parse
the WebVTT settings given by a string input for a text tracke cue,
s/tracke/track/

[philipj_slow, 2015-08-21 12:19:00]

https://codereview.chromium.org/1306833003/diff/40001/LayoutTests/media/track...
File LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html (right):

https://codereview.chromium.org/1306833003/diff/40001/LayoutTests/media/track...
LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html:24:
addCue(track, { line: 0, align: 'start', text: 'PAS' });
If just XXX and YYY would work, that's also different enough to tell from the
output which it is :)

[fs, 2015-08-21 12:25:17]

On 2015/08/21 12:17:53, philipj wrote:
> LGTM, I haven't actually checked the comment updates against the spec, but
they
> look very similar to the diffs of the spec itself, so I'm sure this is now
> *more* like the spec, although as you say there's probably lots more not in
> sync. Some day? Probably not :)

The double-edged sword of documentation...
 
>
https://codereview.chromium.org/1306833003/diff/40001/Source/core/html/track/...
> File Source/core/html/track/vtt/VTTCue.cpp (right):
> 
>
https://codereview.chromium.org/1306833003/diff/40001/Source/core/html/track/...
> Source/core/html/track/vtt/VTTCue.cpp:960: // When the user agent is to parse
> the WebVTT settings given by a string input for a text tracke cue,
> s/tracke/track/

Undo fail. Fixed.

On 2015/08/21 12:19:00, philipj wrote:
>
https://codereview.chromium.org/1306833003/diff/40001/LayoutTests/media/track...
> File LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html (right):
> 
>
https://codereview.chromium.org/1306833003/diff/40001/LayoutTests/media/track...
> LayoutTests/media/track/track-cue-rendering-line-doesnt-fit.html:24:
> addCue(track, { line: 0, align: 'start', text: 'PAS' });
> If just XXX and YYY would work, that's also different enough to tell from the
> output which it is :)

Anything short enough... I think I prefer PAS and FAI before XXX and YYY though.

[philipj_slow, 2015-08-21 12:54:15]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-08-21 12:54:16]

lgtm

PAS/FAI WFM.

[commit-bot: I haz the power, 2015-08-21 12:54:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306833003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306833003/60001

[commit-bot: I haz the power, 2015-08-21 14:20:24]

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