121889 2013-09-24 19:07:56 -0700 [Windows] Attempting to scroll from a non-scrollable layer results the main document getting scrolled 2013-09-25 16:27:02 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 roger_fong webkit-unassigned bdakin bfulgham commit-queue esprehn+autocc glenn hyatt jonlee kondapallykalyan roger_fong simon.fraser thorton oldest_to_newest 932848 0 roger_fong 2013-09-24 19:07:56 -0700 This applies to anything that uses RenderLayer::scrollbyRecursive to scroll and where the initial caller of the method is a layer that is not scrollable and has no overflow clip. The win port uses this for scrolling in response to a panning gesture. If a layer that for which hasOverflowCiip returns false is first touched when intiating the pan we end up just scrolling the main document. This is because the method doesn't appropriately recursively search up the RenderLayer tree. It should walk up the RenderLayer tree in its entirety and not decide to just bail as soon as we reach a layer that has no overflow clip. 932849 1 roger_fong 2013-09-24 19:08:52 -0700 <rdar://problem/15072441> 932851 2 roger_fong 2013-09-24 19:16:21 -0700 I'm not sure why we haven't seen any issues on mac...hrmmm...will have to look through usages of scrollbyrecursive in the mac port. 932856 3 roger_fong 2013-09-24 19:21:33 -0700 (In reply to comment #2) > I'm not sure why we haven't seen any issues on mac...hrmmm...will have to look through usages of scrollbyrecursive in the mac port. O! The win port is calling this method from a layer returned by "RenderLayer::enclosingLayer()" instead of "RenderLayer::enclosingScrollableLayer()", that might be the real issue. 933209 4 roger_fong 2013-09-25 14:55:48 -0700 I could make scrollbyrecursively calling itself on its enclosingScrollableLayer if the layer itself is not scrollable. However, it seems the pattern here in general is that whatever calls this method is already a scrollable layer, so I will change the win port to match that for now. I think we should consider fixing this method in the future though. Will add a fixme. 933217 5 212630 roger_fong 2013-09-25 15:16:22 -0700 Created attachment 212630 patch 933234 6 212632 roger_fong 2013-09-25 16:01:12 -0700 Created attachment 212632 patch removed the fixme, filed a separate bug for the issue: https://bugs.webkit.org/show_bug.cgi?id=121929 933236 7 thorton 2013-09-25 16:04:35 -0700 The changelog could be better. 933248 8 roger_fong 2013-09-25 16:24:48 -0700 Committed: http://trac.webkit.org/changeset/156433 with a better changelog 933249 9 thorton 2013-09-25 16:27:02 -0700 (In reply to comment #8) > Committed: http://trac.webkit.org/changeset/156433 > with a better changelog agreed 212630 2013-09-25 15:16:22 -0700 2013-09-25 16:01:12 -0700 patch patch.patch text/plain 3202 roger_fong SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NjQyMikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDEzLTA5LTI1ICBSb2dlciBG b25nICA8cm9nZXJfZm9uZ0BhcHBsZS5jb20+CisKKyAgICAgICAgW1dpbmRvd3NdIEF0dGVtcHRp bmcgdG8gc2Nyb2xsIGZyb20gYSBub24tc2Nyb2xsYWJsZSBsYXllciByZXN1bHRzIHRoZSBtYWlu IGRvY3VtZW50IGdldHRpbmcgc2Nyb2xsZWQuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xMjE4ODkuCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNTA3 MjQ0MT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IHJlbmRlcmluZy9SZW5kZXJMYXllci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJMYXll cjo6c2Nyb2xsQnlSZWN1cnNpdmVseSk6IEFkZCBhIEZJWE1FLgorCiAyMDEzLTA5LTI1ICBCZW5q YW1pbiBQb3VsYWluICA8YmVuamFtaW5Ad2Via2l0Lm9yZz4KIAogICAgICAgICBUaWUgdGhlIGxp ZmUgb2YgRG9jdW1lbnRTdHlsZVNoZWV0Q29sbGVjdGlvbiBhbmQgRG9jdW1lbnQgdG9nZXRoZXIK SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJMYXllci5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckxheWVyLmNwcAkocmV2aXNpb24g MTU2NDE2KQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckxheWVyLmNwcAkod29y a2luZyBjb3B5KQpAQCAtMjE3Niw2ICsyMTc2LDkgQEAKICAgICBpZiAocmVuZGVyZXIoKS5wYXJl bnQoKSkKICAgICAgICAgcmVzdHJpY3RlZEJ5TGluZUNsYW1wID0gIXJlbmRlcmVyKCkucGFyZW50 KCktPnN0eWxlKCktPmxpbmVDbGFtcCgpLmlzTm9uZSgpOwogCisgICAgLy8gRklYTUU6IElmIHRo ZSBSZW5kZXJMYXllciB0aGF0IGNhbGxzIHRoaXMgbWV0aG9kIGlzIG5vdCBzY3JvbGxhYmxlIHdl IGVuZCB1cCBub3QgbW92aW5nIHVwIHRoZSBSZW5kZXJMYXllciB0cmVlCisgICAgLy8gYW5kIHdl IGNhbiBza2lwIGxheWVycyB0aGF0IHNob3VsZCBoYXZlIGJlZW4gc2Nyb2xsZWQuIFdlIHNob3Vs ZCBmaXggdGhpcyBzbyB0aGF0IGlmIHRoaXMgbWV0aG9kIGlzIG5vdCBjYWxsZWQgb24gYSBzY3Jv bGxhYmxlIFJlbmRlckxheWVyCisgICAgLy8gaXQgY2FsbCdzIGl0c2VsZiBmcm9tIHRoZSBsYXll ciByZXR1cm5lZCBieSBlbmNsb3NpbmdTY3JvbGxhYmxlTGF5ZXIoKQogICAgIGlmIChyZW5kZXJl cigpLmhhc092ZXJmbG93Q2xpcCgpICYmICFyZXN0cmljdGVkQnlMaW5lQ2xhbXApIHsKICAgICAg ICAgSW50U2l6ZSBuZXdTY3JvbGxPZmZzZXQgPSBzY3JvbGxPZmZzZXQoKSArIGRlbHRhOwogICAg ICAgICBzY3JvbGxUb09mZnNldChuZXdTY3JvbGxPZmZzZXQsIGNsYW1wKTsKSW5kZXg6IFNvdXJj ZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L3dpbi9D aGFuZ2VMb2cJKHJldmlzaW9uIDE1NjQyMikKKysrIFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEyIEBACisyMDEzLTA5LTI1ICBSb2dlciBGb25n ICA8cm9nZXJfZm9uZ0BhcHBsZS5jb20+CisKKyAgICAgICAgW1dpbmRvd3NdIEF0dGVtcHRpbmcg dG8gc2Nyb2xsIGZyb20gYSBub24tc2Nyb2xsYWJsZSBsYXllciByZXN1bHRzIHRoZSBtYWluIGRv Y3VtZW50IGdldHRpbmcgc2Nyb2xsZWQuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMjE4ODkuCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNTA3MjQ0 MT4KKworICAgICAgICAqIFdlYlZpZXcuY3BwOgorICAgICAgICAoV2ViVmlldzo6Z2VzdHVyZSk6 CisKIDIwMTMtMDktMjQgIFJvZ2VyIEZvbmcgIDxyb2dlcl9mb25nQGFwcGxlLmNvbT4KIAogICAg ICAgICBbV2luZG93c10gT3ZlcnBhbm5pbmcgb2NjdXJzIGV2ZW4gd2hlbiB3aGF0IGlzIGJlaW5n IHBhbm5lZCBpc24ndCB0aGUgbWFpbiBkb2N1bWVudC4KSW5kZXg6IFNvdXJjZS9XZWJLaXQvd2lu L1dlYlZpZXcuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvd2luL1dlYlZpZXcuY3Bw CShyZXZpc2lvbiAxNTY0MTYpCisrKyBTb3VyY2UvV2ViS2l0L3dpbi9XZWJWaWV3LmNwcAkod29y a2luZyBjb3B5KQpAQCAtMTcwNiw3ICsxNzA2LDcgQEAKICAgICAgICAgICAgIGNvcmVGcmFtZS0+ dmlldygpLT5zY3JvbGxCeShJbnRTaXplKC1kZWx0YVgsIC1kZWx0YVkpKTsKICAgICAgICAgICAg IHNjcm9sbGVkQXJlYSA9IGNvcmVGcmFtZS0+dmlldygpOwogICAgICAgICB9IGVsc2UKLSAgICAg ICAgICAgIG1fZ2VzdHVyZVRhcmdldE5vZGUtPnJlbmRlcmVyKCktPmVuY2xvc2luZ0xheWVyKCkt PnNjcm9sbEJ5UmVjdXJzaXZlbHkoSW50U2l6ZSgtZGVsdGFYLCAtZGVsdGFZKSwgV2ViQ29yZTo6 UmVuZGVyTGF5ZXI6OlNjcm9sbE9mZnNldENsYW1wZWQsICZzY3JvbGxlZEFyZWEpOworICAgICAg ICAgICAgbV9nZXN0dXJlVGFyZ2V0Tm9kZS0+cmVuZGVyZXIoKS0+ZW5jbG9zaW5nTGF5ZXIoKS0+ ZW5jbG9zaW5nU2Nyb2xsYWJsZUxheWVyKCktPnNjcm9sbEJ5UmVjdXJzaXZlbHkoSW50U2l6ZSgt ZGVsdGFYLCAtZGVsdGFZKSwgV2ViQ29yZTo6UmVuZGVyTGF5ZXI6OlNjcm9sbE9mZnNldENsYW1w ZWQsICZzY3JvbGxlZEFyZWEpOwogCiAgICAgICAgIGlmICghKFVwZGF0ZVBhbm5pbmdGZWVkYmFj a1B0cigpICYmIEJlZ2luUGFubmluZ0ZlZWRiYWNrUHRyKCkgJiYgRW5kUGFubmluZ0ZlZWRiYWNr UHRyKCkpKSB7CiAgICAgICAgICAgICBDbG9zZUdlc3R1cmVJbmZvSGFuZGxlUHRyKCkoZ2VzdHVy ZUhhbmRsZSk7Cg== 212632 2013-09-25 16:01:12 -0700 2013-09-25 16:04:23 -0700 patch patch.patch text/plain 1561 roger_fong SW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L3dpbi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NjQyMikKKysrIFNvdXJjZS9XZWJLaXQv d2luL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEyIEBACisyMDEzLTA5LTI1 ICBSb2dlciBGb25nICA8cm9nZXJfZm9uZ0BhcHBsZS5jb20+CisKKyAgICAgICAgW1dpbmRvd3Nd IEF0dGVtcHRpbmcgdG8gc2Nyb2xsIGZyb20gYSBub24tc2Nyb2xsYWJsZSBsYXllciByZXN1bHRz IHRoZSBtYWluIGRvY3VtZW50IGdldHRpbmcgc2Nyb2xsZWQuCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjE4ODkuCisgICAgICAgIDxyZGFyOi8vcHJv YmxlbS8xNTA3MjQ0MT4KKworICAgICAgICAqIFdlYlZpZXcuY3BwOgorICAgICAgICAoV2ViVmll dzo6Z2VzdHVyZSk6CisKIDIwMTMtMDktMjQgIFJvZ2VyIEZvbmcgIDxyb2dlcl9mb25nQGFwcGxl LmNvbT4KIAogICAgICAgICBbV2luZG93c10gT3ZlcnBhbm5pbmcgb2NjdXJzIGV2ZW4gd2hlbiB3 aGF0IGlzIGJlaW5nIHBhbm5lZCBpc24ndCB0aGUgbWFpbiBkb2N1bWVudC4KSW5kZXg6IFNvdXJj ZS9XZWJLaXQvd2luL1dlYlZpZXcuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvd2lu L1dlYlZpZXcuY3BwCShyZXZpc2lvbiAxNTY0MTYpCisrKyBTb3VyY2UvV2ViS2l0L3dpbi9XZWJW aWV3LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTcwNiw3ICsxNzA2LDcgQEAKICAgICAgICAgICAg IGNvcmVGcmFtZS0+dmlldygpLT5zY3JvbGxCeShJbnRTaXplKC1kZWx0YVgsIC1kZWx0YVkpKTsK ICAgICAgICAgICAgIHNjcm9sbGVkQXJlYSA9IGNvcmVGcmFtZS0+dmlldygpOwogICAgICAgICB9 IGVsc2UKLSAgICAgICAgICAgIG1fZ2VzdHVyZVRhcmdldE5vZGUtPnJlbmRlcmVyKCktPmVuY2xv c2luZ0xheWVyKCktPnNjcm9sbEJ5UmVjdXJzaXZlbHkoSW50U2l6ZSgtZGVsdGFYLCAtZGVsdGFZ KSwgV2ViQ29yZTo6UmVuZGVyTGF5ZXI6OlNjcm9sbE9mZnNldENsYW1wZWQsICZzY3JvbGxlZEFy ZWEpOworICAgICAgICAgICAgbV9nZXN0dXJlVGFyZ2V0Tm9kZS0+cmVuZGVyZXIoKS0+ZW5jbG9z aW5nTGF5ZXIoKS0+ZW5jbG9zaW5nU2Nyb2xsYWJsZUxheWVyKCktPnNjcm9sbEJ5UmVjdXJzaXZl bHkoSW50U2l6ZSgtZGVsdGFYLCAtZGVsdGFZKSwgV2ViQ29yZTo6UmVuZGVyTGF5ZXI6OlNjcm9s bE9mZnNldENsYW1wZWQsICZzY3JvbGxlZEFyZWEpOwogCiAgICAgICAgIGlmICghKFVwZGF0ZVBh bm5pbmdGZWVkYmFja1B0cigpICYmIEJlZ2luUGFubmluZ0ZlZWRiYWNrUHRyKCkgJiYgRW5kUGFu bmluZ0ZlZWRiYWNrUHRyKCkpKSB7CiAgICAgICAgICAgICBDbG9zZUdlc3R1cmVJbmZvSGFuZGxl UHRyKCkoZ2VzdHVyZUhhbmRsZSk7Cg==