Bug 39821 - [EFL] Add Context Menu support
Summary: [EFL] Add Context Menu support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 39629
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-27 02:02 PDT by Lucas De Marchi
Modified: 2010-05-31 15:59 PDT (History)
3 users (show)

See Also:


Attachments
implement context menu (17.59 KB, patch)
2010-05-27 02:08 PDT, Lucas De Marchi
no flags Details | Formatted Diff | Diff
Patch (33.96 KB, patch)
2010-05-28 03:53 PDT, Lucas De Marchi
no flags Details | Formatted Diff | Diff
Patch (35.68 KB, patch)
2010-05-28 18:42 PDT, Lucas De Marchi
no flags Details | Formatted Diff | Diff
Patch (35.61 KB, patch)
2010-05-31 14:53 PDT, Lucas De Marchi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas De Marchi 2010-05-27 02:02:17 PDT
Add context menu support in EFL port.
Comment 1 Lucas De Marchi 2010-05-27 02:08:37 PDT
Created attachment 57211 [details]
implement context menu

This is the patch that must be applied after its dependency lands. Please, review #39629.
Comment 2 Lucas De Marchi 2010-05-28 03:53:27 PDT
Created attachment 57314 [details]
Patch
Comment 3 Lucas De Marchi 2010-05-28 03:55:16 PDT
Updated to latest changes and include files in build system.
Comment 4 Lucas De Marchi 2010-05-28 10:29:52 PDT
Comment on attachment 57314 [details]
Patch

Clearing flags to update new version and force style check
Comment 5 Lucas De Marchi 2010-05-28 18:42:23 PDT
Created attachment 57401 [details]
Patch
Comment 6 Kenneth Rohde Christiansen 2010-05-31 08:39:38 PDT
Comment on attachment 57401 [details]
Patch

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
Comment 7 Lucas De Marchi 2010-05-31 14:53:35 PDT
Created attachment 57496 [details]
Patch
Comment 8 WebKit Commit Bot 2010-05-31 15:58:56 PDT
Comment on attachment 57496 [details]
Patch

Clearing flags on attachment: 57496

Committed r60454: <http://trac.webkit.org/changeset/60454>
Comment 9 WebKit Commit Bot 2010-05-31 15:59:02 PDT
All reviewed patches have been landed.  Closing bug.