[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