[webkit-reviews] review granted: [Bug 132811] [EFL][WK2] Add ewk_view_fixed_layout_size_set|get() : [Attachment 231277] Patch

[bugzilla-daemon at webkit.org]

Christophe Dumez <dchris at gmail.com> has granted Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 132811: [EFL][WK2] Add ewk_view_fixed_layout_size_set|get()
https://bugs.webkit.org/show_bug.cgi?id=132811

Attachment 231277: Patch
https://bugs.webkit.org/attachment.cgi?id=231277&action=review

------- Additional Comments from Christophe Dumez <dchris at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231277&action=review


r=me % nits

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:903
> + * The webview size will be set with given size. The size value should be
set to "positive number".

Nit: I don't think "The size value should be set to "positive number"" is very
useful here.

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:909
> +EAPI void ewk_view_layout_fixed_size_set(const Evas_Object *o, unsigned
width, unsigned height);

nit: Evas_Coord

>>>>> Source/WebKit2/UIProcess/API/efl/ewk_view.h:919
>>>>> +EAPI void ewk_view_layout_fixed_size_get(const Evas_Object *o, unsigned
*width, unsigned *height);
>>>> 
>>>> How about Evas_Coord?
>>>> 
>>>> Below is example.
>>>> EAPI void evas_object_resize(Eo *obj, Evas_Coord w, Evas_Coord h);
>>> 
>>> As far as I know, Evas_Coord is integer type, isn't it ? In this API, I
hope to use *unsigned* because this API needs to handle positive number.
>> 
>> Similarly, EFL didn't use unsigned for the size values before.
> 
> If so, I want to change it as well.

nit: +1 for Evas_Coord

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:1093
> +    unsigned width;

nit: please initialize to 0

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:1094
> +    unsigned height;

ditto.