Summary: | [GTK][Regression][2.30] Application cannot override drag&drop callbacks | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Milan Crha <mcrha> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | amarok, berto, bugs-noreply, cgarcia, ews-watchlist, gustavo, ht990332, mcatanzaro, nekohayo | ||||
Priority: | P2 | ||||||
Version: | Other | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Milan Crha
2020-11-04 06:08:27 PST
Created attachment 413169 [details]
proposed patch
This fixes it for me. Three changes are included, all are needed:
1) use g_signal_connect_after(), thus the descendants can get before that callback, as it used to be when the d&d callbacks had been assigned as GtkWidgetClass members;
2) update the inner m_drop context whenever it's different from the passed-in context in the "drag-motion" handler, otherwise one cannot drag the second time, because the "drag-leave" was not called (it is not called here);
3) in the DropTarget::didPerformAction() always call gdk_drag_status(), otherwise the drag can look like being frozen.
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API (In reply to Milan Crha from comment #1) > 1) use g_signal_connect_after(), thus the descendants can get before that > callback, as it used to be when the d&d callbacks had been assigned as > GtkWidgetClass members; Good. > 2) update the inner m_drop context whenever it's different from the > passed-in context in the "drag-motion" handler, otherwise one cannot drag > the second time, because the "drag-leave" was not called (it is not called > here); Odd... seems like drag-leave ought to be called? > 3) in the DropTarget::didPerformAction() always call gdk_drag_status(), > otherwise the drag can look like being frozen. OK, good. (In reply to Michael Catanzaro from comment #3) > Odd... seems like drag-leave ought to be called? Right, it should be called, according to the gtk+ documentation, but it's not called for some reason. I didn't try to figure out why. In any case, the system can have "running" only a single drag operation in the time, thus the idea of always updating the m_drop when it doesn't match the current context makes sense. Comment on attachment 413169 [details] proposed patch OK, all this makes sense to me. I'm not giving cq+ yet, though, because Carlos Garcia will want to review this before it lands, so let's allow some time for that. If we commit this, we should also remember to close bug #218462. Comment on attachment 413169 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=413169&action=review > Source/WebKit/UIProcess/API/gtk/DropTargetGtk3.cpp:59 > - g_signal_connect(m_webView, "drag-motion", G_CALLBACK(+[](GtkWidget*, GdkDragContext* context, gint x, gint y, guint time, gpointer userData) -> gboolean { > + g_signal_connect_after(m_webView, "drag-motion", G_CALLBACK(+[](GtkWidget*, GdkDragContext* context, gint x, gint y, guint time, gpointer userData) -> gboolean { I guess this is not needed in GTK4, because apps wanting to override this can use the capture phase on GtkDropTargetAsync? *** Bug 218163 has been marked as a duplicate of this bug. *** Committed r269505: <https://trac.webkit.org/changeset/269505> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413169 [details]. (In reply to Carlos Garcia Campos from comment #6) > I guess this is not needed in GTK4, because apps wanting to override this > can use the capture phase on GtkDropTargetAsync? I do not know gtk4 at all, but my quick check on what WebKit does there didn't show anything relevant (the signals being attached to are very different from those in gtk3). *** Bug 216828 has been marked as a duplicate of this bug. *** |