[GTK] gtk_drag_begin should be initiated with a motion event

• Component: WebKitGTK
• Status: RESOLVED INVALID

[Martin Robinson]

This is necessary so that the internal drag context will be in the proper state for simulating dropping events in the DRT.

[Martin Robinson]

Created attachment 56273 [details]
Patch

[Xan Lopez]

Comment on attachment 56273 [details]
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 @@
>[phone]  Martin Robinson  <[e-mail]>
>+
>+        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.

[Martin Robinson]

(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.

[Martin Robinson]

This change should no longer be necessary after some other fixes to the DRT.