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

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Justin Michaud]

Currently on gtk, pinching to zoom causes the page to be zoomed in the same way as pressing ctrl+`+`. It should scale and pan the page like on ios or macos.

[Justin Michaud]

Questions for reviewer:
- The performance is not very good (it wasn't before either). Any ideas on how to improve it?
- I have no idea how/where to write a test for this

[Michael Catanzaro]

(In reply to Justin Michaud from comment #1)
> - I have no idea how/where to write a test for this

It would be pretty hard since you'd need to develop some way to emulate pinching a touchscreen. We require tests for everything that can be easily tested, not for absolutely everything. I think manual testing will suffice for this.

[Justin Michaud]

Created attachment 347503 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Jan-Michael Brummer]

For me it's a question of philosophy. My current development focus is feature parity with Firefox and not iOS apps.

I really like the current situation as it simplifies the usage within the hamburger menu for touch input: One simple common function which does it all. So at the moment i do not want a change here. My two cents...

[Michael Catanzaro]

I'm struggling to understand (a) what is the current behavior, (b) how this patch changes the behavior, and (c) why you (Jan-Michael) think the change would be undesirable.

My understanding is this somehow reverts the recetn zooming/scaling change by Carlos Garcia? I've already lost track of that bug, and don't remember it at all.

How is mobile Firefox's behavior different from Safari's? In general, we do try to emulate Safari's behavior -- since we are WebKit after all -- except when there is good reason to do otherwise.

[EWS Watchlist]

Comment on attachment 347503 [details]
Patch

Attachment 347503 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8919107

New failing tests:
http/tests/security/local-video-source-from-remote.html

[EWS Watchlist]

Created attachment 347520 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Michael Catanzaro]

Comment on attachment 347503 [details]
Patch

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

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3121
> + * webkit_web_view_scale_page:
> + * @web_view: a #WebKitWebView
> + * @zoom_level: the zoom level
> + * @scaleCenterX: X coordinate of scale center in page coordinates
> + * @scaleCenterY: y coordinate of scale center in page coordinates
> + * @viewX: X coordinate where scale center should appear, in view coordinates
> + * @viewY: y coordinate where scale center should appear, in view coordinates

Looking at the parameters, I don't understand how this API could plausibly be used by a client application. Where would the coordinates come from? Could you show some example usage, e.g. by updating MiniBrowser to use this API somehow? I suspect it can only sanely be called from PageClientImpl::scale, and if so, it should be declared in WebKitWebViewPrivate.h instead of being public.

Don't we also need a companion API to get the current zoom, scale center, and view coordinates?

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3125
> + * the web view.

If this remains public, then you'll need to add:

Since: 2.24

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3155
> +

Avoid the stray whitespace change.

> Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp:497
> +void PageClientImpl::scale(double scale, const IntPoint& scaleCenterInPageCoordinates, const IntPoint& newScaleCenterInViewCoordinates)
> +{
> +    if (WEBKIT_IS_WEB_VIEW(m_viewWidget)) {
> +        webkit_web_view_scale_page(WEBKIT_WEB_VIEW(m_viewWidget), scale,
> +            scaleCenterInPageCoordinates.x(), scaleCenterInPageCoordinates.y(),
> +            newScaleCenterInViewCoordinates.x(), newScaleCenterInViewCoordinates.y());
> +        return;
> +    }
> +
> +    webkitWebViewBaseGetPage(WEBKIT_WEB_VIEW_BASE(m_viewWidget))->scaleAndPanPage(scale, scaleCenterInPageCoordinates, newScaleCenterInViewCoordinates);
> +}

Why doesn't iOS need to implement this function?

> Source/WebKit/UIProcess/API/gtk/WebKitWebView.h:413
> +WEBKIT_API void
> +webkit_web_view_scale_page                           (WebKitWebView             *web_view,
> +                                                      gdouble                    zoom_level,
> +                                                      int                        scaleCenterX,
> +                                                      int                        scaleCenterY,
> +                                                      int                        viewX,
> +                                                      int                        viewY);

gint

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1125
> +    void zoom(double scale, const IntPoint& initialPoint, const IntPoint& centre) final

"center" would be more familiar to most.

