RESOLVED FIXED 217955
[GTK] Zooming causes page to scroll to top
https://bugs.webkit.org/show_bug.cgi?id=217955
Summary [GTK] Zooming causes page to scroll to top
erusan
Reported 2020-10-20 07:52:43 PDT
Zooming on a a page (Ctrl+- or Ctrl++ or Ctrl+0) immediately "scrolls" (doesn't actually trigger a scroll animation) to the top of the page, losing the user's place. This can be tested on probably any page, including planet.gnome.org 1) Scroll down a page 2) Ctrl++ to zoom in Result: You are now back at the top of the page epiphany-browser 3.38.1 webkitgtk 2.30.1
Attachments
Patch (1.43 KB, patch)
2020-10-29 05:54 PDT, Chris Lord
no flags
Patch (1.44 KB, patch)
2020-11-03 01:42 PST, Chris Lord
no flags
Adrian Perez
Comment 1 2020-10-28 15:35:49 PDT
This is reproducible also in the current trunk—just checked with a recent build from yesterday.
Chris Lord
Comment 2 2020-10-29 05:54:01 PDT
EWS Watchlist
Comment 3 2020-10-29 05:55:05 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Chris Lord
Comment 4 2020-10-29 05:57:05 PDT
This is pretty weird, it seems the behaviour was intentional...? Either way, here's the patch that reverses it, this certainly isn't desirable behaviour in any situation as far as I can tell, but maybe we'll find out in review :)
Carlos Garcia Campos
Comment 5 2020-10-29 06:34:28 PDT
Comment on attachment 412635 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412635&action=review > Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:-3604 > - page.scalePage(1.0, IntPoint()); // Reset page scale when zoom level is changed This was added for pinch to zoom in r235529, are you sure this is no longer needed?
Chris Lord
Comment 6 2020-10-29 06:45:34 PDT
(In reply to Carlos Garcia Campos from comment #5) > Comment on attachment 412635 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=412635&action=review > > > Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:-3604 > > - page.scalePage(1.0, IntPoint()); // Reset page scale when zoom level is changed > > This was added for pinch to zoom in r235529, are you sure this is no longer > needed? Not certain at all, was it actually needed in the first place? Is there a platform or method that would make this easy to test? I've cc'd Justin in case they're available to comment on this.
Chris Lord
Comment 7 2020-11-02 09:12:05 PST
(In reply to Carlos Garcia Campos from comment #5) > Comment on attachment 412635 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=412635&action=review > > > Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:-3604 > > - page.scalePage(1.0, IntPoint()); // Reset page scale when zoom level is changed > > This was added for pinch to zoom in r235529, are you sure this is no longer > needed? Reading that patch and reading what scalePage does, I don't see why this is necessary. It seems to be a user experience thing, but currently, it just has a negative effect. I don't see any situation where you'd really want the scroll position to suddenly reset and text zoom and page scale are two different things, I don't see why changing text zoom would necessarily imply that page scale should be reset. My suggestion is to just check in this change, and if there's some situation we're not seeing here, it ought to become evident soon.
Chris Lord
Comment 8 2020-11-03 01:42:10 PST
EWS
Comment 9 2020-11-03 02:18:34 PST
Committed r269290: <https://trac.webkit.org/changeset/269290> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413020 [details].
Note You need to log in before you can comment on or make changes to this bug.