Bug 109083 - [EFL][WK2] Add C API for popup menu and popup item
Summary: [EFL][WK2] Add C API for popup menu and popup item
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 108798
  Show dependency treegraph
 
Reported: 2013-02-06 12:05 PST by Chris Dumez
Modified: 2013-02-18 16:42 PST (History)
13 users (show)

See Also:


Attachments
WIP Patch (26.74 KB, patch)
2013-02-06 12:10 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (57.15 KB, patch)
2013-02-07 07:43 PST, Chris Dumez
andersca: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (57.10 KB, patch)
2013-02-18 13:44 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2013-02-06 12:05:33 PST
EFL WK2 delegates the display of the popup menu to the browser. As a consequence, we currently have a strong dependency between our Ewk API implementation (EwkPopupMenu, EwkPopupMenuItem, EwkView) and internal WK2 C++ classes (WebPopupMenuProxyEfl, WebPopupMenu, WebPopupItem). To break this dependency, we should add a C API for the popup menu and the popup item.
Comment 1 Chris Dumez 2013-02-06 12:10:01 PST
Created attachment 186890 [details]
WIP Patch
Comment 2 WebKit Review Bot 2013-02-06 12:14:10 PST
Attachment 186890 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/PlatformEfl.cmake', u'Source/WebKit2/Shared/API/c/efl/WKBaseEfl.h', u'Source/WebKit2/Shared/APIObject.h', u'Source/WebKit2/UIProcess/API/C/efl/WKAPICastEfl.h', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.cpp', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupMenu.cpp', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupMenu.h', u'Source/WebKit2/UIProcess/efl/PageClientBase.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupItemEfl.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupItemEfl.h', u'Source/WebKit2/UIProcess/efl/WebPopupMenuClient.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupMenuClient.h', u'Source/WebKit2/UIProcess/efl/WebPopupMenuProxyEfl.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupMenuProxyEfl.h']" exit_code: 1
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:42:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupMenu.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 5 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Kenneth Rohde Christiansen 2013-02-06 13:46:24 PST
Comment on attachment 186890 [details]
WIP Patch

Should probably have an API test. Would be nice to see how this would work for multi selects
Comment 4 Chris Dumez 2013-02-07 07:43:55 PST
Created attachment 187110 [details]
Patch
Comment 5 WebKit Review Bot 2013-02-07 07:47:58 PST
Attachment 187110 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/PlatformEfl.cmake', u'Source/WebKit2/Shared/API/c/efl/WKBaseEfl.h', u'Source/WebKit2/Shared/APIObject.h', u'Source/WebKit2/UIProcess/API/C/efl/WKAPICastEfl.h', u'Source/WebKit2/UIProcess/API/C/efl/WKPageEfl.cpp', u'Source/WebKit2/UIProcess/API/C/efl/WKPageEfl.h', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.cpp', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupMenuListener.cpp', u'Source/WebKit2/UIProcess/API/C/efl/WKPopupMenuListener.h', u'Source/WebKit2/UIProcess/API/efl/EwkView.cpp', u'Source/WebKit2/UIProcess/API/efl/EwkView.h', u'Source/WebKit2/UIProcess/API/efl/ewk_popup_menu.cpp', u'Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_private.h', u'Source/WebKit2/UIProcess/WebPageProxy.cpp', u'Source/WebKit2/UIProcess/WebPageProxy.h', u'Source/WebKit2/UIProcess/WebPopupMenuProxy.h', u'Source/WebKit2/UIProcess/efl/PageClientBase.cpp', u'Source/WebKit2/UIProcess/efl/PageUIClientEfl.cpp', u'Source/WebKit2/UIProcess/efl/PageUIClientEfl.h', u'Source/WebKit2/UIProcess/efl/WebPageProxyEfl.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupItemEfl.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupItemEfl.h', u'Source/WebKit2/UIProcess/efl/WebPopupMenuListenerEfl.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupMenuListenerEfl.h', u'Source/WebKit2/UIProcess/efl/WebPopupMenuProxyEfl.cpp', u'Source/WebKit2/UIProcess/efl/WebPopupMenuProxyEfl.h', u'Source/WebKit2/UIProcess/efl/WebUIPopupMenuClient.cpp', u'Source/WebKit2/UIProcess/efl/WebUIPopupMenuClient.h']" exit_code: 1
Source/WebKit2/UIProcess/API/C/efl/WKPageEfl.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:42:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebKit2/UIProcess/API/C/efl/WKPopupItem.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 5 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 WebKit Review Bot 2013-02-18 12:13:18 PST
Comment on attachment 187110 [details]
Patch

Rejecting attachment 187110 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 187110, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
pMenuProxyEfl.cpp
rm 'Source/WebKit2/UIProcess/efl/WebPopupMenuProxyEfl.cpp'
patching file Source/WebKit2/UIProcess/efl/WebPopupMenuProxyEfl.h
rm 'Source/WebKit2/UIProcess/efl/WebPopupMenuProxyEfl.h'
patching file Source/WebKit2/UIProcess/efl/WebUIPopupMenuClient.cpp
patching file Source/WebKit2/UIProcess/efl/WebUIPopupMenuClient.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Anders Carlsson']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16611352
Comment 7 Simon Hausmann 2013-02-18 12:20:47 PST
It would've been nice if this wasn't all EFL'ed - naturally the other ports need the same functionality
Comment 8 Chris Dumez 2013-02-18 12:45:22 PST
(In reply to comment #7)
> It would've been nice if this wasn't all EFL'ed - naturally the other ports need the same functionality

I see. I wasn't sure other ports needed this. It would be nice to share code with other ports as much as possible. Do you think it would be OK to land this and then later move / rename files as needed when Qt port needs it?

The thing is that some things are port specific. For example, the WKPopupMenuListener API is likely going to be different on Qt port. However, things like WKPopupItem, WebPopupItemEfl and likely WebUIPopupMenuClient can certainly be shared.
Comment 9 Chris Dumez 2013-02-18 13:44:28 PST
Created attachment 188939 [details]
Patch for landing
Comment 10 WebKit Review Bot 2013-02-18 16:42:26 PST
Comment on attachment 188939 [details]
Patch for landing

Clearing flags on attachment: 188939

Committed r143275: <http://trac.webkit.org/changeset/143275>
Comment 11 WebKit Review Bot 2013-02-18 16:42:31 PST
All reviewed patches have been landed.  Closing bug.