[webkit-reviews] review denied: [Bug 44759] [EFL] Add custom network resource handler : [Attachment 105627] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 6 09:37:02 PDT 2011
Martin Robinson <mrobinson at webkit.org> has denied Flavio Ceolin
<flavio.ceolin at profusion.mobi>'s request for review:
Bug 44759: [EFL] Add custom network resource handler
https://bugs.webkit.org/show_bug.cgi?id=44759
Attachment 105627: Patch
https://bugs.webkit.org/attachment.cgi?id=105627&action=review
------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105627&action=review
Looks good, but I have a few style nits. With another iteration and the
unoffocial r+, I think this patch is fine.
> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:42
> +static unsigned custom_protocol_added_count = 0;
Please either use camelCase everywhere or this_thing. WebKit style is to use
camelCase.
> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:57
> +
EwkProtocolHandlerPrivate);
The indent looks off here.
> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:71
> + void* buf;
I'd prefer buffer to buf.
> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:145
> + guint protocols_size;
> + SoupSession* session = WebCore::ResourceHandle::defaultSession();
It's odd that you use camelCase above bug something_else here.
> Source/WebKit/efl/ewk/ewk_view.cpp:94
> + struct {
Extra space after struct.
> Source/WebKit/efl/ewk/ewk_view.cpp:96
> + void* ctxt;
> + Ewk_View_Resource_Handler_Cb func;
"context" and "function" In Webkit we do not typically abbreviate variable
names.
> Source/WebKit/efl/ewk/ewk_view.cpp:3618
> +Eina_Bool ewk_view_protocol_handler_set(Evas_Object* o, const char**
protocols, Ewk_View_Resource_Handler_Cb handler, void* ctxt)
Ditto.
> Source/WebKit/efl/ewk/ewk_view.h:2227
> + void* ctxt);
ditto.
More information about the webkit-reviews
mailing list