Bug 95708

Summary:

[EFL][WK2] Implement missing feature to support <select> tag

Product:

WebKit

Reporter:

Ryuan Choi <ryuan.choi>

Component:

WebKit EFL

Assignee:

Ryuan Choi <ryuan.choi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, gyuyoung.kim, lucas.de.marchi

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

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>