[webkit-reviews] review denied: [Bug 113284] Add method in ewk_settings for setting the CSS media type : [Attachment 198505] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 17 18:40:01 PDT 2013
Gyuyoung Kim <gyuyoung.kim at samsung.com> has denied Jose Lejin PJ
<jose.lejin at gmail.com>'s request for review:
Bug 113284: Add method in ewk_settings for setting the CSS media type
https://bugs.webkit.org/show_bug.cgi?id=113284
Attachment 198505: Patch
https://bugs.webkit.org/attachment.cgi?id=198505&action=review
------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at samsung.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=198505&action=review
Please read WebKit EFL coding style :
http://trac.webkit.org/wiki/EFLWebKitCodingStyle
> Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:429
> + if (ewk_settings_css_media_type_get())
Why should we call this function twice ? Look at the implementation on mac
port.
http://trac.webkit.org/browser/trunk/Source/WebKit/mac/WebCoreSupport/WebFrameL
oaderClient.mm#L1948
> Source/WebKit/efl/ewk/ewk_settings.cpp:393
> +void ewk_settings_css_media_type_set(const char *type)
Wrong * place.
> Source/WebKit/efl/ewk/ewk_settings.cpp:398
> +const char* ewk_settings_css_media_type_get(void)
Do not use *void* in implementation file. We use WebKit coding style except for
public header.
> Source/WebKit/efl/ewk/ewk_settings.h:396
> + * @param type css media type to be set, must be write-able.
We have not used . at @param, @return.
> Source/WebKit/efl/ewk/ewk_settings.h:407
> + * @return css media type set by user.
ditto.
More information about the webkit-reviews
mailing list