[webkit-reviews] review denied: [Bug 38845] Canvas color property getters should serialize them according to spec : [Attachment 56334] Proposed patch v8

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 18 14:59:50 PDT 2010


Darin Adler <darin at apple.com> has denied Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 38845: Canvas color property getters should serialize them according to
spec
https://bugs.webkit.org/show_bug.cgi?id=38845

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

------- Additional Comments from Darin Adler <darin at apple.com>
>  CanvasStyle::CanvasStyle(PassRefPtr<CanvasGradient> gradient)
> -    : m_type(gradient ? Gradient : ColorString)
> +    : m_type(gradient ? Gradient : RGBA)
>      , m_gradient(gradient)
>  {
>  }

This will create a style with an uninitialized m_rgba with type RGBA. That's
not good. The old code would have a null color string, not an uninitialized
color string.

Instead, I suggest we put the null check in the CanvasStyle::create function
and return 0 in that case.

Also, I'd like to see test cases for this.

>  CanvasStyle::CanvasStyle(PassRefPtr<CanvasPattern> pattern)
> -    : m_type(pattern ? ImagePattern : ColorString)
> +    : m_type(pattern ? ImagePattern : RGBA)
>      , m_pattern(pattern)
>  {
>  }

Same thing.

Everything else looks fine to me.


More information about the webkit-reviews mailing list