[webkit-reviews] review denied: [Bug 87495] [WK2] Color chooser API missing : [Attachment 147284] Patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has denied Thiago Marcos P. Santos
<tmpsantos at gmail.com>'s request for review:
Bug 87495: [WK2] Color chooser API missing
https://bugs.webkit.org/show_bug.cgi?id=87495

Attachment 147284: Patch
https://bugs.webkit.org/attachment.cgi?id=147284&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=147284&action=review


According to http://www.webkit.org/coding/contributing.html, new files should
have the WebKit license and not LGPL. Patch looks good otherwise.

> Source/WebKit2/UIProcess/PageClient.h:176
> +    virtual PassRefPtr<WebColorChooserProxy>
createColorChooserProxy(WebPageProxy*, const WebCore::Color&) = 0;

I think it's good to name this parameter initialColor.

> Source/WebKit2/UIProcess/WebPageProxy.h:794
> +    void showColorChooser(const WebCore::Color&);

I think it's good to name this parameter initialColor.

> Source/WebKit2/WebKit2.xcodeproj/project.pbxproj:891
> +		E131CFA8157D1C9A002A66B3 /* WebColorChooser.cpp in CopyFiles */
= {isa = PBXBuildFile; fileRef = E131CFA6157D1C9A002A66B3 /*
WebColorChooser.cpp */; };
> +		E131CFA9157D1C9A002A66B3 /* WebColorChooser.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = E131CFA6157D1C9A002A66B3 /* WebColorChooser.cpp
*/; };
> +		E131CFAA157D1C9A002A66B3 /* WebColorChooser.h in CopyFiles */ =
{isa = PBXBuildFile; fileRef = E131CFA7157D1C9A002A66B3 /* WebColorChooser.h
*/; };
> +		E131CFAD157D1CD7002A66B3 /* WebColorChooserProxy.h in CopyFiles
*/ = {isa = PBXBuildFile; fileRef = E131CFAC157D1CD7002A66B3 /*
WebColorChooserProxy.h */; };

This part is incorrect. You have to add the files using Xcode and not edit the
project file manually.