> Source/WebKit/UIProcess/WebPageProxy.h:775
> +    void scaleAndPanPage(double scale, WebCore::IntPoint scaleCenterInPageCoordinates, WebCore::IntPoint newScaleCenterInViewCoordinates);

I think this is acceptable, but it is a bit unfortunate to need to add a new message here that is used only by GTK. That's why I'm curious why iOS doesn't need to implement PageClientImpl::scale.

[Michael Catanzaro]

(In reply to Build Bot from comment #7)
> Comment on attachment 347503 [details]
> Patch
> 
> Attachment 347503 [details] did not pass win-ews (win):
> Output: https://webkit-queues.webkit.org/results/8919107
> 
> New failing tests:
> http/tests/security/local-video-source-from-remote.html

By the way, just ignore this bot. We know your patch didn't cause this unrelated test failure.

[Jan-Michael Brummer]

@Justin: Would it be possible to show us a video before/after this change for those of us who do not have a touchscreen (Michael)?

[Carlos Garcia Campos]

Regarding platform integration or UI related stuff we don't try to emulate Safari, but GNOME. So, pinch to zoom should be consistent with other GNOME applications. Only when the Web has a clearly different approach to other desktop applications we follow what firefox and/or chromium does, not Safari. And without any public API required, it should be transparent to the applications.

[Justin Michaud]

https://imgur.com/a/LVrSMGj <- Before & After

This seems to me to be consistent with other gnome applications (ex: maps) and chromium on linux. The "before" behaviour is consistent with Firefox on Linux, but the "after" behaviour is consistent with Firefox and Edge on 
Windows last I checked.

Michael raises a great point about iOS not having a message for scaling. On second thought, it is not needed. I made the new message because I originally had a call to convert the scale centre from view coordinates to page coordinates, but later I realised that wasn't necessary.

To verify, it seems this is what iOS does:

``` WKWebView.mm:4111
void WebViewImpl::setMagnification(double magnification, CGPoint centerPoint)
{
    if (magnification <= 0 || isnan(magnification) || isinf(magnification))
        [NSException raise:NSInvalidArgumentException format:@"Magnification should be a positive number"];

    dismissContentRelativeChildWindowsWithAnimation(false);

    m_page->scalePageInViewCoordinates(magnification, WebCore::roundedIntPoint(centerPoint));
}
```

``` ViewGestureControllerMac.mm:141
void ViewGestureController::handleMagnificationGestureEvent(NSEvent *event, FloatPoint origin)
{
    ASSERT(m_activeGestureType == ViewGestureType::None || m_activeGestureType == ViewGestureType::Magnification);

    if (m_activeGestureType == ViewGestureType::None) {
        if (event.phase != NSEventPhaseBegan)
            return;

        // FIXME: We drop the first frame of the gesture on the floor, because we don't have the visible content bounds yet.
        m_magnification = m_webPageProxy.pageScaleFactor();
        m_webPageProxy.process().send(Messages::ViewGestureGeometryCollector::CollectGeometryForMagnificationGesture(), m_webPageProxy.pageID());
        m_lastMagnificationGestureWasSmartMagnification = false;

        return;
    }

    // We're still waiting for the DidCollectGeometry callback.
    if (!m_visibleContentRectIsValid)
        return;

    willBeginGesture(ViewGestureType::Magnification);

    double scale = event.magnification;
    double scaleWithResistance = resistanceForDelta(scale, m_magnification) * scale;

    m_magnification += m_magnification * scaleWithResistance;
    m_magnification = std::min(std::max(m_magnification, minElasticMagnification), maxElasticMagnification);

    m_magnificationOrigin = origin;

    if (m_frameHandlesMagnificationGesture)
        m_webPageProxy.scalePage(m_magnification, roundedIntPoint(origin));
    else
        m_webPageProxy.drawingArea()->adjustTransientZoom(m_magnification, scaledMagnificationOrigin(origin, m_magnification));

    if (event.phase == NSEventPhaseEnded || event.phase == NSEventPhaseCancelled)
        endMagnificationGesture();
}
```

I will have a new patch shortly. Thanks!

[Justin Michaud]

Created attachment 347640 [details]
Patch

[Michael Catanzaro]

Could you post the before/after videos as webm please? I think imgur is mp4.

[Michael Catanzaro]

Comment on attachment 347640 [details]
Patch

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

Well the code changes, at least, are way simpler now. That looks good.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1128
> +        auto* page = webkitWebViewBaseGetPage(m_webView);

Instead of webkitWebViewBaseGetPage(), just use webView->priv->pageProxy directly.

> Source/WebKit/UIProcess/gtk/GestureController.h:54
> +    virtual void zoom(double scale, const WebCore::IntPoint& initialPoint, const WebCore::IntPoint& centre) = 0;

center here, too

[Michael Catanzaro]

https://tabos.de/jbrummer/before.webm
https://tabos.de/jbrummer/after.webm

[Michael Catanzaro]

Discussed a bit with Jan-Michael on IRC. He's changed his mind and given approval.

(In reply to Carlos Garcia Campos from comment #12)
> Regarding platform integration or UI related stuff we don't try to emulate
> Safari, but GNOME. So, pinch to zoom should be consistent with other GNOME
> applications. Only when the Web has a clearly different approach to other
> desktop applications we follow what firefox and/or chromium does, not
> Safari. And without any public API required, it should be transparent to the
> applications.

Jan-Michael is saying he changed GNOME Photos from the Firefox on Linux to the iOS-style behavior last year... so it sounds like we are paving the way regardless.

[Michael Catanzaro]

Comment on attachment 347640 [details]
Patch

r=me, but cq- due to two minor review comments

[Michael Catanzaro]

BTW, I assume this does not regress normal non-touchscreen zooming.

[Justin Michaud]

Created attachment 347699 [details]
Patch

[Justin Michaud]

It seems this patch also fixes this bug: https://bugs.webkit.org/show_bug.cgi?id=154963

I fixed the Canadian-ism for center. For the second comment, webkitWebViewBaseGetPage is also used by the previous function, so I am wondering if I should change both, or leave it as-is.

For non touchscreen zooming, it works but the two zooms are independent. I wonder if this is a change that should be made in epiphany (to reset the page scaling when you press ctrl + [0-+]), or if we should reset the page scale when you set the zoom. In the first case, I think a getter / setter would need to be added to the public api. What do you think?

Thanks again!

[Michael Catanzaro]

(In reply to Justin Michaud from comment #22)
> It seems this patch also fixes this bug:
> https://bugs.webkit.org/show_bug.cgi?id=154963
> 
> I fixed the Canadian-ism for center. For the second comment,
> webkitWebViewBaseGetPage is also used by the previous function, so I am
> wondering if I should change both, or leave it as-is.

Hm yes, I would, while you're at it, since you changed a line just below it. That's the only place in this file where it's used. Of course it makes no real difference, but our style is to access the priv variable directly when there's no difference.

> For non touchscreen zooming, it works but the two zooms are independent. I
> wonder if this is a change that should be made in epiphany (to reset the
> page scaling when you press ctrl + [0-+]), or if we should reset the page
> scale when you set the zoom. In the first case, I think a getter / setter
> would need to be added to the public api. What do you think?

OK, it sounds like this is the last bit that needs to be worked out.

I'm confused why scale and zoom are different concepts in the first place. By far the simplest solution is to have just one zoom that changes when you pinch. It would be unfortunate if we have to expose a second setting for scale.

[EWS Watchlist]

Comment on attachment 347699 [details]
Patch

Attachment 347699 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8936302

New failing tests:
http/wpt/workers/name-property-enhanced.html

[EWS Watchlist]

Created attachment 347737 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[Justin Michaud]

The current behaviour (before this patch) is that pinching is the same as zooming with ctrl-+.

After this patch (and on other platforms and browsers), there is a difference between scaling the page with a pinch gesture, and zooming in with ctrl-[+-] which re-lays out the page. If you want to avoid exposing a second setting for scale, we can re-set the scale when the zoom level changes. I don't see a use case where an embedder would not want this behaviour, but I wasn't sure.

I am going to:
- Replace the two use cases of webkitWebViewBaseGetPage with priv
- Reset the scale to 1 whenever the zoom level is changed

Does that sound good?

[EWS Watchlist]

Comment on attachment 347699 [details]
Patch

Attachment 347699 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8938988

New failing tests:
http/wpt/workers/name-property-enhanced.html

[EWS Watchlist]

Created attachment 347756 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Michael Catanzaro]

(In reply to Justin Michaud from comment #26)
> I am going to:
> - Replace the two use cases of webkitWebViewBaseGetPage with priv
> - Reset the scale to 1 whenever the zoom level is changed
> 
> Does that sound good?

OK, that makes sense to me.

I don't know if resetting the scale is what users are going to expect from other browsers(?), but we can always change that in the future if needed.

[Justin Michaud]

I just checked, you are right. It is just ctrl+0 that resets scaling. I guess adding a public api for the scalePage is probably the best option then. I will add that tomorrow, and hopefully everything will be good to go. I am thinking webkit_web_view_set_scale_level(WebKitWebView*, gdouble scale, gint pageX, gint pageY) + webkit_web_view_get_scale_level.

Once this lands, I can submit a patch to epiphany to fix ctrl+0.

Thanks again for the help!

[Carlos Garcia Campos]

Comment on attachment 347699 [details]
Patch

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

> Source/WebKit/ChangeLog:7
> +

Please, explain the changes here.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:-1127
> -        m_webView->priv->pageClient->zoom(scale);

I think this is the only user of PageClientImpl::zoom() so it can be removed.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1131
> +        FloatPoint scaledZoomCenter = FloatPoint(initialPoint);

FloatPoint scaledZoomCenter(initialPoint);

> Source/WebKit/UIProcess/gtk/GestureController.cpp:206
> -    m_client.zoom(m_scale);
> +    m_client.zoom(m_scale, m_initialPoint, m_viewPoint);

I wonder if, instead of passing both values to the client and let them compute the center in page coordinates, we could do the conversion here and pass only the center to the client that would only need to call WebPageProxy::scalePage(scale, center);

[Justin Michaud]

Created attachment 347807 [details]
Patch

[Justin Michaud]

I tested the new api with the following patch to epiphany:

```
diff --git a/src/ephy-window.c b/src/ephy-window.c
index a86fd45b2..d7f005ba3 100644
--- a/src/ephy-window.c
+++ b/src/ephy-window.c
@@ -967,7 +967,7 @@ sync_tab_zoom (WebKitWebView *web_view, GParamSpec *pspec, EphyWindow *window)
 {
   GActionGroup *action_group;
   GAction *action;
-  gboolean can_zoom_in = TRUE, can_zoom_out = TRUE, can_zoom_normal = FALSE;
+  gboolean can_zoom_in = TRUE, can_zoom_out = TRUE;
   double zoom;
   GtkWidget *zoom_level_button;
   gchar *zoom_level;
@@ -991,10 +991,6 @@ sync_tab_zoom (WebKitWebView *web_view, GParamSpec *pspec, EphyWindow *window)
     can_zoom_out = FALSE;
   }
 
-  if (zoom != 1.0) {
-    can_zoom_normal = TRUE;
-  }
-
   action_group = gtk_widget_get_action_group (GTK_WIDGET (window),
                                               "win");
 
@@ -1003,7 +999,7 @@ sync_tab_zoom (WebKitWebView *web_view, GParamSpec *pspec, EphyWindow *window)
   action = g_action_map_lookup_action (G_ACTION_MAP (action_group), "zoom-out");
   g_simple_action_set_enabled (G_SIMPLE_ACTION (action), can_zoom_out);
   action = g_action_map_lookup_action (G_ACTION_MAP (action_group), "zoom-normal");
-  g_simple_action_set_enabled (G_SIMPLE_ACTION (action), can_zoom_normal);
+  g_simple_action_set_enabled (G_SIMPLE_ACTION (action), true);
 }
 
 static void
@@ -3472,6 +3468,30 @@ ephy_window_set_zoom (EphyWindow *window,
     webkit_web_view_set_zoom_level (web_view, zoom);
 }
 
+/**
+ * ephy_window_set_scale:
+ * @window: an #EphyWindow
+ * @scale: the desired zoom level
+ *
+ * Sets the scale on @window's active #EphyEmbed. A @scale of 1.0 corresponds to
+ * 100% scale (normal size).
+ **/
+void
+ephy_window_set_scale (EphyWindow *window,
+                      double      scale)
+{
+  EphyEmbed *embed;
+  WebKitWebView *web_view;
+
+  g_assert (EPHY_IS_WINDOW (window));
+
+  embed = window->active_embed;
+  g_assert (embed != NULL);
+
+  web_view = EPHY_GET_WEBKIT_WEB_VIEW_FROM_EMBED (embed);
+  webkit_web_view_set_page_scale (web_view, scale, 0, 0);
+}
+
 static void
 ephy_window_change_allow_popup_windows_state (GSimpleAction *action,
                                               GVariant      *state,
diff --git a/src/ephy-window.h b/src/ephy-window.h
index 1a6feab4d..6f66733ff 100644
--- a/src/ephy-window.h
+++ b/src/ephy-window.h
@@ -53,6 +53,9 @@ void              ephy_window_load_url            (EphyWindow *window,
 void              ephy_window_set_zoom            (EphyWindow *window,
                                                    double zoom);
 
+void              ephy_window_set_scale           (EphyWindow *window,
+                                                   double scale);
+
 void              ephy_window_activate_location   (EphyWindow *window);
 const char       *ephy_window_get_location        (EphyWindow *window);
 
diff --git a/src/window-commands.c b/src/window-commands.c
index 0d483808c..0a25bd18a 100644
--- a/src/window-commands.c
+++ b/src/window-commands.c
@@ -1782,6 +1782,7 @@ window_cmd_zoom_normal (GSimpleAction *action,
                         gpointer       user_data)
 {
   EphyWindow *window = user_data;
+  ephy_window_set_scale (window, 1.0);
   ephy_window_set_zoom (window, 1.0);
 }
``` 

With this patch, ctrl + 0 works as expected.

Thanks for your patience!

[Michael Catanzaro]

Comment on attachment 347807 [details]
Patch

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

I was OK with the previous patch, BTW.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3160
> + * @origin_x: the x coordinate for the origin
> + * @origin_y: the y coordinate for the origin

Well this brings back my old question: where is the supposed to get the values to pass here?

In your Epiphany patch, it always passes 0, which is fine when doing Ctrl+0.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #34)
> Well this brings back my old question: where is the supposed to get the
> values to pass here?

"where is the client application"

[Justin Michaud]

No idea. I assumed that there would be some api to get the current page scroll position, but it seems there isn't. I'm really not sure what to do now.

[Michael Catanzaro]

I was about ready to give r+ on attachment #347699 [details] before you brought back the public API.

Changing the zoom level is an unusual operation. While not great, I think it's OK if that resets the scale for now. We can always improve with new APIs in a future patch, right?

[Carlos Garcia Campos]

Comment on attachment 347807 [details]
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.

[Justin Michaud]

Created attachment 347921 [details]
Patch

[Michael Catanzaro]

Comment on attachment 347921 [details]
Patch

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

What does the Epiphany patch look like now? Epiphany has to call webkit_web_view_reset_page_scale() when modifying the zoom?

Is there any time a browser would want to use this API *except* when modifying the zoom? If so, r=me. If not, then WebKit should probably just reset the scale itself when the zoom changes, like you had proposed earlier?

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:3160
> + * Resets the page scale of @web_view, i.e. the factor by
> + * which the page is magnified without changing the layout

OH. So the difference between scale and zoom is that zoom changes the layout?

I finally understand!

P.S. add a period at the end of this sentence.

[Justin Michaud]

Created attachment 347924 [details]
Patch

[Justin Michaud]

Fixed the period.

I don't know if there is any other time the page scale would need to be reset. The problem is that we need to reset the scale when the zoom is reset, but not when it is changed. I suppose it is a pretty minor detail.

I am not sure how to add a test, since there is no way to get or set the page scale from the public api. I would appreciate some guidance. I think the unit test is the last remaining part of this, if it is required.

Ephy patch:
```
diff --git a/src/ephy-window.c b/src/ephy-window.c
index a86fd45b2..366c4d50a 100644
--- a/src/ephy-window.c
+++ b/src/ephy-window.c
@@ -967,7 +967,7 @@ sync_tab_zoom (WebKitWebView *web_view, GParamSpec *pspec, EphyWindow *window)
 {
   GActionGroup *action_group;
   GAction *action;
-  gboolean can_zoom_in = TRUE, can_zoom_out = TRUE, can_zoom_normal = FALSE;
+  gboolean can_zoom_in = TRUE, can_zoom_out = TRUE;
   double zoom;
   GtkWidget *zoom_level_button;
   gchar *zoom_level;
@@ -991,10 +991,6 @@ sync_tab_zoom (WebKitWebView *web_view, GParamSpec *pspec, EphyWindow *window)
     can_zoom_out = FALSE;
   }
 
-  if (zoom != 1.0) {
-    can_zoom_normal = TRUE;
-  }
-
   action_group = gtk_widget_get_action_group (GTK_WIDGET (window),
                                               "win");
 
@@ -1003,7 +999,7 @@ sync_tab_zoom (WebKitWebView *web_view, GParamSpec *pspec, EphyWindow *window)
   action = g_action_map_lookup_action (G_ACTION_MAP (action_group), "zoom-out");
   g_simple_action_set_enabled (G_SIMPLE_ACTION (action), can_zoom_out);
   action = g_action_map_lookup_action (G_ACTION_MAP (action_group), "zoom-normal");
-  g_simple_action_set_enabled (G_SIMPLE_ACTION (action), can_zoom_normal);
+  g_simple_action_set_enabled (G_SIMPLE_ACTION (action), true);
 }
 
 static void
@@ -3472,6 +3468,27 @@ ephy_window_set_zoom (EphyWindow *window,
     webkit_web_view_set_zoom_level (web_view, zoom);
 }
 
+/**
+ * ephy_window_reset_scale:
+ * @window: an #EphyWindow
+ *
+ * Resets the page scale on @window's active #EphyEmbed.
+ **/
+void
+ephy_window_reset_scale (EphyWindow *window)
+{
+  EphyEmbed *embed;
+  WebKitWebView *web_view;
+
+  g_assert (EPHY_IS_WINDOW (window));
+
+  embed = window->active_embed;
+  g_assert (embed != NULL);
+
+  web_view = EPHY_GET_WEBKIT_WEB_VIEW_FROM_EMBED (embed);
+  webkit_web_view_reset_page_scale (web_view);
+}
+
 static void
 ephy_window_change_allow_popup_windows_state (GSimpleAction *action,
                                               GVariant      *state,
diff --git a/src/ephy-window.h b/src/ephy-window.h
index 1a6feab4d..ca8d72707 100644
--- a/src/ephy-window.h
+++ b/src/ephy-window.h
@@ -53,6 +53,8 @@ void              ephy_window_load_url            (EphyWindow *window,
 void              ephy_window_set_zoom            (EphyWindow *window,
                                                    double zoom);
 
+void              ephy_window_reset_scale         (EphyWindow *window);
+
 void              ephy_window_activate_location   (EphyWindow *window);
 const char       *ephy_window_get_location        (EphyWindow *window);
 
diff --git a/src/window-commands.c b/src/window-commands.c
index 0d483808c..a014500ad 100644
--- a/src/window-commands.c
+++ b/src/window-commands.c
@@ -1783,6 +1783,7 @@ window_cmd_zoom_normal (GSimpleAction *action,
 {
   EphyWindow *window = user_data;
   ephy_window_set_zoom (window, 1.0);
+  ephy_window_reset_scale (window);
 }
 
 void
```

[Carlos Garcia Campos]

I prefer not to add new API when we don't know any other use case than resetting the scale when the zoom level is changed to 1.0. Because we can do that ourselves. So, I agree with Michael, let's do that for now. If that's eventually a problem, then we can simply revert the behavior and provide the public API.

[Justin Michaud]

Created attachment 348004 [details]
Patch

[Michael Catanzaro]

Comment on attachment 348004 [details]
Patch

Please set cq? to request commit!

[Justin Michaud]

Hey Michael,

How long after setting commit-queue? does it take usually to get commit-queue+?

[Adrian Perez]

Comment on attachment 348004 [details]
Patch

I think the reviewer forgot to set cq+, so I've just flipped the
flag (committers like myself can set the commit-queue flag, but
of course you still need a positive review beforehand ;D)

Thanks for your patch!

[WebKit Commit Bot]

Comment on attachment 348004 [details]
Patch

Clearing flags on attachment: 348004

Committed r235529: <https://trac.webkit.org/changeset/235529>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.