Bug 111877 - [EFL] Use CROSS_PLATFORM_CONTEXT_MENU
Summary: [EFL] Use CROSS_PLATFORM_CONTEXT_MENU
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jesus Sanchez-Palencia
URL:
Keywords:
Depends on: 111876
Blocks: 111874
  Show dependency treegraph
 
Reported: 2013-03-08 11:20 PST by Jesus Sanchez-Palencia
Modified: 2013-03-14 13:11 PDT (History)
6 users (show)

See Also:


Attachments
Patch (13.06 KB, patch)
2013-03-08 12:04 PST, Jesus Sanchez-Palencia
no flags Details | Formatted Diff | Diff
Patch (13.22 KB, patch)
2013-03-13 12:54 PDT, Jesus Sanchez-Palencia
no flags Details | Formatted Diff | Diff
Patch (13.12 KB, patch)
2013-03-14 10:50 PDT, Jesus Sanchez-Palencia
no flags Details | Formatted Diff | Diff
Patch for landing (13.17 KB, patch)
2013-03-14 12:24 PDT, Jesus Sanchez-Palencia
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesus Sanchez-Palencia 2013-03-08 11:20:18 PST
EFL could easily switch to Cross platform context menu implementation and reduce the related code to this significantly.
Comment 1 Jesus Sanchez-Palencia 2013-03-08 12:04:26 PST
Created attachment 192263 [details]
Patch
Comment 2 EFL EWS Bot 2013-03-08 14:18:22 PST
Comment on attachment 192263 [details]
Patch

Attachment 192263 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17110229
Comment 3 Jesus Sanchez-Palencia 2013-03-11 05:15:23 PDT
(In reply to comment #2)
> (From update of attachment 192263 [details])
> Attachment 192263 [details] did not pass efl-ews (efl):
> Output: http://webkit-commit-queue.appspot.com/results/17110229

This won't build until https://bugs.webkit.org/show_bug.cgi?id=111876 is landed.
The reason why split this into a separate patch was to avoid reviewers asking for that. :)
Comment 4 Caio Marcelo de Oliveira Filho 2013-03-12 14:57:11 PDT
Comment on attachment 192263 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192263&action=review

> Source/WebCore/ChangeLog:9
> +        Make EFL work with CROSS_PLATFORM_CONTEXT_MENU
> +        USE flag. This flag provides a full cross-platform

"Make EFL work with ..." kind of suggests it will still work without. I think it'll be more clear to state that now EFL uses CROSS_PLATFORM_CONTEXT_MENU.

> Source/WebCore/ChangeLog:15
> +        No new tests needed, already covered.

Which tests cover this change?

> Source/WebKit/efl/ewk/ewk_contextmenu.cpp:245
> +    const Vector<WebCore::ContextMenuItem> itemsList = coreMenu->items();

Missing "&" here. You want const Vector<...>&.
Comment 5 Caio Marcelo de Oliveira Filho 2013-03-12 15:00:38 PDT
This change looks good to me after those comments are considered, but let's make EFL bot green before landing. (So probably landing your other patch first)
Comment 6 Jesus Sanchez-Palencia 2013-03-13 12:54:27 PDT
Created attachment 192970 [details]
Patch
Comment 7 Jesus Sanchez-Palencia 2013-03-14 10:50:11 PDT
Created attachment 193149 [details]
Patch
Comment 8 WebKit Review Bot 2013-03-14 12:08:51 PDT
Comment on attachment 193149 [details]
Patch

Rejecting attachment 193149 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 193149, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
rce/WebKit/chromium/webkit/media/crypto/ppapi/cdm --revision 181763 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
58>At revision 181763.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://webkit-commit-queue.appspot.com/results/17126437
Comment 9 Jesus Sanchez-Palencia 2013-03-14 12:24:01 PDT
Created attachment 193171 [details]
Patch for landing
Comment 10 WebKit Review Bot 2013-03-14 13:11:07 PDT
Comment on attachment 193171 [details]
Patch for landing

Clearing flags on attachment: 193171

Committed r145841: <http://trac.webkit.org/changeset/145841>
Comment 11 WebKit Review Bot 2013-03-14 13:11:12 PDT
All reviewed patches have been landed.  Closing bug.