217955 2020-10-20 07:52:43 -0700 [GTK] Zooming causes page to scroll to top 2020-11-03 02:18:36 -0800 1 1 1 Unclassified WebKit WebKitGTK Other Unspecified Unspecified RESOLVED FIXED DoNotImportToRadar P2 Normal --- 1 erusan clord achristensen aperez beidson berto bugs-noreply cgarcia clord ews-watchlist gustavo justin_michaud youennf oldest_to_newest 1699708 0 erusan 2020-10-20 07:52:43 -0700 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 1702523 1 aperez 2020-10-28 15:35:49 -0700 This is reproducible also in the current trunk—just checked with a recent build from yesterday. 1702682 2 412635 clord 2020-10-29 05:54:01 -0700 Created attachment 412635 Patch 1702683 3 ews-watchlist 2020-10-29 05:55:05 -0700 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 1702687 4 clord 2020-10-29 05:57:05 -0700 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 :) 1702702 5 412635 cgarcia 2020-10-29 06:34:28 -0700 Comment on attachment 412635 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? 1702705 6 clord 2020-10-29 06:45:34 -0700 (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. 1703553 7 clord 2020-11-02 09:12:05 -0800 (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. 1703867 8 413020 clord 2020-11-03 01:42:10 -0800 Created attachment 413020 Patch 1703877 9 ews-feeder 2020-11-03 02:18:34 -0800 Committed r269290: <https://trac.webkit.org/changeset/269290> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413020. 412635 2020-10-29 05:54:01 -0700 2020-11-03 01:42:06 -0800 Patch bug-217955-20201029125400.patch text/plain 1465 clord U3VidmVyc2lvbiBSZXZpc2lvbjogMjY4OTc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGZmZGY3ZDc4ZWU1ZDI5NjEy NDUyNmY2NzVlODFjZGM2Y2RhMjA4MjIuLjAzZjc1MjM1ZTI1ZmVlM2RlMGIwNjVhMDA3YzYxZGJl ZDdjMjIxMzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMjAtMTAtMjkgIENocmlzIExv cmQgIDxjbG9yZEBpZ2FsaWEuY29tPgorCisgICAgICAgIFtHVEtdIFpvb21pbmcgY2F1c2VzIHBh Z2UgdG8gc2Nyb2xsIHRvIHRvcAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjE3OTU1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgRG9uJ3QgcmVzZXQgcGFnZSBzY2FsZSB3aGVuIHNldHRpbmcgem9vbSBsZXZl bCBvbiBHTElCIGJhY2tlbmQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvQVBJL2dsaWIvV2ViS2l0 V2ViVmlldy5jcHA6CisgICAgICAgICh3ZWJraXRfd2ViX3ZpZXdfc2V0X3pvb21fbGV2ZWwpOgor CiAyMDIwLTEwLTI5ICBDaHJpcyBMb3JkICA8Y2xvcmRAaWdhbGlhLmNvbT4KIAogICAgICAgICBb R1RLXSBTbW9vdGggc2Nyb2xsaW5nIHNob3VsZCBub3QgYXBwbHkgdG8gY29udGludW91cyBzY3Jv bGxpbmcgd2l0aCBzeW5jIHNjcm9sbGluZwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVBy b2Nlc3MvQVBJL2dsaWIvV2ViS2l0V2ViVmlldy5jcHAgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vz cy9BUEkvZ2xpYi9XZWJLaXRXZWJWaWV3LmNwcAppbmRleCAzMjM3ZDJhNzg0NDJhODFmNTdmNTJl OGY5YmIwYjQ1NGQwYTZkMTFkLi41OWZlYWFkNTU5OWVlMWM5ZmVhODM1NGQxOWU2ZWI1MjVjZjgx NjhiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9BUEkvZ2xpYi9XZWJLaXRX ZWJWaWV3LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9BUEkvZ2xpYi9XZWJLaXRX ZWJWaWV3LmNwcApAQCAtMzYwMSw3ICszNjAxLDYgQEAgdm9pZCB3ZWJraXRfd2ViX3ZpZXdfc2V0 X3pvb21fbGV2ZWwoV2ViS2l0V2ViVmlldyogd2ViVmlldywgZ2RvdWJsZSB6b29tTGV2ZWwpCiAg ICAgICAgIHJldHVybjsKIAogICAgIGF1dG8mIHBhZ2UgPSBnZXRQYWdlKHdlYlZpZXcpOwotICAg IHBhZ2Uuc2NhbGVQYWdlKDEuMCwgSW50UG9pbnQoKSk7IC8vIFJlc2V0IHBhZ2Ugc2NhbGUgd2hl biB6b29tIGxldmVsIGlzIGNoYW5nZWQKICAgICBpZiAod2Via2l0X3NldHRpbmdzX2dldF96b29t X3RleHRfb25seSh3ZWJWaWV3LT5wcml2LT5zZXR0aW5ncy5nZXQoKSkpCiAgICAgICAgIHBhZ2Uu c2V0VGV4dFpvb21GYWN0b3Ioem9vbUxldmVsKTsKICAgICBlbHNlCg== 413020 2020-11-03 01:42:10 -0800 2020-11-03 02:18:35 -0800 Patch bug-217955-20201103094209.patch text/plain 1471 clord U3VidmVyc2lvbiBSZXZpc2lvbjogMjY4OTc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGZmZGY3ZDc4ZWU1ZDI5NjEy NDUyNmY2NzVlODFjZGM2Y2RhMjA4MjIuLjVkYzFkZTU4Y2M0YzcwMDA2OTczNjNiYzAzYWFlODQ3 YmFhMzBjNmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMjAtMTAtMjkgIENocmlzIExv cmQgIDxjbG9yZEBpZ2FsaWEuY29tPgorCisgICAgICAgIFtHVEtdIFpvb21pbmcgY2F1c2VzIHBh Z2UgdG8gc2Nyb2xsIHRvIHRvcAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjE3OTU1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgQ2FybG9zIEdhcmNpYSBD YW1wb3MuCisKKyAgICAgICAgRG9uJ3QgcmVzZXQgcGFnZSBzY2FsZSB3aGVuIHNldHRpbmcgem9v bSBsZXZlbCBvbiBHTElCIGJhY2tlbmQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvQVBJL2dsaWIv V2ViS2l0V2ViVmlldy5jcHA6CisgICAgICAgICh3ZWJraXRfd2ViX3ZpZXdfc2V0X3pvb21fbGV2 ZWwpOgorCiAyMDIwLTEwLTI5ICBDaHJpcyBMb3JkICA8Y2xvcmRAaWdhbGlhLmNvbT4KIAogICAg ICAgICBbR1RLXSBTbW9vdGggc2Nyb2xsaW5nIHNob3VsZCBub3QgYXBwbHkgdG8gY29udGludW91 cyBzY3JvbGxpbmcgd2l0aCBzeW5jIHNjcm9sbGluZwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktp dC9VSVByb2Nlc3MvQVBJL2dsaWIvV2ViS2l0V2ViVmlldy5jcHAgYi9Tb3VyY2UvV2ViS2l0L1VJ UHJvY2Vzcy9BUEkvZ2xpYi9XZWJLaXRXZWJWaWV3LmNwcAppbmRleCAzMjM3ZDJhNzg0NDJhODFm NTdmNTJlOGY5YmIwYjQ1NGQwYTZkMTFkLi41OWZlYWFkNTU5OWVlMWM5ZmVhODM1NGQxOWU2ZWI1 MjVjZjgxNjhiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9BUEkvZ2xpYi9X ZWJLaXRXZWJWaWV3LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9BUEkvZ2xpYi9X ZWJLaXRXZWJWaWV3LmNwcApAQCAtMzYwMSw3ICszNjAxLDYgQEAgdm9pZCB3ZWJraXRfd2ViX3Zp ZXdfc2V0X3pvb21fbGV2ZWwoV2ViS2l0V2ViVmlldyogd2ViVmlldywgZ2RvdWJsZSB6b29tTGV2 ZWwpCiAgICAgICAgIHJldHVybjsKIAogICAgIGF1dG8mIHBhZ2UgPSBnZXRQYWdlKHdlYlZpZXcp OwotICAgIHBhZ2Uuc2NhbGVQYWdlKDEuMCwgSW50UG9pbnQoKSk7IC8vIFJlc2V0IHBhZ2Ugc2Nh bGUgd2hlbiB6b29tIGxldmVsIGlzIGNoYW5nZWQKICAgICBpZiAod2Via2l0X3NldHRpbmdzX2dl dF96b29tX3RleHRfb25seSh3ZWJWaWV3LT5wcml2LT5zZXR0aW5ncy5nZXQoKSkpCiAgICAgICAg IHBhZ2Uuc2V0VGV4dFpvb21GYWN0b3Ioem9vbUxldmVsKTsKICAgICBlbHNlCg==