WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(1.44 KB, patch)
2020-11-03 01:42 PST
,
Chris Lord
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 412635
[details]
Patch
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
Created
attachment 413020
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug