Summary: | [EFL][WK2] Add ewk_view_fixed_layout_size_set|get() | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||
Component: | WebKit EFL | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bunhere, cdumez, commit-queue, gyuyoung.kim, hyuki.kim, lucas.de.marchi, ryuan.choi, seojuyung2, sergio | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2014-05-11 22:40:50 PDT
Created attachment 231274 [details]
WIP
Created attachment 231277 [details]
Patch
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); 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. (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. (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. (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 (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 ? 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. Created attachment 231436 [details]
Patch for landing
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. :) Comment on attachment 231436 [details] Patch for landing Clearing flags on attachment: 231436 Committed r168787: <http://trac.webkit.org/changeset/168787> All reviewed patches have been landed. Closing bug. |