[Webkit-unassigned] [Bug 188746] [GTK] Touchscreen pinch to zoom should scale the page like other platforms

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188746

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #347807|review?                     |review-
              Flags|                            |

--- Comment #38 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 347807
  --> https://bugs.webkit.org/attachment.cgi?id=347807
Patch

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

I would also avoid to add new API. Also note that all new API must include a unit test. If apps only need to reset the scale, maybe it's better to only expose webkit_web_view_reset_scale(). If we really want to allow users to scale the page, we should provide a test case to ensure it's really possible with the proposed API. The ephy example always passing 1.0 and 0,0 as center is not enough. Note also that without new API we could merge this to 2.22 branch.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3158
> + * @scale_level: the page scale

I would use scale_factor

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3166
> + * example, a pinch-to-zoom touch gesture). The point (origin_x, origin_y)
> + * should be in page coordinates, scaled by the new scale factor.

I don't think we should receive page coordinates here. This function could be used to implement gestures differently, overriding the default WebKit behavior. In that case the user will be handling widget coords.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3168
> + * Since 2.24

Since:

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3170
> +void webkit_web_view_set_page_scale(WebKitWebView* webView, gdouble scaleLevel, gint originX, gint originY)

set_page_scale sounds like you are setting a property, but page scale isn't and the setter receives more parameters. I think this should be webkit_web_view_scale().

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3174
> +    auto& page = getPage(webView);

Here we would need to convert to page coordinates.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3179
> + * webkit_web_view_get_zoom_level:

get_page_scale

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3182
> + * Get the zoom level of @web_view, i.e. the factor by which the

zoom level -> page scale

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3187
> + * Since 2.24

Since:

> Source/WebKit/UIProcess/API/gtk/WebKitWebView.h:415
> +webkit_web_view_get_page_scale                       (WebKitWebView             *web_view);

wpe header would need to be updated too.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180823/772127f4/attachment.html>