Bug 95708 - [EFL][WK2] Implement missing feature to support <select> tag
Summary: [EFL][WK2] Implement missing feature to support <select> tag
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: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-03 22:39 PDT by Ryuan Choi
Modified: 2012-09-06 02:09 PDT (History)
3 users (show)

See Also:


Attachments
Patch (13.74 KB, patch)
2012-09-03 23:20 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (13.42 KB, patch)
2012-09-04 05:12 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2012-09-03 22:39:42 PDT
In bug 88616, basic functionality of popup menu is implemented (type and title)

This bug implements other members of WebPopupMenu.
Comment 1 Ryuan Choi 2012-09-03 23:20:56 PDT
Created attachment 161970 [details]
Patch
Comment 2 Chris Dumez 2012-09-03 23:29:56 PDT
Comment on attachment 161970 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:53
> +    _Ewk_Popup_Menu_Item(Ewk_Popup_Menu_Item_Type _type, const char* _text, Ewk_Text_Direction _textDirection, bool _hasTextDirectionOverride, const char* _toolTip, const char* _accessibilityText, bool _isEnabled, bool _isLabel, bool _isSelected)

Why not simply pass a WebKit::WebPopupItem to the constructor? It would be much simpler.

> Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:119
> +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);

Should be EINA_SAFETY_ON_NULL_RETURN_VAL(item, false);

> Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:126
> +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);

Ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:133
> +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);

Ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:140
> +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);

Ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:147
> +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);

Ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:154
> +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);

Ditto.
Comment 3 Ryuan Choi 2012-09-04 05:12:19 PDT
Created attachment 162017 [details]
Patch
Comment 4 Ryuan Choi 2012-09-04 05:13:32 PDT
(In reply to comment #2)
> (From update of attachment 161970 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161970&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:53
> > +    _Ewk_Popup_Menu_Item(Ewk_Popup_Menu_Item_Type _type, const char* _text, Ewk_Text_Direction _textDirection, bool _hasTextDirectionOverride, const char* _toolTip, const char* _accessibilityText, bool _isEnabled, bool _isLabel, bool _isSelected)
> 
> Why not simply pass a WebKit::WebPopupItem to the constructor? It would be much simpler.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:119
> > +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);
> 
> Should be EINA_SAFETY_ON_NULL_RETURN_VAL(item, false);
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:126
> > +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);
> 
> Ditto.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:133
> > +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);
> 
> Ditto.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:140
> > +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);
> 
> Ditto.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:147
> > +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);
> 
> Ditto.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_popup_menu_item.cpp:154
> > +    EINA_SAFETY_ON_NULL_RETURN_VAL(item, 0);
> 
> Ditto.

Fixed. thank you.
Comment 5 Chris Dumez 2012-09-04 05:15:49 PDT
Comment on attachment 162017 [details]
Patch

LGTM.
Comment 6 Gyuyoung Kim 2012-09-05 19:14:09 PDT
Comment on attachment 162017 [details]
Patch

Looks good to me.
Comment 7 WebKit Review Bot 2012-09-05 20:58:03 PDT
Comment on attachment 162017 [details]
Patch

Clearing flags on attachment: 162017

Committed r127686: <http://trac.webkit.org/changeset/127686>