Bug 74995

Summary: [EFL][WK2] Implement context menu for EFL port
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: WebKit EFLAssignee: Gyuyoung Kim <gyuyoung.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, mpakulavelrutka, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

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.