126054 2013-12-19 22:34:54 -0800 REGRESSION: cnn.com will continue to reveal 1 px of overhang after rubber-banding at the top 2013-12-19 23:17:20 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 bdakin bdakin andersca bdakin cmarcelo commit-queue jamesr luiz simon.fraser thorton tonikitoo webkit-bug-importer oldest_to_newest 961747 0 bdakin 2013-12-19 22:34:54 -0800 http://trac.webkit.org/changeset/160791 caused this regression. Patch forthcoming. 961748 1 webkit-bug-importer 2013-12-19 22:35:40 -0800 <rdar://problem/15706328> 961749 2 219731 bdakin 2013-12-19 22:38:52 -0800 Created attachment 219731 Patch 961752 3 219731 simon.fraser 2013-12-19 22:44:48 -0800 Comment on attachment 219731 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219731&action=review > Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:287 > + float nearestXWithinBounds = std::max<float>(std::min<float>(currentScrollPosition.x(), maxPosition.x()), minPosition.x()); > + float nearestYWithinBounds = std::max<float>(std::min<float>(currentScrollPosition.y(), maxPosition.y()), minPosition.y()); Not sure why you're doing this in floats (yet). > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:739 > + bool currentlyConstrainsToContentEdge = m_scrollableArea->constrainsScrollingToContentEdge(); > + m_scrollableArea->setConstrainsScrollingToContentEdge(true); We can't use TemporaryChange<> here? 961757 4 bdakin 2013-12-19 23:12:36 -0800 (In reply to comment #3) > (From update of attachment 219731 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=219731&action=review > > > Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:287 > > + float nearestXWithinBounds = std::max<float>(std::min<float>(currentScrollPosition.x(), maxPosition.x()), minPosition.x()); > > + float nearestYWithinBounds = std::max<float>(std::min<float>(currentScrollPosition.y(), maxPosition.y()), minPosition.y()); > > Not sure why you're doing this in floats (yet). > Will change to ints. > > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:739 > > + bool currentlyConstrainsToContentEdge = m_scrollableArea->constrainsScrollingToContentEdge(); > > + m_scrollableArea->setConstrainsScrollingToContentEdge(true); > > We can't use TemporaryChange<> here? I don't think we can because the bool belongs to ScrollableArea and this is ScrollAnimator. I think that ScrollAnimator would need a direct reference to the variable to use a TemporaryChange<>. Let me know if I am wrong though! I would be happy to fix it in a follow-up patch. 961758 5 bdakin 2013-12-19 23:17:20 -0800 http://trac.webkit.org/changeset/160898 219731 2013-12-19 22:38:52 -0800 2013-12-19 22:44:48 -0800 Patch for-review.txt text/plain 6969 bdakin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2MDg5NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI5IEBACisyMDEzLTEyLTE5ICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OOiBjbm4uY29tIHdp bGwgY29udGludWUgdG8gcmV2ZWFsIDEgcHggb2Ygb3ZlcmhhbmcgYWZ0ZXIgcnViYmVyLWJhbmRp bmcgCisgICAgICAgIGF0IHRoZSB0b3AKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTEyNjA1NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIFRoaXMgcmVncmVzc2lvbiB3YXMgY2F1c2VkIGJ5IGh0dHA6Ly90cmFj LndlYmtpdC5vcmcvY2hhbmdlc2V0LzE2MDc5MSBJdCB0dXJucyBvdXQgCisgICAgICAgIHRoYXQg dGhlIGxpbmUgb2YgY29kZSBJIHJlbW92ZWQgd2FzIG5vdCBhbHdheXMgYSBuby1vcC4gSW4gc29t ZSBpbnN0YW5jZXMsIGxpa2Ugb24gCisgICAgICAgIGNubi5jb20sIGl0IHdvdWxkIGVuc3VyZSB0 aGF0IG91ciBmaW5hbCBzY3JvbGwgcG9zaXRpb24gYWZ0ZXIgYSBydWJiZXItYmFuZCB3YXMgCisg ICAgICAgIG5vdCBzb21ldGhpbmcgd2l0aGluIHRoZSBvdmVyaGFuZyBhcmVhLiBJdCB3YXMgc3Rp bGwgd3JvbmcgaW4gaXRzIGFzc3VtcHRpb24gdGhhdCAKKyAgICAgICAgcnViYmVyLWJhbmQgaXMg YWx3YXlzIGJvdW5jaW5nIGJhY2sgdGhlIHNwb3QgaXQgb3JpZ2luYXRlZCBmcm9tLiBTbyB0aGlz IHBhdGNoIAorICAgICAgICBjb250aW51ZXMgdG8gaWdub3JlIHRoZSBydWJiZXItYmFuZHMgb3Jp Z2luLCBhbmQgaW5zdGVhZCBmaW5kcyB0aGUgbmVhcmVzdCBwb2ludCAKKyAgICAgICAgdGhhdCBp cyBub3QgaW4gdGhlIG92ZXJoYW5nIGFyZWEsIGFuZCBzY3JvbGxzIHRvIHRoYXQgcG9pbnQgaW5z dGVhZCBvZiB0aGUgb3JpZ2luLgorCisgICAgICAgICogcGFnZS9zY3JvbGxpbmcvbWFjL1Njcm9s bGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFjLmg6CisgICAgICAgICogcGFnZS9zY3JvbGxpbmcvbWFj L1Njcm9sbGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFjLm1tOgorICAgICAgICAoV2ViQ29yZTo6U2Ny b2xsaW5nVHJlZVNjcm9sbGluZ05vZGVNYWM6OmFkanVzdFNjcm9sbFBvc2l0aW9uVG9Cb3VuZHNJ Zk5lY2Vzc2FyeSk6CisgICAgICAgICogcGxhdGZvcm0vbWFjL1Njcm9sbEFuaW1hdG9yTWFjLmg6 CisgICAgICAgICogcGxhdGZvcm0vbWFjL1Njcm9sbEFuaW1hdG9yTWFjLm1tOgorICAgICAgICAo V2ViQ29yZTo6U2Nyb2xsQW5pbWF0b3JNYWM6OmFkanVzdFNjcm9sbFBvc2l0aW9uVG9Cb3VuZHNJ Zk5lY2Vzc2FyeSk6CisgICAgICAgICogcGxhdGZvcm0vbWFjL1Njcm9sbEVsYXN0aWNpdHlDb250 cm9sbGVyLmg6CisgICAgICAgICogcGxhdGZvcm0vbWFjL1Njcm9sbEVsYXN0aWNpdHlDb250cm9s bGVyLm1tOgorICAgICAgICAoV2ViQ29yZTo6U2Nyb2xsRWxhc3RpY2l0eUNvbnRyb2xsZXI6OnNu YXBSdWJiZXJCYW5kVGltZXJGaXJlZCk6CisKIDIwMTMtMTItMTkgIFNpbW9uIEZyYXNlciAgPHNp bW9uLmZyYXNlckBhcHBsZS5jb20+CiAKICAgICAgICAgQ2xhcmlmeSB0aGUgYmVoYXZpb3Igb2Yg Y29tcG9zaXRlZCBjYW52YXNlcwpJbmRleDogU291cmNlL1dlYkNvcmUvcGFnZS9zY3JvbGxpbmcv bWFjL1Njcm9sbGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFjLmgKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L1dlYkNvcmUvcGFnZS9zY3JvbGxpbmcvbWFjL1Njcm9sbGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFj LmgJKHJldmlzaW9uIDE2MDg0NykKKysrIFNvdXJjZS9XZWJDb3JlL3BhZ2Uvc2Nyb2xsaW5nL21h Yy9TY3JvbGxpbmdUcmVlU2Nyb2xsaW5nTm9kZU1hYy5oCSh3b3JraW5nIGNvcHkpCkBAIC02MSw2 ICs2MSw3IEBAIHByaXZhdGU6CiAgICAgdmlydHVhbCB2b2lkIGltbWVkaWF0ZVNjcm9sbEJ5V2l0 aG91dENvbnRlbnRFZGdlQ29uc3RyYWludHMoY29uc3QgRmxvYXRTaXplJikgT1ZFUlJJREU7CiAg ICAgdmlydHVhbCB2b2lkIHN0YXJ0U25hcFJ1YmJlcmJhbmRUaW1lcigpIE9WRVJSSURFOwogICAg IHZpcnR1YWwgdm9pZCBzdG9wU25hcFJ1YmJlcmJhbmRUaW1lcigpIE9WRVJSSURFOworICAgIHZp cnR1YWwgdm9pZCBhZGp1c3RTY3JvbGxQb3NpdGlvblRvQm91bmRzSWZOZWNlc3NhcnkoKSBPVkVS UklERTsKIAogICAgIEludFBvaW50IHNjcm9sbFBvc2l0aW9uKCkgY29uc3Q7CiAgICAgdm9pZCBz ZXRTY3JvbGxQb3NpdGlvbihjb25zdCBJbnRQb2ludCYpOwpJbmRleDogU291cmNlL1dlYkNvcmUv cGFnZS9zY3JvbGxpbmcvbWFjL1Njcm9sbGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFjLm1tCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BhZ2Uvc2Nyb2xsaW5nL21hYy9TY3JvbGxpbmdUcmVl U2Nyb2xsaW5nTm9kZU1hYy5tbQkocmV2aXNpb24gMTYwODQ3KQorKysgU291cmNlL1dlYkNvcmUv cGFnZS9zY3JvbGxpbmcvbWFjL1Njcm9sbGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFjLm1tCSh3b3Jr aW5nIGNvcHkpCkBAIC0yNzcsNiArMjc3LDE5IEBAIHZvaWQgU2Nyb2xsaW5nVHJlZVNjcm9sbGlu Z05vZGVNYWM6OnN0b3AKICAgICBtX3NuYXBSdWJiZXJiYW5kVGltZXIgPSBudWxscHRyOwogfQog Cit2b2lkIFNjcm9sbGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFjOjphZGp1c3RTY3JvbGxQb3NpdGlv blRvQm91bmRzSWZOZWNlc3NhcnkoKQoreworICAgIEludFBvaW50IGN1cnJlbnRTY3JvbGxQb3Np dGlvbiA9IGFic29sdXRlU2Nyb2xsUG9zaXRpb24oKTsKKyAgICBJbnRQb2ludCBtaW5Qb3NpdGlv biA9IG1pbmltdW1TY3JvbGxQb3NpdGlvbigpOworICAgIEludFBvaW50IG1heFBvc2l0aW9uID0g bWF4aW11bVNjcm9sbFBvc2l0aW9uKCk7CisKKyAgICBmbG9hdCBuZWFyZXN0WFdpdGhpbkJvdW5k cyA9IHN0ZDo6bWF4PGZsb2F0PihzdGQ6Om1pbjxmbG9hdD4oY3VycmVudFNjcm9sbFBvc2l0aW9u LngoKSwgbWF4UG9zaXRpb24ueCgpKSwgbWluUG9zaXRpb24ueCgpKTsKKyAgICBmbG9hdCBuZWFy ZXN0WVdpdGhpbkJvdW5kcyA9IHN0ZDo6bWF4PGZsb2F0PihzdGQ6Om1pbjxmbG9hdD4oY3VycmVu dFNjcm9sbFBvc2l0aW9uLnkoKSwgbWF4UG9zaXRpb24ueSgpKSwgbWluUG9zaXRpb24ueSgpKTsK KworICAgIEludFBvaW50IG5lYXJlc3RQb2ludFdpdGhpbkJvdW5kcyhuZWFyZXN0WFdpdGhpbkJv dW5kcywgbmVhcmVzdFlXaXRoaW5Cb3VuZHMpOworICAgIGltbWVkaWF0ZVNjcm9sbEJ5KG5lYXJl c3RQb2ludFdpdGhpbkJvdW5kcyAtIGN1cnJlbnRTY3JvbGxQb3NpdGlvbik7Cit9CisKIEludFBv aW50IFNjcm9sbGluZ1RyZWVTY3JvbGxpbmdOb2RlTWFjOjpzY3JvbGxQb3NpdGlvbigpIGNvbnN0 CiB7CiAgICAgaWYgKHNob3VsZFVwZGF0ZVNjcm9sbExheWVyUG9zaXRpb25Pbk1haW5UaHJlYWQo KSkKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21hYy9TY3JvbGxBbmltYXRvck1hYy5o Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21hYy9TY3JvbGxBbmltYXRv ck1hYy5oCShyZXZpc2lvbiAxNjA4NDcpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9tYWMv U2Nyb2xsQW5pbWF0b3JNYWMuaAkod29ya2luZyBjb3B5KQpAQCAtMTUyLDYgKzE1Miw3IEBAIHBy aXZhdGU6CiAgICAgdmlydHVhbCB2b2lkIGltbWVkaWF0ZVNjcm9sbEJ5KGNvbnN0IEZsb2F0U2l6 ZSYpIE9WRVJSSURFOwogICAgIHZpcnR1YWwgdm9pZCBzdGFydFNuYXBSdWJiZXJiYW5kVGltZXIo KSBPVkVSUklERTsKICAgICB2aXJ0dWFsIHZvaWQgc3RvcFNuYXBSdWJiZXJiYW5kVGltZXIoKSBP VkVSUklERTsKKyAgICB2aXJ0dWFsIHZvaWQgYWRqdXN0U2Nyb2xsUG9zaXRpb25Ub0JvdW5kc0lm TmVjZXNzYXJ5KCkgT1ZFUlJJREU7CiAKICAgICBib29sIHBpbm5lZEluRGlyZWN0aW9uKGZsb2F0 IGRlbHRhWCwgZmxvYXQgZGVsdGFZKTsKICAgICB2b2lkIHNuYXBSdWJiZXJCYW5kVGltZXJGaXJl ZChUaW1lcjxTY3JvbGxBbmltYXRvck1hYz4qKTsKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL21hYy9TY3JvbGxBbmltYXRvck1hYy5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9tYWMvU2Nyb2xsQW5pbWF0b3JNYWMubW0JKHJldmlzaW9uIDE2MDg0NykKKysr IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21hYy9TY3JvbGxBbmltYXRvck1hYy5tbQkod29ya2lu ZyBjb3B5KQpAQCAtNzMzLDYgKzczMywxOCBAQCBGbG9hdFBvaW50IFNjcm9sbEFuaW1hdG9yTWFj OjphZGp1c3RTY3JvCiAgICAgcmV0dXJuIEZsb2F0UG9pbnQobmV3WCwgbmV3WSk7CiB9CiAKK3Zv aWQgU2Nyb2xsQW5pbWF0b3JNYWM6OmFkanVzdFNjcm9sbFBvc2l0aW9uVG9Cb3VuZHNJZk5lY2Vz c2FyeSgpCit7CisgICAgYm9vbCBjdXJyZW50bHlDb25zdHJhaW5zVG9Db250ZW50RWRnZSA9IG1f c2Nyb2xsYWJsZUFyZWEtPmNvbnN0cmFpbnNTY3JvbGxpbmdUb0NvbnRlbnRFZGdlKCk7CisgICAg bV9zY3JvbGxhYmxlQXJlYS0+c2V0Q29uc3RyYWluc1Njcm9sbGluZ1RvQ29udGVudEVkZ2UodHJ1 ZSk7CisKKyAgICBJbnRQb2ludCBjdXJyZW50U2Nyb2xsUG9zaXRpb24gPSBhYnNvbHV0ZVNjcm9s bFBvc2l0aW9uKCk7CisgICAgRmxvYXRQb2ludCBuZWFyZXN0UG9pbnRXaXRoaW5Cb3VuZHMgPSBh ZGp1c3RTY3JvbGxQb3NpdGlvbklmTmVjZXNzYXJ5KGFic29sdXRlU2Nyb2xsUG9zaXRpb24oKSk7 CisgICAgaW1tZWRpYXRlU2Nyb2xsQnkobmVhcmVzdFBvaW50V2l0aGluQm91bmRzIC0gY3VycmVu dFNjcm9sbFBvc2l0aW9uKTsKKworICAgIG1fc2Nyb2xsYWJsZUFyZWEtPnNldENvbnN0cmFpbnNT Y3JvbGxpbmdUb0NvbnRlbnRFZGdlKGN1cnJlbnRseUNvbnN0cmFpbnNUb0NvbnRlbnRFZGdlKTsK K30KKwogdm9pZCBTY3JvbGxBbmltYXRvck1hYzo6aW1tZWRpYXRlU2Nyb2xsVG8oY29uc3QgRmxv YXRQb2ludCYgbmV3UG9zaXRpb24pCiB7CiAgICAgRmxvYXRQb2ludCBhZGp1c3RlZFBvc2l0aW9u ID0gYWRqdXN0U2Nyb2xsUG9zaXRpb25JZk5lY2Vzc2FyeShuZXdQb3NpdGlvbik7CkluZGV4OiBT b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9tYWMvU2Nyb2xsRWxhc3RpY2l0eUNvbnRyb2xsZXIuaAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9tYWMvU2Nyb2xsRWxhc3RpY2l0 eUNvbnRyb2xsZXIuaAkocmV2aXNpb24gMTYwODQ3KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vbWFjL1Njcm9sbEVsYXN0aWNpdHlDb250cm9sbGVyLmgJKHdvcmtpbmcgY29weSkKQEAgLTU3 LDYgKzU3LDEwIEBAIHB1YmxpYzoKICAgICB2aXJ0dWFsIHZvaWQgaW1tZWRpYXRlU2Nyb2xsQnlX aXRob3V0Q29udGVudEVkZ2VDb25zdHJhaW50cyhjb25zdCBGbG9hdFNpemUmKSA9IDA7CiAgICAg dmlydHVhbCB2b2lkIHN0YXJ0U25hcFJ1YmJlcmJhbmRUaW1lcigpID0gMDsKICAgICB2aXJ0dWFs IHZvaWQgc3RvcFNuYXBSdWJiZXJiYW5kVGltZXIoKSA9IDA7CisKKyAgICAvLyBJZiB0aGUgY3Vy cmVudCBzY3JvbGwgcG9zaXRpb24gaXMgd2l0aGluIHRoZSBvdmVyaGFuZyBhcmVhLCB0aGlzIGZ1 bmN0aW9uIHdpbGwgY2F1c2UKKyAgICAvLyB0aGUgcGFnZSB0byBzY3JvbGwgdG8gdGhlIG5lYXJl c3QgYm91bmRhcnkgcG9pbnQuCisgICAgdmlydHVhbCB2b2lkIGFkanVzdFNjcm9sbFBvc2l0aW9u VG9Cb3VuZHNJZk5lY2Vzc2FyeSgpID0gMDsKIH07CiAKIGNsYXNzIFNjcm9sbEVsYXN0aWNpdHlD b250cm9sbGVyIHsKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21hYy9TY3JvbGxFbGFz dGljaXR5Q29udHJvbGxlci5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9tYWMvU2Nyb2xsRWxhc3RpY2l0eUNvbnRyb2xsZXIubW0JKHJldmlzaW9uIDE2MDg0NykKKysr IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21hYy9TY3JvbGxFbGFzdGljaXR5Q29udHJvbGxlci5t bQkod29ya2luZyBjb3B5KQpAQCAtMzQyLDYgKzM0Miw4IEBAIHZvaWQgU2Nyb2xsRWxhc3RpY2l0 eUNvbnRyb2xsZXI6OnNuYXBSdWIKICAgICAgICAgICAgIG1fc3RyZXRjaFNjcm9sbEZvcmNlLnNl dFdpZHRoKHJlYm91bmREZWx0YUZvckVsYXN0aWNEZWx0YShuZXdTdHJldGNoLndpZHRoKCkpKTsK ICAgICAgICAgICAgIG1fc3RyZXRjaFNjcm9sbEZvcmNlLnNldEhlaWdodChyZWJvdW5kRGVsdGFG b3JFbGFzdGljRGVsdGEobmV3U3RyZXRjaC5oZWlnaHQoKSkpOwogICAgICAgICB9IGVsc2Ugewor ICAgICAgICAgICAgbV9jbGllbnQtPmFkanVzdFNjcm9sbFBvc2l0aW9uVG9Cb3VuZHNJZk5lY2Vz c2FyeSgpOworCiAgICAgICAgICAgICBzdG9wU25hcFJ1YmJlcmJhbmRUaW1lcigpOwogICAgICAg ICAgICAgbV9zdHJldGNoU2Nyb2xsRm9yY2UgPSBGbG9hdFNpemUoKTsKICAgICAgICAgICAgIG1f c3RhcnRUaW1lID0gMDsK