[Webkit-unassigned] [Bug 91832] [EFL][WK2] Implemented color chooser proxy and input picker interface

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91832





--- Comment #49 from KwangYong Choi <ky0.choi at samsung.com>  2012-08-19 19:09:19 PST ---
(From update of attachment 159079)
View in context: https://bugs.webkit.org/attachment.cgi?id=159079&action=review

Well, I will apply new patch with a test. I think, it's better to submit a feature with a test at the same time.

>> Source/WebKit2/UIProcess/efl/WebColorChooserProxyEfl.h:35
>> +
> 
> Don't you need to declare Color namespace as Qt port ?
> 
> http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/qt/WebColorChooserProxyQt.h#L36

#include "Color.h" is enough to use WebCore::Color.

>>> Source/WebKit2/UIProcess/efl/WebColorChooserProxyEfl.h:46
>>> +    virtual ~WebColorChooserProxyEfl();
>> 
>> I think we don't need to use virtual keyword in derived class. Look at Qt port. 
>> http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/qt/WebColorChooserProxyQt.h#L50
> 
> Yes, it is unlikely that we subclass WebColorChooserProxyEfl so I tend to agree with Gyugyoung (although not really an issue).

OK. I will remove virtual keyword.

>> Source/WebKit2/UIProcess/efl/WebColorChooserProxyEfl.h:48
>> +    virtual void endChooser();
> 
> If you follow Gyuyoung's advice and remove virtual destructor from the destructor then please do the same for this method (or it would be confusing).

OK

>> Source/WebKit2/UIProcess/efl/WebColorChooserProxyEfl.h:49
>> +    virtual void setSelectedColor(const WebCore::Color&);
> 
> Ditto.

OK

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.