Bug 74995 - [EFL][WK2] Implement context menu for EFL port
Summary: [EFL][WK2] Implement context menu for EFL port
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: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-20 22:50 PST by Gyuyoung Kim
Modified: 2011-12-26 23:30 PST (History)
7 users (show)

See Also:


Attachments
Patch (9.09 KB, patch)
2011-12-20 22:52 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (7.64 KB, patch)
2011-12-20 23:15 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (7.93 KB, patch)
2011-12-21 23:18 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (7.93 KB, patch)
2011-12-21 23:21 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (8.18 KB, patch)
2011-12-26 02:27 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2011-12-20 22:50:19 PST
Implement missing ContextMenuEfl class in order to support WK2's context menu. Because WK2's context menu still needs WebCore's context menu implementation.
And of course, this patch also can be used for WK1 without CROSS_PLATFORM_CONTEXT_MENU option.

Now, Bug 74179 supports context menu as well. But, the patch is to support context menu using CROSS_PLATFORM_CONTEXT_MENUS option. I think we also need to support context menu when CROSS_PLATFORM_CONTEXT_MENUS is disabled.
Comment 1 Gyuyoung Kim 2011-12-20 22:52:04 PST
Created attachment 120146 [details]
Patch
Comment 2 Gyuyoung Kim 2011-12-20 22:53:28 PST
In addition, I use Vector class for PlatformMenuDescription. I didn't feel like using efl library for this.
Comment 3 Gyuyoung Kim 2011-12-20 23:15:52 PST
Created attachment 120149 [details]
Patch
Comment 4 Martin Robinson 2011-12-21 21:59:01 PST
Comment on attachment 120149 [details]
Patch

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

> Source/WebCore/platform/efl/ContextMenuItemEfl.cpp:103
>  void ContextMenuItem::setChecked(bool)
>  {
> -    notImplemented();
> +    m_platformDescription.checked = checked;

Shouldn't this use the argument?

> Source/WebCore/platform/efl/ContextMenuItemEfl.cpp:114
>  void ContextMenuItem::setEnabled(bool)
>  {
> -    notImplemented();
> +    m_platformDescription.enabled = enabled;
>  }

Ditto.
Comment 5 Gyuyoung Kim 2011-12-21 23:18:06 PST
Created attachment 120278 [details]
Patch
Comment 6 Gyuyoung Kim 2011-12-21 23:18:40 PST
(In reply to comment #4)
> (From update of attachment 120149 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120149&action=review
> 
> > Source/WebCore/platform/efl/ContextMenuItemEfl.cpp:103
> >  void ContextMenuItem::setChecked(bool)
> >  {
> > -    notImplemented();
> > +    m_platformDescription.checked = checked;
> 
> Shouldn't this use the argument?
> 
> > Source/WebCore/platform/efl/ContextMenuItemEfl.cpp:114
> >  void ContextMenuItem::setEnabled(bool)
> >  {
> > -    notImplemented();
> > +    m_platformDescription.enabled = enabled;
> >  }
> 
> Ditto.

Oops, my mistake. Fix them.
Comment 7 Gyuyoung Kim 2011-12-21 23:21:22 PST
Created attachment 120279 [details]
Patch
Comment 8 Gyuyoung Kim 2011-12-26 02:27:52 PST
Created attachment 120540 [details]
Patch
Comment 9 WebKit Review Bot 2011-12-26 23:30:36 PST
Comment on attachment 120540 [details]
Patch

Clearing flags on attachment: 120540

Committed r103702: <http://trac.webkit.org/changeset/103702>
Comment 10 WebKit Review Bot 2011-12-26 23:30:45 PST
All reviewed patches have been landed.  Closing bug.