Bug 182174 – [GTK] Zooming gesture incorrectly uses scale instead of zoom

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Jan-Michael Brummer]

Currently zooming gesture is implemented using pageScale. Unfortuantely this isn't the typical expectation of a user:
A user wants to zoom the page. Therefore page zoom must be used. Additionally this fixes the async of epiphany after
zooming through gesture and the zoom option within the hamburger menu. Otherwise the user doesn't have a possibility
to revert to the "default" zoom level.

[Jan-Michael Brummer]

Created attachment 332376 [details]
Browser using scaling

[Jan-Michael Brummer]

Created attachment 332377 [details]
Browser using zooming

[Jan-Michael Brummer]

Attached two image showing the difference between those two function. FWIW: In addition i will attach a image of firefox after zooming gesture.

[Jan-Michael Brummer]

Created attachment 332379 [details]
Firefox after zooming

[Jan-Michael Brummer]

Created attachment 332382 [details]
Patch changing scale to zoom

[Michael Catanzaro]

Comment on attachment 332382 [details]
Patch changing scale to zoom

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

> Source/WebKit/UIProcess/gtk/GestureController.cpp:258
> +    g_object_notify(G_OBJECT(m_page.viewWidget()), "zoom-level");

Notifying for another object's property feels pretty fragile. I guess if it's the only place outside WebKitWebView where the zoom level needs to be changed, it's not the end of the world. But can you use webkit_web_view_set_zoom_level() instead? Then the gesture would additionally respect the text-only zoom setting.

[Jan-Michael Brummer]

Created attachment 332545 [details]
Patch changing scale to zoom - V2

Use webkit_web_view_set_zoom_level ().

[Michael Catanzaro]

Comment on attachment 332545 [details]
Patch changing scale to zoom - V2

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

> Source/WebKit/UIProcess/gtk/GestureController.cpp:33
> +#include "WebKitWebViewPrivate.h"

Better to #include "WebKitWebView.h" since you don't need any of the private bits.

[Jan-Michael Brummer]

Created attachment 332549 [details]
Patch changing scale to zoom - V3

[WebKit Commit Bot]

Comment on attachment 332549 [details]
Patch changing scale to zoom - V3

Clearing flags on attachment: 332549

Committed r227744: <https://trac.webkit.org/changeset/227744>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Garcia Campos]

Comment on attachment 332549 [details]
Patch changing scale to zoom - V3

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

> Source/WebKit/UIProcess/gtk/GestureController.cpp:33
> +#include "WebKitWebView.h"

This is wrong. We shouldn't include WebKitWebView here, is kind of a layering violation, see below.

> Source/WebKit/UIProcess/gtk/GestureController.cpp:257
> +    webkit_web_view_set_zoom_level(WEBKIT_WEB_VIEW(m_page.viewWidget()), m_scale);

We can't assume viewWidget will be a WebKitWebView, it can be a WebKitWebViewBase. This will probably crash if you try to zoom the web inspector, for example. Anyway, I'll fix this as part of bug #182224.