[webkit-reviews] review granted: [Bug 193171] Implement the css-color-4 behavior for inheritance of currentColor : [Attachment 395441] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Apr 4 08:44:14 PDT 2020
Simon Fraser (smfr) <simon.fraser at apple.com> has granted Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 193171: Implement the css-color-4 behavior for inheritance of currentColor
https://bugs.webkit.org/show_bug.cgi?id=193171
Attachment 395441: patch
https://bugs.webkit.org/attachment.cgi?id=395441&action=review
--- Comment #14 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 395441
--> https://bugs.webkit.org/attachment.cgi?id=395441
patch
View in context: https://bugs.webkit.org/attachment.cgi?id=395441&action=review
> LayoutTests/fast/borders/border-color-inherit-expected.html:1
> +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN">
Maybe just <!DOCTYPE html> (and in the test too).
> Source/WebCore/rendering/style/RenderStyle.h:1735
> + static Color currentColor() { return { }; }
Maybe change initialTextEmphasisColor() to return curentColor().
> Source/WebCore/style/StyleBuilderState.cpp:324
> + // FIXME: 'currentcolor' should be resolved at use time to make it
inherit correctly.
You should file a bug on the FIXME and reference it here. That seems fairly
important to fix.
> Source/WebCore/svg/SVGStopElement.cpp:97
> + if (!stopColor.isValid())
> + stopColor = style.color();
A little helper function like RenderStyle::resolveCurrentColor or something
would make this much more understandable.
More information about the webkit-reviews
mailing list