Bug 218462 – REGRESSION(r261570): [GTK] Drag of unknown type aborted on motion above WebView under X11

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Under X11, drag an email in Evolution **through a web view** to the folders treeview sidebar.

Expected behavior: you can drag an email to the sidebar

Actual behavior: if your drag traverses the web view, the drag is cancelled

For some reason, under X11 we receive only two drag-motion events from GTK, both immediately after the drag begins. Then no more events until drag leave and drag drop. Whereas under Wayland, we receive a huge number of drag-motion events, once every time the cursor moves from one pixel to the next, as expected.

Complication: our drag-data-received signal handler is supposed to tell GTK whether the drag will be accepted or not by calling gdk_drag_status(). But this is impossible to do, since we have to pass the drag data to the web process, so we do it later in DropTarget::didPerformAction, even though that's not really permitted by the documentation of drag-data-received. That might be related.

Prior to this regression, we receive *no* drag-motion events, which is extremely confusing (WTF?). In particular, neither webkitWebViewBaseDragMotion nor DragAndDropHandler::dragMotion is ever called. I wondered if some other drag-motion handler was handling it first, but no drag-motion callback in evolution itself is called either. That's confusing, but it suggests this workaround:

diff --git a/Source/WebKit/UIProcess/API/gtk/DropTargetGtk3.cpp b/Source/WebKit/UIProcess/API/gtk/DropTargetGtk3.cpp
index 388dec07db2e..13013aee14d8 100644
--- a/Source/WebKit/UIProcess/API/gtk/DropTargetGtk3.cpp
+++ b/Source/WebKit/UIProcess/API/gtk/DropTargetGtk3.cpp
@@ -62,7 +62,7 @@ DropTarget::DropTarget(GtkWidget* webView)
             drop.accept(time);
         } else if (drop.m_drop == context)
             drop.update({ x, y }, time);
