39240 2010-05-17 14:47:54 -0700 [GTK] gtk_drag_begin should be initiated with a motion event 2010-06-14 17:16:20 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED INVALID P2 Normal --- 30623 1 mrobinson webkit-unassigned oldest_to_newest 226903 0 mrobinson 2010-05-17 14:47:54 -0700 This is necessary so that the internal drag context will be in the proper state for simulating dropping events in the DRT. 226905 1 56273 mrobinson 2010-05-17 14:51:06 -0700 Created attachment 56273 Patch 229248 2 56273 xan.lopez 2010-05-22 01:55:22 -0700 Comment on attachment 56273 Patch >diff --git a/WebKit/gtk/ChangeLog b/WebKit/gtk/ChangeLog >index e166b52a5cd7016ad0bf34adb2a69e4144a6abb8..b619c550eb61166801b588cfe5ba84aed70d3542 100644 >--- a/WebKit/gtk/ChangeLog >+++ b/WebKit/gtk/ChangeLog >@@ -1,3 +1,18 @@ >+2010-05-17 Martin Robinson <mrobinson@igalia.com> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ [GTK] gtk_drag_begin should be initiated with a motion event >+ https://bugs.webkit.org/show_bug.cgi?id=39240 >+ >+ * WebCoreSupport/DragClientGtk.cpp: >+ (WebKit::DragClient::startDrag): Use a motion event to initiate the drag. >+ * webkit/webkitprivate.cpp: Abstract this logic from webkitwebview.cpp into a helper method. >+ (WebKit::clientPositionToRootWindowPosition): Added. >+ * webkit/webkitprivate.h: Added declaration for WebKit::clientPositionToRootWindowPosition. >+ * webkit/webkitwebview.cpp: >+ (webkit_web_view_popup_menu_handler): Use the new helper method. OK, now you *are* missing the explanation in the ChangeLog :) >- GdkEvent* event = gdk_event_new(GDK_BUTTON_PRESS); >- reinterpret_cast<GdkEventButton*>(event)->window = gtk_widget_get_window(GTK_WIDGET(m_webView)); >- reinterpret_cast<GdkEventButton*>(event)->time = GDK_CURRENT_TIME; >+ GdkWindow* gdkWindow = gtk_widget_get_window(GTK_WIDGET(m_webView)); >+ IntPoint rootPosition(clientPositionToRootWindowPosition(gdkWindow, eventPos)); >+ >+ GdkEvent* event = gdk_event_new(GDK_MOTION_NOTIFY); >+ event->motion.window = gdkWindow; >+ event->motion.time = GDK_CURRENT_TIME; >+ event->motion.x_root = rootPosition.x(); >+ event->motion.y_root = rootPosition.y(); > Are you leaking the event and not adding a ref to the window? I remember some patch fixing this, so maybe I'm reviewing stuff in the wrong order, sigh. 229297 3 mrobinson 2010-05-22 12:15:20 -0700 (In reply to comment #2) > OK, now you *are* missing the explanation in the ChangeLog :) Agreed, will fix for the next patch! > Are you leaking the event and not adding a ref to the window? I remember some patch fixing this, so maybe I'm reviewing stuff in the wrong order, sigh. Yeah, this patch was made before I found and fixed the memory leak. I think I'll wait on this one until next week when I can get some second opinions on the most elegant way to accomplish what I need. Essentially the crux of the problem is that since the DRT will be simulating drag and drop without processing real mouse events, the internal state of the drag will be in the wrong place when it starts. Perhaps there is a better way to do this though. 238154 4 mrobinson 2010-06-14 17:16:20 -0700 This change should no longer be necessary after some other fixes to the DRT. 56273 2010-05-17 14:51:06 -0700 2010-06-11 13:12:33 -0700 Patch bug-39240-20100517145105.patch text/plain 5359 mrobinson ZGlmZiAtLWdpdCBhL1dlYktpdC9ndGsvQ2hhbmdlTG9nIGIvV2ViS2l0L2d0ay9DaGFuZ2VMb2cK aW5kZXggZTE2NmI1MmE1Y2Q3MDE2YWQwYmYzNGFkYjJhNjllNDE0NGE2YWJiOC4uYjYxOWM1NTBl YjYxMTY2ODAxYjU4OGNmZTViYTg0YWVkNzBkMzU0MiAxMDA2NDQKLS0tIGEvV2ViS2l0L2d0ay9D aGFuZ2VMb2cKKysrIGIvV2ViS2l0L2d0ay9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAx MC0wNS0xNyAgTWFydGluIFJvYmluc29uICA8bXJvYmluc29uQGlnYWxpYS5jb20+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW0dUS10gZ3RrX2RyYWdf YmVnaW4gc2hvdWxkIGJlIGluaXRpYXRlZCB3aXRoIGEgbW90aW9uIGV2ZW50CisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zOTI0MAorCisgICAgICAgICog V2ViQ29yZVN1cHBvcnQvRHJhZ0NsaWVudEd0ay5jcHA6CisgICAgICAgIChXZWJLaXQ6OkRyYWdD bGllbnQ6OnN0YXJ0RHJhZyk6IFVzZSBhIG1vdGlvbiBldmVudCB0byBpbml0aWF0ZSB0aGUgZHJh Zy4KKyAgICAgICAgKiB3ZWJraXQvd2Via2l0cHJpdmF0ZS5jcHA6IEFic3RyYWN0IHRoaXMgbG9n aWMgZnJvbSB3ZWJraXR3ZWJ2aWV3LmNwcCBpbnRvIGEgaGVscGVyIG1ldGhvZC4KKyAgICAgICAg KFdlYktpdDo6Y2xpZW50UG9zaXRpb25Ub1Jvb3RXaW5kb3dQb3NpdGlvbik6IEFkZGVkLgorICAg ICAgICAqIHdlYmtpdC93ZWJraXRwcml2YXRlLmg6IEFkZGVkIGRlY2xhcmF0aW9uIGZvciBXZWJL aXQ6OmNsaWVudFBvc2l0aW9uVG9Sb290V2luZG93UG9zaXRpb24uCisgICAgICAgICogd2Via2l0 L3dlYmtpdHdlYnZpZXcuY3BwOiAKKyAgICAgICAgKHdlYmtpdF93ZWJfdmlld19wb3B1cF9tZW51 X2hhbmRsZXIpOiBVc2UgdGhlIG5ldyBoZWxwZXIgbWV0aG9kLgorCiAyMDEwLTA1LTEyICBNYXJ0 aW4gUm9iaW5zb24gIDxtcm9iaW5zb25AaWdhbGlhLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBi eSBYYW4gTG9wZXouCmRpZmYgLS1naXQgYS9XZWJLaXQvZ3RrL1dlYkNvcmVTdXBwb3J0L0RyYWdD bGllbnRHdGsuY3BwIGIvV2ViS2l0L2d0ay9XZWJDb3JlU3VwcG9ydC9EcmFnQ2xpZW50R3RrLmNw cAppbmRleCA5MDBmYjYxYmVhMzkwZjFjZjdkNWJmNzI0YTVlYTA3NTYyM2Y0N2I1Li42ZmYyMTNk OGUyNGM4ZjM1NjI5ZjVmZGUxMjZkZjRiYjY1MTg1NzcwIDEwMDY0NAotLS0gYS9XZWJLaXQvZ3Rr L1dlYkNvcmVTdXBwb3J0L0RyYWdDbGllbnRHdGsuY3BwCisrKyBiL1dlYktpdC9ndGsvV2ViQ29y ZVN1cHBvcnQvRHJhZ0NsaWVudEd0ay5jcHAKQEAgLTc5LDkgKzc5LDE0IEBAIHZvaWQgRHJhZ0Ns aWVudDo6c3RhcnREcmFnKERyYWdJbWFnZVJlZiBpbWFnZSwgY29uc3QgSW50UG9pbnQmIGRyYWdJ bWFnZU9yaWdpbiwKICAgICBSZWZQdHI8RGF0YU9iamVjdEd0az4gZGF0YU9iamVjdCA9IGNsaXBi b2FyZEd0ay0+ZGF0YU9iamVjdCgpOwogCiAgICAgR1JlZlB0cjxHdGtUYXJnZXRMaXN0PiB0YXJn ZXRMaXN0KGNsaXBib2FyZEd0ay0+aGVscGVyKCktPnRhcmdldExpc3RGb3JEYXRhT2JqZWN0KGRh dGFPYmplY3QuZ2V0KCkpKTsKLSAgICBHZGtFdmVudCogZXZlbnQgPSBnZGtfZXZlbnRfbmV3KEdE S19CVVRUT05fUFJFU1MpOwotICAgIHJlaW50ZXJwcmV0X2Nhc3Q8R2RrRXZlbnRCdXR0b24qPihl dmVudCktPndpbmRvdyA9IGd0a193aWRnZXRfZ2V0X3dpbmRvdyhHVEtfV0lER0VUKG1fd2ViVmll dykpOwotICAgIHJlaW50ZXJwcmV0X2Nhc3Q8R2RrRXZlbnRCdXR0b24qPihldmVudCktPnRpbWUg PSBHREtfQ1VSUkVOVF9USU1FOworICAgIEdka1dpbmRvdyogZ2RrV2luZG93ID0gZ3RrX3dpZGdl dF9nZXRfd2luZG93KEdUS19XSURHRVQobV93ZWJWaWV3KSk7CisgICAgSW50UG9pbnQgcm9vdFBv c2l0aW9uKGNsaWVudFBvc2l0aW9uVG9Sb290V2luZG93UG9zaXRpb24oZ2RrV2luZG93LCBldmVu dFBvcykpOworCisgICAgR2RrRXZlbnQqIGV2ZW50ID0gZ2RrX2V2ZW50X25ldyhHREtfTU9USU9O X05PVElGWSk7CisgICAgZXZlbnQtPm1vdGlvbi53aW5kb3cgPSBnZGtXaW5kb3c7CisgICAgZXZl bnQtPm1vdGlvbi50aW1lID0gR0RLX0NVUlJFTlRfVElNRTsKKyAgICBldmVudC0+bW90aW9uLnhf cm9vdCA9IHJvb3RQb3NpdGlvbi54KCk7CisgICAgZXZlbnQtPm1vdGlvbi55X3Jvb3QgPSByb290 UG9zaXRpb24ueSgpOwogCiAgICAgR2RrRHJhZ0NvbnRleHQqIGNvbnRleHQgPSBndGtfZHJhZ19i ZWdpbihHVEtfV0lER0VUKG1fd2ViVmlldyksIHRhcmdldExpc3QuZ2V0KCksIGRyYWdBY3Rpb24s IDEsIGV2ZW50KTsKICAgICB3ZWJWaWV3LT5wcml2LT5kcmFnZ2luZ0RhdGFPYmplY3RzLnNldChj b250ZXh0LCBkYXRhT2JqZWN0KTsKZGlmZiAtLWdpdCBhL1dlYktpdC9ndGsvd2Via2l0L3dlYmtp dHByaXZhdGUuY3BwIGIvV2ViS2l0L2d0ay93ZWJraXQvd2Via2l0cHJpdmF0ZS5jcHAKaW5kZXgg ZDI3NDcwODZlMjc2MDQ3YTUwZDczMzY3M2Y5ZjdjODJmOTM4MTFlNS4uMGIwMWQxMmU1MGZhNDQ3 NjI3ZTdjNjhmMDJlZGFmNjdhMGFkYjFhNyAxMDA2NDQKLS0tIGEvV2ViS2l0L2d0ay93ZWJraXQv d2Via2l0cHJpdmF0ZS5jcHAKKysrIGIvV2ViS2l0L2d0ay93ZWJraXQvd2Via2l0cHJpdmF0ZS5j cHAKQEAgLTE3Niw2ICsxNzYsMTMgQEAgUGFzdGVib2FyZEhlbHBlckd0ayogcGFzdGVib2FyZEhl bHBlckluc3RhbmNlKCkKICAgICByZXR1cm4gaGVscGVyOwogfQogCitJbnRQb2ludCBjbGllbnRQ b3NpdGlvblRvUm9vdFdpbmRvd1Bvc2l0aW9uKEdka1dpbmRvdyogd2luZG93LCBjb25zdCBJbnRQ b2ludCYgY2xpZW50UG9zaXRpb24pCit7CisgICAgaW50IHgsIHk7CisgICAgZ2RrX3dpbmRvd19n ZXRfb3JpZ2luKHdpbmRvdywgJngsICZ5KTsKKyAgICByZXR1cm4gY2xpZW50UG9zaXRpb24gKyBJ bnRTaXplKHgsIHkpOworfQorCiB9IC8qKiBlbmQgbmFtZXNwYWNlIFdlYktpdCAqLwogCiBzdGF0 aWMgR3RrV2lkZ2V0KiBjdXJyZW50VG9wbGV2ZWxDYWxsYmFjayhXZWJLaXRTb3VwQXV0aERpYWxv ZyogZmVhdHVyZSwgU291cE1lc3NhZ2UqIG1lc3NhZ2UsIGdwb2ludGVyIHVzZXJEYXRhKQpkaWZm IC0tZ2l0IGEvV2ViS2l0L2d0ay93ZWJraXQvd2Via2l0cHJpdmF0ZS5oIGIvV2ViS2l0L2d0ay93 ZWJraXQvd2Via2l0cHJpdmF0ZS5oCmluZGV4IGFiY2EzYzgzY2M0MzA0MzkzMzFkYjY3ZmNmZjQ3 MmI0MjY4MzFmZTMuLmZjZmNkNmQ0MTE5YjI3NWZhNzAyNTBkNGQwNTY0OTc5YjVhZmYzNjQgMTAw NjQ0Ci0tLSBhL1dlYktpdC9ndGsvd2Via2l0L3dlYmtpdHByaXZhdGUuaAorKysgYi9XZWJLaXQv Z3RrL3dlYmtpdC93ZWJraXRwcml2YXRlLmgKQEAgLTU2LDYgKzU2LDcgQEAKICNpbmNsdWRlICJQ YWdlLmgiCiAjaW5jbHVkZSAiRnJhbWUuaCIKICNpbmNsdWRlICJJbnNwZWN0b3JDbGllbnRHdGsu aCIKKyNpbmNsdWRlICJJbnRQb2ludC5oIgogI2luY2x1ZGUgIkZyYW1lTG9hZGVyQ2xpZW50Lmgi CiAjaW5jbHVkZSAiUmVzb3VyY2VIYW5kbGUuaCIKICNpbmNsdWRlICJSZXNvdXJjZVJlcXVlc3Qu aCIKQEAgLTY4LDYgKzY5LDcgQEAKICNpbmNsdWRlIDxnbGliLmg+CiAjaW5jbHVkZSA8bGlic291 cC9zb3VwLmg+CiAKK3VzaW5nIFdlYkNvcmU6OkludFBvaW50OwogY2xhc3MgRG93bmxvYWRDbGll bnQ7CiAKIG5hbWVzcGFjZSBXZWJLaXQgewpAQCAtMTAzLDYgKzEwNSw4IEBAIG5hbWVzcGFjZSBX ZWJLaXQgewogICAgIFdlYktpdEhpdFRlc3RSZXN1bHQqIGtpdChjb25zdCBXZWJDb3JlOjpIaXRU ZXN0UmVzdWx0Jik7CiAKICAgICBXZWJLaXQ6OlBhc3RlYm9hcmRIZWxwZXJHdGsqIHBhc3RlYm9h cmRIZWxwZXJJbnN0YW5jZSgpOworCisgICAgSW50UG9pbnQgY2xpZW50UG9zaXRpb25Ub1Jvb3RX aW5kb3dQb3NpdGlvbihHZGtXaW5kb3cqLCBjb25zdCBJbnRQb2ludCYpOwogfQogCiBleHRlcm4g IkMiIHsKZGlmZiAtLWdpdCBhL1dlYktpdC9ndGsvd2Via2l0L3dlYmtpdHdlYnZpZXcuY3BwIGIv V2ViS2l0L2d0ay93ZWJraXQvd2Via2l0d2Vidmlldy5jcHAKaW5kZXggY2E3MjY2NWVkOGEwZmY4 YTQwZWU0YzM1N2ZjYzEwZTZiY2Y1YWUzMi4uNGY3ZDNkODUxMDliNDE0Y2Q3NDA1MDQwZDVlMjJh Mzc4MDhhYTg4NCAxMDA2NDQKLS0tIGEvV2ViS2l0L2d0ay93ZWJraXQvd2Via2l0d2Vidmlldy5j cHAKKysrIGIvV2ViS2l0L2d0ay93ZWJraXQvd2Via2l0d2Vidmlldy5jcHAKQEAgLTM2Niw5ICsz NjYsNiBAQCBzdGF0aWMgZ2Jvb2xlYW4gd2Via2l0X3dlYl92aWV3X3BvcHVwX21lbnVfaGFuZGxl cihHdGtXaWRnZXQqIHdpZGdldCkKICAgICAgICAgbG9jYXRpb24gPSBJbnRQb2ludChyaWdodEFs aWduZWQgPyBmaXJzdFJlY3QucmlnaHQoKSA6IGZpcnN0UmVjdC54KCksIGZpcnN0UmVjdC5ib3R0 b20oKSk7CiAgICAgfQogCi0gICAgaW50IHgsIHk7Ci0gICAgZ2RrX3dpbmRvd19nZXRfb3JpZ2lu KEdUS19XSURHRVQodmlldy0+aG9zdFdpbmRvdygpLT5wbGF0Zm9ybVBhZ2VDbGllbnQoKSktPndp bmRvdywgJngsICZ5KTsKLQogICAgIC8vIEZJWE1FOiBUaGUgSW50U2l6ZSgwLCAtMSkgaXMgYSBo YWNrIHRvIGdldCB0aGUgaGl0LXRlc3RpbmcgdG8gcmVzdWx0IGluIHRoZSBzZWxlY3RlZCBlbGVt ZW50LgogICAgIC8vIElkZWFsbHkgd2UnZCBoYXZlIHRoZSBwb3NpdGlvbiBvZiBhIGNvbnRleHQg bWVudSBldmVudCBiZSBzZXBhcmF0ZSBmcm9tIGl0cyB0YXJnZXQgbm9kZS4KICAgICBsb2NhdGlv biA9IHZpZXctPmNvbnRlbnRzVG9XaW5kb3cobG9jYXRpb24pICsgSW50U2l6ZSgwLCAtMSk7CkBA IC0zODAsNyArMzc3LDcgQEAgc3RhdGljIGdib29sZWFuIHdlYmtpdF93ZWJfdmlld19wb3B1cF9t ZW51X2hhbmRsZXIoR3RrV2lkZ2V0KiB3aWRnZXQpCiAgICAgICAgIGxvY2F0aW9uLnNldFgoY29u dGV4dE1lbnVNYXJnaW4pOwogICAgIGVsc2UgaWYgKGxvY2F0aW9uLngoKSA+IHZpZXctPndpZHRo KCkpCiAgICAgICAgIGxvY2F0aW9uLnNldFgodmlldy0+d2lkdGgoKSAtIGNvbnRleHRNZW51TWFy Z2luKTsKLSAgICBJbnRQb2ludCBnbG9iYWwgPSBsb2NhdGlvbiArIEludFNpemUoeCwgeSk7Cisg ICAgSW50UG9pbnQgZ2xvYmFsID0gY2xpZW50UG9zaXRpb25Ub1Jvb3RXaW5kb3dQb3NpdGlvbih3 aWRnZXQtPndpbmRvdywgbG9jYXRpb24pOwogCiAgICAgUGxhdGZvcm1Nb3VzZUV2ZW50IGV2ZW50 KGxvY2F0aW9uLCBnbG9iYWwsIFJpZ2h0QnV0dG9uLCBNb3VzZUV2ZW50UHJlc3NlZCwgMCwgZmFs c2UsIGZhbHNlLCBmYWxzZSwgZmFsc2UsIGd0a19nZXRfY3VycmVudF9ldmVudF90aW1lKCkpOwog Cg==