[webkit-reviews] review denied: [Bug 40273] Canvas: "currentColor" should inherit the canvas element's color : [Attachment 59849] Proposed patch v2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 40273: Canvas: "currentColor" should inherit the canvas element's color
https://bugs.webkit.org/show_bug.cgi?id=40273

Attachment 59849: Proposed patch v2
https://bugs.webkit.org/attachment.cgi?id=59849&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
This looks really good.

> -	   static PassRefPtr<CanvasGradient> create(const FloatPoint& p0, const
FloatPoint& p1)
> +	   static PassRefPtr<CanvasGradient> create(HTMLCanvasElement* canvas,
const FloatPoint& p0, const FloatPoint& p1)

I am pretty sure this is wrong. There is no guarantee that a gradient will be
used only with the canvas it was created from. Instead, the canvas or its
current color should be passed to functions in CanvasStyle, and in turn passed
in to the functions on the CanvasGradient.

We should create at least one test case that creates a gradient on one canvas
and then uses it with another, which has a different current color, to test
that the behavior is correct.

> +    RGBA32 currentColor(HTMLCanvasElement* canvas);
> +    bool parseColorOrCurrentColor(RGBA32& parsedColor, const String&
colorString, HTMLCanvasElement* canvas);

The argument name "canvas" should be omitted from both of these function
declarations.