After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 669208 - x11: Cancel _NET_WM_MOVERESIZE if we get a matching ButtonRelease
x11: Cancel _NET_WM_MOVERESIZE if we get a matching ButtonRelease
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2012-02-01 22:43 UTC by Rui Matos
Modified: 2012-02-09 22:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
x11: Cancel _NET_WM_MOVERESIZE if we get a matching ButtonRelease (7.92 KB, patch)
2012-02-01 22:43 UTC, Rui Matos
reviewed Details | Review
x11: Cancel _NET_WM_MOVERESIZE if we get a matching ButtonRelease (7.82 KB, patch)
2012-02-08 23:54 UTC, Rui Matos
committed Details | Review

Description Rui Matos 2012-02-01 22:43:19 UTC
This implements the following part of the EWMH spec:

"The special value _NET_WM_MOVERESIZE_CANCEL also allows clients to cancel the
operation by sending such message if they detect the release themselves
(clients should send it if they get the button release after sending the move
resize message, indicating that the WM did not get a grab in time to get the
release)."

In particular, it fixes the case of clicking widgets that use
gdk_window_begin_[resize|move]_drag*() and the click "sticking", i.e. the
mouse button getting released but the resize or move operation remaining in
effect.
Comment 1 Rui Matos 2012-02-01 22:43:21 UTC
Created attachment 206598 [details] [review]
x11: Cancel _NET_WM_MOVERESIZE if we get a matching ButtonRelease
Comment 2 Matthias Clasen 2012-02-05 23:57:03 UTC
Review of attachment 206598 [details] [review]:

Do you have any reproduction instructions for the click 'sticking' ?

The patch is a little hard to follow due to all the code motion.

::: gdk/x11/gdkwindow-x11.c
@@ +4057,2 @@
 {
+  g_object_set_qdata (G_OBJECT (display), wmspec_button_quark (), GINT_TO_POINTER (button));

Why use object data here instead of just adding this to the GdkDisplayX11 struct ?
Comment 3 Rui Matos 2012-02-08 23:54:26 UTC
Created attachment 207155 [details] [review]
x11: Cancel _NET_WM_MOVERESIZE if we get a matching ButtonRelease

--
(In reply to comment #2)
> Do you have any reproduction instructions for the click 'sticking' ?

It's easy for me to trigger when clicking on the toolbar of a partially
visible backdrop widget-factory, the intention being to just raise the
window. It usually reproduces after 2-3 times. I think the race is more likely
to happen then because of all the time gtk+ spends redrawing all the widgets.

> The patch is a little hard to follow due to all the code motion.

Yeah, I moved down struct MoveResizeData and up the wmspec defines in order to
separate the code which handles the wpspec move/resize from the code which
does the client side emulation.

> ::: gdk/x11/gdkwindow-x11.c
> @@ +4057,2 @@
>  {
> +  g_object_set_qdata (G_OBJECT (display), wmspec_button_quark (),
> GINT_TO_POINTER (button));
>
> Why use object data here instead of just adding this to the GdkDisplayX11
> struct ?

Mostly to be consistent with how the move/resize emulation code does it but
yes, it's pointless for so little data. Patch amended.
Comment 4 Matthias Clasen 2012-02-09 14:15:04 UTC
Review of attachment 207155 [details] [review]:

Anyway, looks good to me now.

::: gdk/x11/gdkwindow-x11.c
@@ +4040,3 @@
+                     gint        root_x,
+                     gint        root_y,
+                     gint        direction,

'direction' is a bit of a misnomer here now, isn't it ? 
How about 'message' ?
Comment 5 Rui Matos 2012-02-09 22:49:21 UTC
(In reply to comment #4)
> ::: gdk/x11/gdkwindow-x11.c
> @@ +4040,3 @@
> +                     gint        root_x,
> +                     gint        root_y,
> +                     gint        direction,
>
> 'direction' is a bit of a misnomer here now, isn't it ?
> How about 'message' ?

Yup but the message here is the whole tuple really. I went with 'action' which
is what mutter uses for the corresponding variable on the WM side.

Attachment 207155 [details] pushed as db2eb85 - x11: Cancel _NET_WM_MOVERESIZE if we get a matching ButtonRelease