GNOME Bugzilla – Bug 761954
Tab detach no longer works
Last modified: 2016-02-15 18:09:01 UTC
I guess this is a regression of recent DND changes. I've only tried with ephy, but the problem is that when a tab drag operation fails, the create-window signal is not emitted, because the drag operation didn't fail with GTK_DRAG_RESULT_NO_TARGET but it was cancelled by gdk_dnd_handle_button_event in gdkdnd-x11.c
Created attachment 321268 [details] [review] gdk: Add GdkDragCancelReason enum as argument to GdkDragContext::cancel We should conform to a minimal set of reasons for the gtk side to emit a better GtkDragResult than GTK_DRAG_RESULT_ERROR. This fixes the notebook tab DnD feature, where we rely on GTK_DRAG_RESULT_NO_TARGET. In the wayland side, unfortunately we can't honor either NO_TARGET nor USER_CANCELLED, we don't know of the latter, so we could return false positives on the former.
Created attachment 321269 [details] [review] gdkdnd: Make GtkDragContext::cancel RUN_LAST The default implementation code should act as a catch-all fallback, we let the connected handlers to run first then.
Created attachment 321270 [details] [review] x11: Stick to the first gdk_drag_drop_done() result That way we can let ::cancel callers to override the visual result of the operation (eg. when detaching notebook tabs on NO_TARGET), despite our catch-all gdk_drag_drop_done() call in gdk_x11_drag_context_cancel().
Review of attachment 321268 [details] [review]: Looks good to me
Review of attachment 321269 [details] [review]: makes sense
Review of attachment 321270 [details] [review]: The alternative would be to make ::cancel return a boolean with TRUE-means-handled semantics
(In reply to Matthias Clasen from comment #6) > Review of attachment 321270 [details] [review] [review]: > > The alternative would be to make ::cancel return a boolean with > TRUE-means-handled semantics Hmm, maybe... but we should find some other place for calling drag_context_ungrab() than gdk_x11_drag_context_cancel() if we add a g_signal_accumulator_true_handled() accumulator.
Attachment 321268 [details] pushed as aad3135 - gdk: Add GdkDragCancelReason enum as argument to GdkDragContext::cancel Attachment 321269 [details] pushed as 4636552 - gdkdnd: Make GtkDragContext::cancel RUN_LAST
Review of attachment 321270 [details] [review]: On second thought, making gdk_drag_drop_done() one-shot is perfectly reasonable. Might be worth mentioning that in the docs, though. Also: you could just do this in the frontend code, so it is same for all backends without further work
Pushed to the common code as suggested, and with docs blurb. The following fix has been pushed: 38d0d0a gdkdnd: Stick to the first gdk_drag_drop_done() result
Created attachment 321283 [details] [review] gdkdnd: Stick to the first gdk_drag_drop_done() result That way we can let ::cancel callers to override the visual result of the operation (eg. when detaching notebook tabs on NO_TARGET). Also, document gdk_drag_drop_done() so it is mentioned that this is a one-shot call.