Add support for CSS3 "text-decoration" shorthand

Add support for CSS3 "text-decoration" shorthand

This patch implements the shorthand version of "text-decoration"
property, as specified by the CSS3 Text Decoration specification:

http://dev.w3.org/csswg/css-text-decor-3/#text-decoration-property

Backported from WebKit:

https://bugs.webkit.org/show_bug.cgi?id=92000

BUG=165462

[brunoabinader, 2013-06-19 14:12:39]

Hi all,
This is a preliminary patch that implements the shorthand version of the
"text-decoration" property. There is a bunch of layout tests that needs to be
updated if this patch is accepted, so this version is for review only.

Shall we proceed with this approach and update all affected layout tests?

[Julien - ping for review, 2013-06-19 17:38:36]

It looks like the test went missing in the transition.

[darktears, 2013-06-19 18:12:01]

https://codereview.chromium.org/17101024/diff/1/Source/core/css/StyleProperty...
File Source/core/css/StylePropertyShorthand.cpp (right):

https://codereview.chromium.org/17101024/diff/1/Source/core/css/StyleProperty...
Source/core/css/StylePropertyShorthand.cpp:476: return textDecorationLonghands;
You also need to patch the bottom part of the file.

[brunoabinader, 2013-06-20 15:34:58]

On 2013/06/19 17:38:36, Julien Chaffraix wrote:
> It looks like the test went missing in the transition.

That was intentional :-) In WebKit it is easier because the shorthand property
is prefixed, and thus, does not affect current "text-decoration" implementation.
In Blink, as we're not using prefixed versions any longer, it directly affects
"text-decoration" computed style, and consequently, a bunch of layout tests that
depends on current behavior of "text-decoration" implementation. That
getComputedStyle layout test from the WebKit patch would become an updated
version of the layout test below:

LayoutTests/fast/css/getComputedStyle/getComputedStyle-text-decoration.html

Apart from this layout test, there are at least 20+ layout tests that have their
results affected by this change. This is why I am interested in your feedback on
the best approach I believe we should follow: Accept this patch and rebaseline
all affected layout tests. What do you think?

[brunoabinader, 2013-06-20 15:35:23]

On 2013/06/19 18:12:01, darktears wrote:
>
https://codereview.chromium.org/17101024/diff/1/Source/core/css/StyleProperty...
> File Source/core/css/StylePropertyShorthand.cpp (right):
> 
>
https://codereview.chromium.org/17101024/diff/1/Source/core/css/StyleProperty...
> Source/core/css/StylePropertyShorthand.cpp:476: return
textDecorationLonghands;
> You also need to patch the bottom part of the file.

Thanks Alexis ! I've updated the patch with the missing code.

[brunoabinader, 2013-06-20 15:36:23]

On 2013/06/20 15:34:58, abinader wrote:
> On 2013/06/19 17:38:36, Julien Chaffraix wrote:
> > It looks like the test went missing in the transition.
> 
> That was intentional :-) In WebKit it is easier because the shorthand property
> is prefixed, and thus, does not affect current "text-decoration"
implementation.
> In Blink, as we're not using prefixed versions any longer, it directly affects
> "text-decoration" computed style, and consequently, a bunch of layout tests
that
> depends on current behavior of "text-decoration" implementation. That
> getComputedStyle layout test from the WebKit patch would become an updated
> version of the layout test below:
> 
> LayoutTests/fast/css/getComputedStyle/getComputedStyle-text-decoration.html
> 
> Apart from this layout test, there are at least 20+ layout tests that have
their
> results affected by this change. This is why I am interested in your feedback
on
> the best approach I believe we should follow: Accept this patch and rebaseline
> all affected layout tests. What do you think?

*Not just rebaseline, but also update layout test itself when necessary.

[Julien - ping for review, 2013-06-21 14:49:43]

Landing a change without the proper testing is a no-go from my perspective.

This change suffers from the lack of virtual test suite to test different
runtime flag combinations but that doesn't mean we can't add the required amount
of testing for the change: the shorthand can easily be tested and probably some
tests will need to be updated but that's fine. Ideally we would keep the old
test running as-is as they shouldn't be impacted if the runtime flag is
disabled.

[abinader, 2013-07-08 23:19:26]

Hi all,

Just to let you know I've updated my email address, so I kindly request for
someone to close this issue as I'm going to create a new one soon with the new
login.

[tfarina, 2013-07-08 23:28:57]

On 2013/07/08 23:19:26, abinader wrote:
> Hi all,
> 
> Just to let you know I've updated my email address, so I kindly request for
> someone to close this issue as I'm going to create a new one soon with the new
> login.

OK. I closed it.