-        return TRUE;
+        return FALSE;
     }), this);
 
     g_signal_connect(m_webView, "drag-leave", G_CALLBACK(+[](GtkWidget*, GdkDragContext* context, guint time, gpointer userData) {

That means "reject all drags," so it's not a very good solution, but it does avoid this bug.

[Milan Crha]

The below backtrace is coming from the DropTarget constructor. If I understand it correctly, then the mouse cursor motion creates another WebView (because webkit asked for the selection data during the motion), for which Evolution prints the message. I do not think WebKit should ask the data during the drag operation, it should ask it only on the drop operation.

There is a comment in the sources:
    // WebCore needs the selection data to decide, so we need to preload the
    // data of targets we support. Once all data requests are done we start
    // notifying the web process about the DND events.

With respect of the notifications, you need to provide status of the drag, thus remove:

    if (operation == m_operation)
        return;

from void DropTarget::didPerformAction().

The backtrace of the second constructor:
   at _ZN6WebKit10DropTargetC2EP10_GtkWidget()
   by _ZL28webkitWebViewBaseConstructedP8_GObject()
   by _ZL24webkitWebViewConstructedP8_GObject()
   by web_view_constructed() at e-web-view.c:1524
   by mail_display_constructed() at e-mail-display.c:1614
   by g_object_new_with_custom_constructor() at gobject.c:2061
   by g_object_new_internal() at gobject.c:2089
   by g_object_new_valist() at gobject.c:2421
   by g_object_new() at gobject.c:1929
   by mail_printer_new_web_view() at e-mail-printer.c:341
   by e_mail_printer_print() at e-mail-printer.c:577
   by em_utils_print_messages_to_file() at em-utils.c:519
   by em_utils_selection_set_urilist() at em-utils.c:1085
   by ml_tree_drag_data_get() at message-list.c:2593
   by e_marshal_VOID__INT_POINTER_INT_OBJECT_BOXED_UINT_UINT() at e-marshal.c:1452
   by g_closure_invoke() at gclosure.c:815
   by signal_emit_unlocked_R() at gsignal.c:3747
   by g_signal_emit_valist() at gsignal.c:3498
   by g_signal_emit() at gsignal.c:3556
   by et_drag_data_get() at e-tree.c:2420
   by _gtk_marshal_VOID__OBJECT_BOXED_UINT_UINTv()
   by _g_closure_invoke_va() at gclosure.c:873
   by g_signal_emit_valist() at gsignal.c:3413
   by g_signal_emit_by_name() at gsignal.c:3594
   by gtk_drag_selection_get()
   by _gtk_marshal_VOID__BOXED_UINT_FLAGSv()
   by _g_closure_invoke_va() at gclosure.c:873
   by g_signal_emit_valist() at gsignal.c:3413
   by g_signal_emit_by_name() at gsignal.c:3594
   by gtk_selection_invoke_handler()
   by gtk_selection_convert()
   by _ZN6WebKit10DropTarget6acceptEj()
   by _ZZN6WebKit10DropTargetC4EP10_GtkWidgetENUlS2_P15_GdkDragContextiijPvE_4_FUNES2_S4_iijS5_()
   by _gtk_marshal_BOOLEAN__OBJECT_INT_INT_UINT()
   by g_closure_invoke() at gclosure.c:815
   by signal_emit_unlocked_R() at gsignal.c:3747
   by g_signal_emit_valist() at gsignal.c:3510
   by g_signal_emit_by_name() at gsignal.c:3594
   by gtk_drag_dest_motion()
   by _gtk_drag_dest_handle_event()
   by gtk_main_do_event()
   by _gdk_event_emit()
   by gdk_event_source_dispatch()
   by g_main_dispatch() at gmain.c:3322
   by g_main_context_dispatch() at gmain.c:3990
   by g_main_context_iterate() at gmain.c:4062
   by g_main_loop_run() at gmain.c:4253

[Michael Catanzaro]

(In reply to Milan Crha from comment #1)
> The below backtrace is coming from the DropTarget constructor. If I
> understand it correctly, then the mouse cursor motion creates another
> WebView (because webkit asked for the selection data during the motion),

OK wow, so Evolution needs to create a new webview to get the selection data (for whatever reason)? And we wind up with two different DropTargets both receiving the drop and interfering with each other...?

> for
> which Evolution prints the message. I do not think WebKit should ask the
> data during the drag operation, it should ask it only on the drop operation.

Are you sure? Requesting drag data early to decide whether to accept it is allowed by GTK. (What's not really allowed is our delayed async acceptance, which could be causing problems....)

> There is a comment in the sources:
>     // WebCore needs the selection data to decide, so we need to preload the
>     // data of targets we support. Once all data requests are done we start
>     // notifying the web process about the DND events.
> 
> With respect of the notifications, you need to provide status of the drag,
> thus remove:
> 
>     if (operation == m_operation)
>         return;
> 
> from void DropTarget::didPerformAction().

Hm, does that fix the bug? If so, then let's do it. Otherwise, I'm not so sure it's really needed to update the status of the drag to be the same as it was before? GTK expects us to do the update unconditionally, but it also expects us to do so before our drag-data-received callback returns, which we don't do.

This function should probably be renamed to avoid confusion. here would be good, because didPerformAction is not called where you would expect. It's called when the web process asyncronously responds to *every* drag operation (entered, updated, exited), not just the final action.

[Milan Crha]

(In reply to Milan Crha from comment #1)
> With respect of the notifications, you need to provide status of the drag,
> thus remove:
> 
>     if (operation == m_operation)
>         return;
> 
> from void DropTarget::didPerformAction().

This part is included in bug #218562 comment #1 .

[Milan Crha]

(In reply to Michael Catanzaro from comment #2)
> OK wow, so Evolution needs to create a new webview to get the selection data
> (for whatever reason)?

The drag out of the application exports the data into either mbox or pdf format. I have set the PDF. The save to PDF goes through print and it uses webkit for it. The print cannot be done with the existing web view, because it can show a different message/content (or nothing at all, when it's turned off), thus the print creates a new web view and draws the message there.

> And we wind up with two different DropTargets both
> receiving the drop and interfering with each other...?

They are two, yes, but I doubt they both receive the signals, because the new web view (for the print to PDF operation) is not visible, thus the mouse cannot move above it.

> Are you sure? Requesting drag data early to decide whether to accept it is
> allowed by GTK. (What's not really allowed is our delayed async acceptance,
> which could be causing problems....)

It's allowed, because the data should be available immediately, but they are not in this (and other) case(s). See for example [1] and the related [2].

> Hm, does that fix the bug?

Well, it definitely helped and is needed for gtk3, according to my tests. See bug #218562.

I changed the export format to be mbox (not pdf, thus the application doesn't freeze), and the changes from bug #218562 comment #1 do fix this bug for me.

[1] https://gitlab.gnome.org/GNOME/evolution/-/issues/440
[2] https://gitlab.gnome.org/GNOME/gtk/issues/1885

[Michael Catanzaro]

> I changed the export format to be mbox (not pdf, thus the application
> doesn't freeze), and the changes from bug #218562 comment #1 do fix this bug
> for me.

Er, OK, so if your patch in bug #218562 fixes this issue, then we don't need to do anything else here, right?

[Milan Crha]

(In reply to Michael Catanzaro from comment #5)
> Er, OK, so if your patch in bug #218562 fixes this issue, then we don't need
> to do anything else here, right?

Correct.

[Michael Catanzaro]

Fixed by r269505.