Bug 39240 - [GTK] gtk_drag_begin should be initiated with a motion event
Summary: [GTK] gtk_drag_begin should be initiated with a motion event
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 30623
  Show dependency treegraph
 
Reported: 2010-05-17 14:47 PDT by Martin Robinson
Modified: 2010-06-14 17:16 PDT (History)
0 users

See Also:


Attachments
Patch (5.23 KB, patch)
2010-05-17 14:51 PDT, Martin Robinson
xan.lopez: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2010-05-17 14:47:54 PDT
This is necessary so that the internal drag context will be in the proper state for simulating dropping events in the DRT.
Comment 1 Martin Robinson 2010-05-17 14:51:06 PDT
Created attachment 56273 [details]
Patch
Comment 2 Xan Lopez 2010-05-22 01:55:22 PDT
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 @@
>+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.
Comment 3 Martin Robinson 2010-05-22 12:15:20 PDT
(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.
Comment 4 Martin Robinson 2010-06-14 17:16:20 PDT
This change should no longer be necessary after some other fixes to the DRT.