Bug 132811 – [EFL][WK2] Add ewk_view_fixed_layout_size_set|get()

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Gyuyoung Kim]

Some mobile devices need to set fixed layout size by own decision. This patch supports to set|get fixed layout size by using WKPageSetFixedLayoutSize(), WKPageGetFixedLayoutSize().

[Gyuyoung Kim]

Created attachment 231274 [details]
WIP

[Gyuyoung Kim]

Created attachment 231277 [details]
Patch

[Ryuan Choi]

Comment on attachment 231277 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:725
> +    EWK_VIEW_IMPL_GET_OR_RETURN(ewkView, impl);

How about clearing width, height like ewk_view_contents_size_get ?

> 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);

[Gyuyoung Kim]

Comment on attachment 231277 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:725
>> +    EWK_VIEW_IMPL_GET_OR_RETURN(ewkView, impl);
> 
> How about clearing width, height like ewk_view_contents_size_get ?

I don't know what is benefit when we clear width, height. Any benefit ?

>> 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.

[Ryuan Choi]

(In reply to comment #4)
> (From update of attachment 231277 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231277&action=review
> 
> >> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:725
> >> +    EWK_VIEW_IMPL_GET_OR_RETURN(ewkView, impl);
> > 
> > How about clearing width, height like ewk_view_contents_size_get ?
> 
> I don't know what is benefit when we clear width, height. Any benefit ?
> 
Although I don't like it too, it's common in EFL world.

> >> 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.

[Gyuyoung Kim]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 231277 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=231277&action=review
> > 
> > >> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:725
> > >> +    EWK_VIEW_IMPL_GET_OR_RETURN(ewkView, impl);
> > > 
> > > How about clearing width, height like ewk_view_contents_size_get ?
> > 
> > I don't know what is benefit when we clear width, height. Any benefit ?
> > 
> Although I don't like it too, it's common in EFL world.

Hmm, if there is no benefit or real yoda style, I would like to avoid to follow EFL style. When we review upcoming similar patch, I don't know how to guide this rule. I just can say "EFL is yoda style, so, you should follow it."


> > >> 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.

[Ryuan Choi]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (From update of attachment 231277 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=231277&action=review
> > > 
> > > >> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:725
> > > >> +    EWK_VIEW_IMPL_GET_OR_RETURN(ewkView, impl);
> > > > 
> > > > How about clearing width, height like ewk_view_contents_size_get ?
> > > 
> > > I don't know what is benefit when we clear width, height. Any benefit ?
> > > 
> > Although I don't like it too, it's common in EFL world.
> 
> Hmm, if there is no benefit or real yoda style, I would like to avoid to follow EFL style. When we review upcoming similar patch, I don't know how to guide this rule. I just can say "EFL is yoda style, so, you should follow it."
> 
> 

+1

[Gyuyoung Kim]

(In reply to comment #3)
> (From update of attachment 231277 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231277&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:725
> > +    EWK_VIEW_IMPL_GET_OR_RETURN(ewkView, impl);
> 
> How about clearing width, height like ewk_view_contents_size_get ?
> 
> > 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);

CC'ing juyoung. Juyoung, as mentioned in comment #3, has EFL coding rule or implicit coding style that ryuan pointed out ?

[Chris Dumez]

Comment on attachment 231277 [details]
Patch

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.

[Gyuyoung Kim]

Created attachment 231436 [details]
Patch for landing

[Gyuyoung Kim]

Comment on attachment 231277 [details]
Patch

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

>>>>>> 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

I don't see we should use Evas_Coord for positive number though, I follow to use it according to you guys votes. :)

[WebKit Commit Bot]

Comment on attachment 231436 [details]
Patch for landing

Clearing flags on attachment: 231436

Committed r168787: <http://trac.webkit.org/changeset/168787>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.