Add ewk_pagination_mode_set/ewk_view_pagination_mode_get APIs. The WK APIs for pagination mode set/get was existed, and we need ewk APIs for those.
Created attachment 170340 [details] Patch
Comment on attachment 170340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170340&action=review > Source/WebKit2/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=100213 ewk_view_pagination_mode_set/get() APIs? > Source/WebKit2/ChangeLog:5 > + remove redundant space after 'Add'. > Source/WebKit2/ChangeLog:8 > + Add ewk_pagination_mode_set/ewk_view_pagination_mode_get APIs for paginated display. ditto. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:812 > + * Remove redundant space after 'a'. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:814 > + * @param mode The Ewk_Pagination_Mode to set Remove redundant space in the line. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:821 > + * Get a pagenation mode of the current web page. ditto. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:825 > + * @return The pagination mode of the current web page ditto.
Comment on attachment 170340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170340&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:2207 > + paginationMode =WebCore::Pagination::LeftToRightPaginated; add space after '='
Comment on attachment 170340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170340&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:2200 > + The code in these functions should be moved inside methods in EwkViewImpl and the C function should merely call the new C++ method. > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:2203 > + case EWK_PAGINATION_MODE_UNPAGINATED: You should probably use COMPILE_ASSERT_MATCHING_ENUM macro from ewk_private.h to avoid this switch(). A simple cast would be more efficient. > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:2228 > + Ewk_Pagination_Mode mode; Unused variable? > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:2230 > + EWK_VIEW_PRIV_GET_OR_RETURN(smartData, priv, EWK_PAGINATION_MODE_END); Why return EWK_PAGINATION_MODE_END in case of error? Shouldn't you use a mode such as EWK_PAGINATION_MODE_INVALID with value of -1? > Source/WebKit2/UIProcess/API/efl/ewk_view.h:275 > + * @info Keep this in sync with WebCore::Pagination::Mode in Pagination.h COMPILE_ASSERT_MATCHING_ENUM() will make sure of that. I don't think we should have such comment in the public header (this is implementation specific). > Source/WebKit2/UIProcess/API/efl/ewk_view.h:278 > + EWK_PAGINATION_MODE_UNPAGINATED, Lack of documentation for each enum value. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:283 > + EWK_PAGINATION_MODE_END What is this mode? > Source/WebKit2/UIProcess/API/efl/ewk_view.h:811 > + * Set a pagenation mode to the current web page. "pagenation" > "pagination" remove the "a" in front of pagination (bad english) > Source/WebKit2/UIProcess/API/efl/ewk_view.h:818 > +EAPI Eina_Bool ewk_view_pagination_mode_set(Evas_Object* o, Ewk_Pagination_Mode mode); Star on wrong side >> Source/WebKit2/UIProcess/API/efl/ewk_view.h:821 >> + * Get a pagenation mode of the current web page. > > ditto. "pagenation" > "pagination" remove the "a" in front of pagination (bad english) > Source/WebKit2/UIProcess/API/efl/ewk_view.h:822 > + * Please document what the default pagination mode is. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:823 > + * @param o view object toget the pagenation mode "toget" -> "to get" "pagenation" > "pagination" > Source/WebKit2/UIProcess/API/efl/ewk_view.h:827 > +EAPI Ewk_Pagination_Mode ewk_view_pagination_mode_get(const Evas_Object* o); Star on wrong side > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:972 > + // Reload page to check the pagination modeue. "modeue" -> "mode"
Created attachment 170572 [details] Patch
Comment on attachment 170572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170572&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:1510 > + return static_cast<Ewk_Pagination_Mode>(impl->pageProxy->paginationMode()); Blank line before return statement would be nice. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:283 > + EWK_PAGINATION_MODE_INVALID /**< invalid pagination mode that will be returned when error occured. */ We usually put this one first with "= -1" value I believe. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:821 > + * Get pagination mode of the current web page. The default value is EWK_PAGINATION_MODE_UNPAGINATED. "The default value is EWK_PAGINATION_MODE_UNPAGINATED." should probably not be in the title. Instead, I would put it under, after a blank line.
Created attachment 170582 [details] Patch
Comment on attachment 170582 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170582&action=review LGTM but: > Source/WebKit2/UIProcess/API/efl/ewk_view.h:283 > + EWK_PAGINATION_MODE_END /**< number of pagination mode */ Why do you need this EWK_PAGINATION_MODE_END ? I think we should remove it.
Comment on attachment 170582 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170582&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_view.h:283 >> + EWK_PAGINATION_MODE_END /**< number of pagination mode */ > > Why do you need this EWK_PAGINATION_MODE_END ? I think we should remove it. Agree with Chris.
I thought EWK_PAGINATION_MODE_END is needed when someone needed to change the pagination mode step by step. If it is not a common way, I'll remove it and let use "EWK_PAGINATION_MODE_BOTTOM_TO_TOP + 1" in that case.
Created attachment 170603 [details] Patch
Comment on attachment 170603 [details] Patch LGTM.
Comment on attachment 170603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170603&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.h:808 > + Nit : Unneeded line. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:970 > + ditto.
Created attachment 170621 [details] Patch
Comment on attachment 170621 [details] Patch Clearing flags on attachment: 170621 Committed r132479: <http://trac.webkit.org/changeset/132479>
All reviewed patches have been landed. Closing bug.