Bug 169352 - Implement stroke-color CSS property.
Summary: Implement stroke-color CSS property.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-03-08 05:18 PST by Per Arne Vollan
Modified: 2017-04-12 02:59 PDT (History)
8 users (show)

See Also:


Attachments
Patch (7.37 KB, patch)
2017-03-08 05:28 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (14.23 KB, patch)
2017-03-09 12:40 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (20.01 KB, patch)
2017-03-31 05:58 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (21.61 KB, patch)
2017-04-11 08:04 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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!