98064 2012-10-01 13:54:26 -0700 ScrollView::setScrollPosition is overridden by FrameView, but is not virtual 2012-10-02 09:46:17 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 97287 1 thorton webkit-unassigned andersca bdakin simon.fraser oldest_to_newest 731951 0 thorton 2012-10-01 13:54:26 -0700 This results in some functions (scrollBy) using ScrollView's implementation, and others (scrollTo) using FrameView's, and leads to a lot of confusion/brokenness. 731956 1 166536 thorton 2012-10-01 13:58:48 -0700 Created attachment 166536 patch 732311 2 thorton 2012-10-01 23:20:53 -0700 Thanks! http://trac.webkit.org/changeset/130123 732707 3 166536 darin 2012-10-02 09:20:58 -0700 Comment on attachment 166536 patch View in context: https://bugs.webkit.org/attachment.cgi?id=166536&action=review > Source/WebCore/platform/ScrollView.h:187 > + virtual void setScrollPosition(const IntPoint&); I recommend using the OVERRIDE macro in cases like this. 732732 4 thorton 2012-10-02 09:46:17 -0700 (In reply to comment #3) > (From update of attachment 166536 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166536&action=review > > > Source/WebCore/platform/ScrollView.h:187 > > + virtual void setScrollPosition(const IntPoint&); > > I recommend using the OVERRIDE macro in cases like this. Hi, Darin! I did, on FrameView (the overriding subclass). I don't think it belongs on ScrollView since ScrollView is originating setScrollPosition, right? 166536 2012-10-01 13:58:48 -0700 2012-10-02 09:20:58 -0700 patch 0001-virtualize-setScrollPosition.patch text/plain 2445 thorton RnJvbSAzOWU4OGFiMWUxNjc2NWVjOWRhZjIyNzAxOGRiNmJhYTc3ZTk5Zjk5IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBUaW0gSG9ydG9uIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+ CkRhdGU6IE1vbiwgMSBPY3QgMjAxMiAxMzoyMzoyOCAtMDcwMApTdWJqZWN0OiBbUEFUQ0ggMS80 XSB2aXJ0dWFsaXplIHNldFNjcm9sbFBvc2l0aW9uCgotLS0KIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZyAgICAgICAgICAgICB8ICAgMTYgKysrKysrKysrKysrKysrKwogU291cmNlL1dlYkNvcmUv cGFnZS9GcmFtZVZpZXcuaCAgICAgIHwgICAgMiArLQogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v U2Nyb2xsVmlldy5oIHwgICAgMiArLQogMyBmaWxlcyBjaGFuZ2VkLCAxOCBpbnNlcnRpb25zKCsp LCAyIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBi L1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBiNzI1MWZiLi5lNGZlZGE1IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTItMTAtMDEgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hv cnRvbkBhcHBsZS5jb20+CisKKyAgICAgICAgU2Nyb2xsVmlldzo6c2V0U2Nyb2xsUG9zaXRpb24g aXMgb3ZlcnJpZGRlbiBieSBGcmFtZVZpZXcsIGJ1dCBpcyBub3QgdmlydHVhbAorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTgwNjQKKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBWaXJ0dWFsaXplIFNjcm9sbFZp ZXc6OnNldFNjcm9sbFBvc2l0aW9uLCBhbmQgb3ZlcnJpZGUgaXQgaW4gRnJhbWVWaWV3LgorCisg ICAgICAgIE5vIG5ldyB0ZXN0cywgdGhpcyBjYXVzZXMgc3VidGxlIGJlaGF2aW9yIGRpZmZlcmVu Y2VzIGluIGN1cnJlbnRseS11bnRlc3RhYmxlIGNvZGUuCisKKyAgICAgICAgKiBwYWdlL0ZyYW1l Vmlldy5oOgorICAgICAgICAoRnJhbWVWaWV3KToKKyAgICAgICAgKiBwbGF0Zm9ybS9TY3JvbGxW aWV3Lmg6CisgICAgICAgIChTY3JvbGxWaWV3KToKKwogMjAxMi0xMC0wMSAgR2xlbm4gQWRhbXMg IDxnbGVubkBza3luYXYuY29tPgogCiAgICAgICAgIFlZREVCVUcgZG9lc24ndCBwcmludCB0b2tl biB2YWx1ZXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmggYi9T b3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5oCmluZGV4IGUxMGJkMjAuLjg0YjE1ZDIgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmgKKysrIGIvU291cmNlL1dl YkNvcmUvcGFnZS9GcmFtZVZpZXcuaApAQCAtMTc2LDcgKzE3Niw3IEBAIHB1YmxpYzoKICAgICB2 aXJ0dWFsIEludFJlY3Qgd2luZG93UmVzaXplclJlY3QoKSBjb25zdDsKIAogICAgIHZpcnR1YWwg dm9pZCBzZXRGaXhlZFZpc2libGVDb250ZW50UmVjdChjb25zdCBJbnRSZWN0JikgT1ZFUlJJREU7 Ci0gICAgdm9pZCBzZXRTY3JvbGxQb3NpdGlvbihjb25zdCBJbnRQb2ludCYpOworICAgIHZpcnR1 YWwgdm9pZCBzZXRTY3JvbGxQb3NpdGlvbihjb25zdCBJbnRQb2ludCYpIE9WRVJSSURFOwogICAg IHZvaWQgc2Nyb2xsUG9zaXRpb25DaGFuZ2VkVmlhUGxhdGZvcm1XaWRnZXQoKTsKICAgICB2aXJ0 dWFsIHZvaWQgcmVwYWludEZpeGVkRWxlbWVudHNBZnRlclNjcm9sbGluZygpOwogICAgIHZpcnR1 YWwgdm9pZCB1cGRhdGVGaXhlZEVsZW1lbnRzQWZ0ZXJTY3JvbGxpbmcoKTsKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1Njcm9sbFZpZXcuaCBiL1NvdXJjZS9XZWJDb3JlL3Bs YXRmb3JtL1Njcm9sbFZpZXcuaAppbmRleCAyY2JjYzI3Li5hOGNjN2QwIDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9TY3JvbGxWaWV3LmgKKysrIGIvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vU2Nyb2xsVmlldy5oCkBAIC0xODQsNyArMTg0LDcgQEAgcHVibGljOgogICAgIElu dFBvaW50IGNhY2hlZFNjcm9sbFBvc2l0aW9uKCkgY29uc3QgeyByZXR1cm4gbV9jYWNoZWRTY3Jv bGxQb3NpdGlvbjsgfQogCiAgICAgLy8gRnVuY3Rpb25zIGZvciBzY3JvbGxpbmcgdGhlIHZpZXcu Ci0gICAgdm9pZCBzZXRTY3JvbGxQb3NpdGlvbihjb25zdCBJbnRQb2ludCYpOworICAgIHZpcnR1 YWwgdm9pZCBzZXRTY3JvbGxQb3NpdGlvbihjb25zdCBJbnRQb2ludCYpOwogICAgIHZvaWQgc2Ny b2xsQnkoY29uc3QgSW50U2l6ZSYgcykgeyByZXR1cm4gc2V0U2Nyb2xsUG9zaXRpb24oc2Nyb2xs UG9zaXRpb24oKSArIHMpOyB9CiAKICAgICAvLyBUaGlzIGZ1bmN0aW9uIHNjcm9sbHMgYnkgbGlu ZXMsIHBhZ2VzIG9yIHBpeGVscy4KLS0gCjEuNy45LjYgKEFwcGxlIEdpdC0zMS4xKQoK