[WebVTT] Support text-decoration-{text,color,style} on CSS ::cue pseudo-element

[WebVTT] Support text-decoration-{text,color,style} on CSS ::cue pseudo-element

The CSS ::cue pseudo-element is defined on WebVTT specification as
part of the HTML5 <track> element specification. This issue intends to
incrementally add support for the following properties:

- text-decoration-line
- text-decoration-style
- text-decoration-color

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

[Julien - ping for review, 2013-07-18 01:06:02]

lgtm

https://codereview.chromium.org/19516002/diff/1/LayoutTests/media/track/track...
File LayoutTests/media/track/track-css-all-cues.html (right):

https://codereview.chromium.org/19516002/diff/1/LayoutTests/media/track/track...
LayoutTests/media/track/track-css-all-cues.html:17: text-decoration-color: cyan;
I would love to see the shorthand 'text-decoration' also tested instead of only
the longhands. Maybe we can strike some balance and test both:

text-decoration: overline dashed orange;
text-decoration-line: underline;
text-decoration-color: cyan;

That would still test 2 out of 3 longhands and the shorthand.

https://codereview.chromium.org/19516002/diff/1/Source/core/css/resolver/Styl...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/19516002/diff/1/Source/core/css/resolver/Styl...
Source/core/css/resolver/StyleResolver.cpp:1154: case
CSSPropertyTextDecorationLine:
I would include the 'text-decoration' shorthand here too and move them to an
ASSERT_NOT_REACHED case (with the explanation that we expand shorthands at parse
time [except 'font']). This would make it obvious that we allow them but don't
expect any here. Your code is totally right but would make people wonder if
omitting 'text-decoration' is a coding mistake.

[abinader, 2013-07-18 02:51:42]

Thank you for the review Julien :) The shorthand for text-decoration has not
landed on Blink yet. Actually, I found out this issue with WebVTT as a spin-off
from my investigations on the affected layout tests caused by the changes from
the shorthand patch. I'll keep your suggestions in mind when deploying the
updated shorthand patch. Shall I have a go for commit?

[Julien - ping for review, 2013-07-18 15:26:17]

On 2013/07/18 02:51:42, abinader wrote:
> Thank you for the review Julien :) The shorthand for text-decoration has not
> landed on Blink yet. Actually, I found out this issue with WebVTT as a
spin-off
> from my investigations on the affected layout tests caused by the changes from
> the shorthand patch. I'll keep your suggestions in mind when deploying the
> updated shorthand patch. Shall I have a go for commit?

Sure, how about a FIXME about the shorthand in the code so that we know this is
tracked (with a reference this review)?

[abinader, 2013-07-18 15:44:58]

On 2013/07/18 15:26:17, Julien Chaffraix wrote:
> On 2013/07/18 02:51:42, abinader wrote:
> > Thank you for the review Julien :) The shorthand for text-decoration has not
> > landed on Blink yet. Actually, I found out this issue with WebVTT as a
> spin-off
> > from my investigations on the affected layout tests caused by the changes
from
> > the shorthand patch. I'll keep your suggestions in mind when deploying the
> > updated shorthand patch. Shall I have a go for commit?
> 
> Sure, how about a FIXME about the shorthand in the code so that we know this
is
> tracked (with a reference this review)?

Awesome idea :) I'll update the patch with the corresponding FIXME message in a
bit!

[commit-bot: I haz the power, 2013-07-18 16:20:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bruno.d@partner.samsung.com/19516002/9001

[commit-bot: I haz the power, 2013-07-18 18:17:03]

Change committed as 154500