Bug 169352

Summary: Implement stroke-color CSS property.
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebCore Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, hyatt, jonlee, koivisto, simon.fraser, thorton, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Per Arne Vollan 2017-03-08 05:18:30 PST
Implement stroke-color CSS property, see https://drafts.fxtf.org/paint/.
Comment 1 Per Arne Vollan 2017-03-08 05:28:42 PST
Created attachment 303809 [details]
Patch
Comment 2 Per Arne Vollan 2017-03-09 12:40:33 PST
Created attachment 303953 [details]
Patch
Comment 3 Jon Lee 2017-03-17 09:40:43 PDT
Per, is this ready to be reviewed?
I think we need the same kind of tests for checking the fallback behavior as you did for stroke-width.
Comment 4 Per Arne Vollan 2017-03-17 10:23:07 PDT
(In reply to comment #3)
> Per, is this ready to be reviewed?
> I think we need the same kind of tests for checking the fallback behavior as
> you did for stroke-width.

Thanks! I will add a fallback test. I think the patch needs a little more work before it is ready to be reviewed :)
Comment 5 Per Arne Vollan 2017-03-30 09:27:01 PDT
I will make the patch ready for review soon :)
Comment 6 Per Arne Vollan 2017-03-31 05:58:23 PDT
Created attachment 305966 [details]
Patch
Comment 7 Per Arne Vollan 2017-04-07 03:48:39 PDT
Perhaps it would be better to just define stroke-color as an alias of -webkit-text-stroke-color (or vice versa)?
Comment 8 Jon Lee 2017-04-10 08:26:19 PDT
(In reply to Per Arne Vollan from comment #7)
> Perhaps it would be better to just define stroke-color as an alias of
> -webkit-text-stroke-color (or vice versa)?

Both color and width should be treated the same. I'm ok with this approach for now.
Comment 9 Jon Lee 2017-04-10 09:42:49 PDT
Comment on attachment 305966 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305966&action=review

> Source/WebCore/css/CSSProperties.json:3056
> +        },

I think this needs to be adjusted per 170643.

> Source/WebCore/rendering/style/StyleRareInheritedData.cpp:337
> +        && visitedLinkStrokeColor == o.visitedLinkStrokeColor

We need tests that include :visited links.
Comment 10 Per Arne Vollan 2017-04-11 08:04:17 PDT
Created attachment 306817 [details]
Patch
Comment 11 Per Arne Vollan 2017-04-11 08:06:21 PDT
(In reply to Jon Lee from comment #9)
> Comment on attachment 305966 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305966&action=review
> 
> > Source/WebCore/css/CSSProperties.json:3056
> > +        },
> 
> I think this needs to be adjusted per 170643.
> 
> > Source/WebCore/rendering/style/StyleRareInheritedData.cpp:337
> > +        && visitedLinkStrokeColor == o.visitedLinkStrokeColor
> 
> We need tests that include :visited links.

Thanks for reviewing! I have added a new test.
Comment 12 Jon Lee 2017-04-11 10:23:24 PDT
Comment on attachment 306817 [details]
Patch

Per, could we add tests or extend existing tests to check getComputedStyle results for all of the CSS properties you've added, particularly wrt visited links?
Comment 13 WebKit Commit Bot 2017-04-12 01:01:55 PDT
Comment on attachment 306817 [details]
Patch

Clearing flags on attachment: 306817

Committed r215261: <http://trac.webkit.org/changeset/215261>
Comment 14 WebKit Commit Bot 2017-04-12 01:01:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Per Arne Vollan 2017-04-12 02:52:23 PDT
rdar://problem/30754817
Comment 16 Per Arne Vollan 2017-04-12 02:59:08 PDT
(In reply to Jon Lee from comment #12)
> Comment on attachment 306817 [details]
> Patch
> 
> Per, could we add tests or extend existing tests to check getComputedStyle
> results for all of the CSS properties you've added, particularly wrt visited
> links?

Yes, I will look into that in a separate patch. Thanks for reviewing!