[webkit-reviews] review granted: [Bug 39821] [EFL] Add Context Menu support : [Attachment 57401] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 31 08:39:37 PDT 2010


Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Lucas De Marchi
<lucas.demarchi at profusion.mobi>'s request for review:
Bug 39821: [EFL] Add Context Menu support
https://bugs.webkit.org/show_bug.cgi?id=39821

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
Please take these comments into account

WebCore/platform/efl/ContextMenuEfl.cpp:36
 +	    static_cast<ContextMenuClientEfl*>(controller()->client());
Put this on one line please

WebCore/platform/efl/ContextMenuEfl.cpp:45
 +	    static_cast<ContextMenuClientEfl*>(controller()->client());
here as well

WebCore/platform/efl/ContextMenuEfl.cpp:75
 +	// Ref counting remains the same, just pass it and remove our ref, so
it
ref count remainds the same, that is what you want to say right?

WebCore/platform/efl/ContextMenuItemEfl.cpp:40
 +	// it's inside WebKit that this initialization is done, as WebCore
doesn't
Comments start with capital letter and ends with a dot/punctuation mark.

WebKit/efl/WebCoreSupport/ContextMenuClientEfl.h:53
 +	PlatformMenuDescription newPlatformDescription(ContextMenu*);
what about createPlatformDescription ?

WebKit/efl/ewk/ewk_contextmenu.cpp:114
 +	// don't care about title and submenu as they're not used after this
point
Capital + dot please

WebKit/efl/ewk/ewk_contextmenu.h:68
 +	// these are new tags! not a part of api!!!!
How so? You should probably make a better comment


More information about the webkit-reviews mailing list