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?

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?

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

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

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

Hi all, Just to let you know I've updated my email address, so I kindly ...

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.