CSS systemcolors should work in svg properties using <color>.

CSS systemcolors should work in svg properties using <color>.

This affects the 'stop-color', 'flood-color' and 'lighting-color'
properties.

R=
BUG=234936
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=149844

[Erik Dahlström (inactive), 2013-04-24 07:59:02]

Fixes parsing of 'stop-color', 'flood-color' and 'lighting-color' properties,
they should all allow css systemcolors. See
http://www.w3.org/TR/SVG11/types.html#DataTypeColor.

Not sure if 'grey' even worked before, if it did it certainly took a different
codepath than the other named colors.

[eseidel, 2013-04-24 08:07:52]

can we test this?

[Erik Dahlström (inactive), 2013-04-24 08:23:47]

On 2013/04/24 08:07:52, Eric Seidel (Google) wrote:
> can we test this?

Probably easiest by reftest (comparing to the fill property, or perhaps with css
color + fill=currentColor), how are other cssprops tested?

[Erik Dahlström (inactive), 2013-04-25 14:57:17]

On 2013/04/24 08:23:47, Erik Dahlström wrote:
> On 2013/04/24 08:07:52, Eric Seidel (Google) wrote:
> > can we test this?
> 
> Probably easiest by reftest (comparing to the fill property, or perhaps with
css
> color + fill=currentColor), how are other cssprops tested?

Put some drafts for reftests here:
http://code.google.com/p/chromium/issues/detail?id=234936#c3

[pdr., 2013-05-02 17:41:08]

Oh no, this has gone unreviewed for quite some time! Please be a squeaky wheel
and make noise if you aren't getting reviews :)

The codechange here looks good but we need to add the tests into this patch as
well. The tests you added on the bug look reasonable, can you add them to this
patch?

https://codereview.chromium.org/14108013/diff/1/Source/core/css/SVGCSSParser.cpp
File Source/core/css/SVGCSSParser.cpp (right):

https://codereview.chromium.org/14108013/diff/1/Source/core/css/SVGCSSParser....
Source/core/css/SVGCSSParser.cpp:211: if ((id >= CSSValueActiveborder && id <=
CSSValueWindowtext) || id == CSSValueMenu)
These statements are very similar to the codepaths above for fill and stroke. Is
there an easy way to extract this logic out into a function? This is just a
suggestion.

[Erik Dahlström (inactive), 2013-05-03 08:28:26]

I'll push a test to here soon, just needed to figure out the LayoutTest system.

I'll separate out the systemcolor check too.

[eseidel, 2013-05-06 07:57:35]

lgtm

Seems reasonable.

[commit-bot: I haz the power, 2013-05-06 08:00:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ed@opera.com/14108013/9001

[commit-bot: I haz the power, 2013-05-06 08:32:50]

Retried try job too often on linux_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[Erik Dahlström (inactive), 2013-05-06 14:55:50]

On 2013/05/06 08:32:50, I haz the power (commit-bot) wrote:
> Retried try job too often on linux_layout_rel for step(s) webkit_tests
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

The failing and flaky test(s) are totally unrelated to the change here. What are
my next steps here?

[pdr., 2013-05-07 03:06:46]

On 2013/05/06 14:55:50, Erik Dahlström wrote:
> On 2013/05/06 08:32:50, I haz the power (commit-bot) wrote:
> > Retried try job too often on linux_layout_rel for step(s) webkit_tests
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...
> 
> The failing and flaky test(s) are totally unrelated to the change here. What
are
> my next steps here?

So frustrating :( Let me try the CQ again. If it fails, I'll commit this
manually for you in the morning.

As a non-committer, you don't have many options. I think our best option is to
fix that non-committer thing ASAP :)

[commit-bot: I haz the power, 2013-05-07 03:07:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ed@opera.com/14108013/9001

[commit-bot: I haz the power, 2013-05-07 03:33:05]

Change committed as 149844