i3 - improved tiling WM

Reviews
Bugs

Return DRAG_ABORT on UnmapNotify from drag_pointer

Patch status: merged

Patch by Tony Crisci

Long description:

Add DRAG_ABORT to enum drag_result_t. DRAG_ABORT will indicate the drag
operation cannot be completed.

Return DRAG_ABORT on UnmapNotify, or when the keyboard or pointer cannot
be grabbed.

Add DRAGGING to return value for drag_result_t. DRAGGING is used
internally by drag_pointer to indicate the drag is in progress.

Change DRAG_CANCEL to DRAG_REVERT to clarify the distinction between
"abort" and "revert/cancel" actions.

Fixes an issue that caused i3 to crash when a user is dragging or
resizing a floating window that becomes destroyed.

To apply this patch, use:
curl http://cr.i3wm.org/patch/299/raw.patch | git am

b/include/floating.h

@@ -134,12 +134,27 @@ void floating_toggle_hide(xcb_connection_t *conn, Workspace *workspace);
 
 #endif
 /**
- * This is the return value of a drag operation like drag_pointer. DRAG_CANCEL
- * will indicate the intention of the drag should not be carried out, or that
- * the drag actions should be undone.
+ * This is the return value of a drag operation like drag_pointer.
+ *
+ * DRAGGING will indicate the drag action is still in progress and can be
+ * continued or resolved.
+ *
+ * DRAG_SUCCESS will indicate the intention of the drag action should be
+ * carried out.
+ *
+ * DRAG_REVERT will indicate an attempt should be made to restore the state of
+ * the involved windows to their condition before the drag.
+ *
+ * DRAG_ABORT will indicate that the intention of the drag action cannot be
+ * carried out (e.g. because the window has been unmapped).
  *
  */
-typedef enum { DRAG_SUCCESS = 0, DRAG_CANCEL } drag_result_t;
+typedef enum {
+    DRAGGING = 0,
+    DRAG_SUCCESS,
+    DRAG_REVERT,
+    DRAG_ABORT
+} drag_result_t;
 
 /**
  * This function grabs your pointer and keyboard and lets you drag stuff around

b/src/floating.c

@@ -441,14 +441,14 @@ void floating_drag_window(Con *con, const xcb_button_press_event_t *event) {
      * after the user releases the mouse button */
     tree_render();
 
-    /* Store the initial rect in case of user cancel */
+    /* Store the initial rect in case of user revert/cancel */
     Rect initial_rect = con->rect;
 
     /* Drag the window */
     drag_result_t drag_result = drag_pointer(con, event, XCB_NONE, BORDER_TOP /* irrelevant */, XCURSOR_CURSOR_MOVE, drag_window_callback, event);
 
     /* If the user cancelled, undo the changes. */
-    if (drag_result == DRAG_CANCEL)
+    if (drag_result == DRAG_REVERT)
         floating_reposition(con, initial_rect);
 
     /* If this is a scratchpad window, don't auto center it from now on. */
@@ -553,13 +553,13 @@ void floating_resize_window(Con *con, const bool proportional,
 
     struct resize_window_callback_params params = { corner, proportional, event };
 
-    /* get the initial rect in case of cancel */
+    /* get the initial rect in case of revert/cancel */
     Rect initial_rect = con->rect;
 
     drag_result_t drag_result = drag_pointer(con, event, XCB_NONE, BORDER_TOP /* irrelevant */, cursor, resize_window_callback, &params);
 
     /* If the user cancels, undo the resize */
-    if (drag_result == DRAG_CANCEL)
+    if (drag_result == DRAG_REVERT)
         floating_reposition(con, initial_rect);
 
     /* If this is a scratchpad window, don't auto center it from now on. */
@@ -601,7 +601,7 @@ drag_result_t drag_pointer(Con *con, const xcb_button_press_event_t *event, xcb_
 
     if ((reply = xcb_grab_pointer_reply(conn, cookie, NULL)) == NULL) {
         ELOG("Could not grab pointer\n");
-        return DRAG_CANCEL;
+        return DRAG_ABORT;
     }
 
     free(reply);
@@ -620,7 +620,7 @@ drag_result_t drag_pointer(Con *con, const xcb_button_press_event_t *event, xcb_
 
     if ((keyb_reply = xcb_grab_keyboard_reply(conn, keyb_cookie, NULL)) == NULL) {
         ELOG("Could not grab keyboard\n");
-        return DRAG_CANCEL;
+        return DRAG_ABORT;
     }
 
     free(keyb_reply);
@@ -629,11 +629,9 @@ drag_result_t drag_pointer(Con *con, const xcb_button_press_event_t *event, xcb_
     xcb_flush(conn);
 
     xcb_generic_event_t *inside_event, *last_motion_notify = NULL;
-    bool loop_done = false;
-    /* The return value, set to DRAG_CANCEL on user cancel */
-    drag_result_t drag_result = DRAG_SUCCESS;
+    drag_result_t drag_result = DRAGGING;
     /* I’ve always wanted to have my own eventhandler… */
-    while (!loop_done && (inside_event = xcb_wait_for_event(conn))) {
+    while (drag_result == DRAGGING && (inside_event = xcb_wait_for_event(conn))) {
         /* We now handle all events we can get using xcb_poll_for_event */
         do {
             /* skip x11 errors */
@@ -646,7 +644,7 @@ drag_result_t drag_pointer(Con *con, const xcb_button_press_event_t *event, xcb_
 
             switch (type) {
                 case XCB_BUTTON_RELEASE:
-                    loop_done = true;
+                    drag_result = DRAG_SUCCESS;
                     break;
 
                 case XCB_MOTION_NOTIFY:
@@ -657,16 +655,15 @@ drag_result_t drag_pointer(Con *con, const xcb_button_press_event_t *event, xcb_
 
                 case XCB_UNMAP_NOTIFY:
                     DLOG("Unmap-notify, aborting\n");
+                    drag_result = DRAG_ABORT;
+
                     handle_event(type, inside_event);
-                    loop_done = true;
-                    drag_result = DRAG_CANCEL;
                     break;
 
                 case XCB_KEY_PRESS:
                     /* Cancel the drag if a key was pressed */
-                    DLOG("A key was pressed during drag, canceling.");
-                    loop_done = true;
-                    drag_result = DRAG_CANCEL;
+                    DLOG("A key was pressed during drag, reverting changes.");
+                    drag_result = DRAG_REVERT;
 
                     handle_event(type, inside_event);
                     break;
@@ -681,7 +678,7 @@ drag_result_t drag_pointer(Con *con, const xcb_button_press_event_t *event, xcb_
                 free(inside_event);
         } while ((inside_event = xcb_poll_for_event(conn)) != NULL);
 
-        if (last_motion_notify == NULL || loop_done)
+        if (last_motion_notify == NULL || drag_result != DRAGGING)
             continue;
 
         new_x = ((xcb_motion_notify_event_t*)last_motion_notify)->root_x;

b/src/resize.c

@@ -161,7 +161,7 @@ int resize_graphical_handler(Con *first, Con *second, orientation_t orientation,
     xcb_flush(conn);
 
     /* User cancelled the drag so no action should be taken. */
-    if (drag_result == DRAG_CANCEL)
+    if (drag_result == DRAG_REVERT)
         return 0;
 
     int pixels;

65	@@ -441,14 +441,14 @@ void floating_drag_window(Con con, const xcb_button_press_event_t event) {
66	* after the user releases the mouse button */
67	tree_render();
68
69	- /* Store the initial rect in case of user cancel */
70	+ /* Store the initial rect in case of user revert/cancel */
71	Rect initial_rect = con->rect;
72
73	/* Drag the window */
74	drag_result_t drag_result = drag_pointer(con, event, XCB_NONE, BORDER_TOP /* irrelevant */, XCURSOR_CURSOR_MOVE, drag_window_callback, event);
75
76	/* If the user cancelled, undo the changes. */
77	- if (drag_result == DRAG_CANCEL)
78	+ if (drag_result == DRAG_REVERT)
79	floating_reposition(con, initial_rect);
80
81	/* If this is a scratchpad window, don't auto center it from now on. */
82	@@ -553,13 +553,13 @@ void floating_resize_window(Con *con, const bool proportional,
83
84	struct resize_window_callback_params params = { corner, proportional, event };
85
86	- /* get the initial rect in case of cancel */
87	+ /* get the initial rect in case of revert/cancel */
88	Rect initial_rect = con->rect;
89
90	drag_result_t drag_result = drag_pointer(con, event, XCB_NONE, BORDER_TOP /* irrelevant */, cursor, resize_window_callback, &params);
91
92	/* If the user cancels, undo the resize */
93	- if (drag_result == DRAG_CANCEL)
94	+ if (drag_result == DRAG_REVERT)
95	floating_reposition(con, initial_rect);
96
97	/* If this is a scratchpad window, don't auto center it from now on. */
98	@@ -601,7 +601,7 @@ drag_result_t drag_pointer(Con con, const xcb_button_press_event_t event, xcb_
99
100	if ((reply = xcb_grab_pointer_reply(conn, cookie, NULL)) == NULL) {
101	ELOG("Could not grab pointer\n");
102	- return DRAG_CANCEL;
103	+ return DRAG_ABORT;
104	}
105
106	free(reply);
107	@@ -620,7 +620,7 @@ drag_result_t drag_pointer(Con con, const xcb_button_press_event_t event, xcb_
108
109	if ((keyb_reply = xcb_grab_keyboard_reply(conn, keyb_cookie, NULL)) == NULL) {
110	ELOG("Could not grab keyboard\n");
111	- return DRAG_CANCEL;
112	+ return DRAG_ABORT;
113	}
114
115	free(keyb_reply);
116	@@ -629,11 +629,9 @@ drag_result_t drag_pointer(Con con, const xcb_button_press_event_t event, xcb_
117	xcb_flush(conn);
118
119	xcb_generic_event_t inside_event, last_motion_notify = NULL;
120	- bool loop_done = false;
121	- /* The return value, set to DRAG_CANCEL on user cancel */
122	- drag_result_t drag_result = DRAG_SUCCESS;
123	+ drag_result_t drag_result = DRAGGING;
124	/* I’ve always wanted to have my own eventhandler… */
125	- while (!loop_done && (inside_event = xcb_wait_for_event(conn))) {
126	+ while (drag_result == DRAGGING && (inside_event = xcb_wait_for_event(conn))) {
127	/* We now handle all events we can get using xcb_poll_for_event */
128	do {
129	/* skip x11 errors */
130	@@ -646,7 +644,7 @@ drag_result_t drag_pointer(Con con, const xcb_button_press_event_t event, xcb_
131
132	switch (type) {
133	case XCB_BUTTON_RELEASE:
134	- loop_done = true;
135	+ drag_result = DRAG_SUCCESS;
136	break;
137
138	case XCB_MOTION_NOTIFY:
139	@@ -657,16 +655,15 @@ drag_result_t drag_pointer(Con con, const xcb_button_press_event_t event, xcb_
140
141	case XCB_UNMAP_NOTIFY:
142	DLOG("Unmap-notify, aborting\n");
143	+ drag_result = DRAG_ABORT;
144	+
145	handle_event(type, inside_event);
146	- loop_done = true;
147	- drag_result = DRAG_CANCEL;
148	break;
149
150	case XCB_KEY_PRESS:
151	/* Cancel the drag if a key was pressed */
152	- DLOG("A key was pressed during drag, canceling.");
153	- loop_done = true;
154	- drag_result = DRAG_CANCEL;
155	+ DLOG("A key was pressed during drag, reverting changes.");
156	+ drag_result = DRAG_REVERT;
157
158	handle_event(type, inside_event);
159	break;
160	@@ -681,7 +678,7 @@ drag_result_t drag_pointer(Con con, const xcb_button_press_event_t event, xcb_
161	free(inside_event);
162	} while ((inside_event = xcb_poll_for_event(conn)) != NULL);
163
164	- if (last_motion_notify == NULL \|\| loop_done)
165	+ if (last_motion_notify == NULL \|\| drag_result != DRAGGING)
166	continue;
167
168	new_x = ((xcb_motion_notify_event_t*)last_motion_notify)->root_x;

173	@@ -161,7 +161,7 @@ int resize_graphical_handler(Con first, Con second, orientation_t orientation,
174	xcb_flush(conn);
175
176	/* User cancelled the drag so no action should be taken. */
177	- if (drag_result == DRAG_CANCEL)
178	+ if (drag_result == DRAG_REVERT)
179	return 0;
180
181	int pixels;

29	@@ -134,12 +134,27 @@ void floating_toggle_hide(xcb_connection_t conn, Workspace workspace);
30
31	#endif
32	/**
33	- * This is the return value of a drag operation like drag_pointer. DRAG_CANCEL
34	- * will indicate the intention of the drag should not be carried out, or that
35	- * the drag actions should be undone.
36	+ * This is the return value of a drag operation like drag_pointer.
37	+ *
38	+ * DRAGGING will indicate the drag action is still in progress and can be
39	+ * continued or resolved.
40	+ *
41	+ * DRAG_SUCCESS will indicate the intention of the drag action should be
42	+ * carried out.
43	+ *
44	+ * DRAG_REVERT will indicate an attempt should be made to restore the state of
45	+ * the involved windows to their condition before the drag.
46	+ *
47	+ * DRAG_ABORT will indicate that the intention of the drag action cannot be
48	+ * carried out (e.g. because the window has been unmapped).
49	*
50	*/
51	-typedef enum { DRAG_SUCCESS = 0, DRAG_CANCEL } drag_result_t;
52	+typedef enum {
53	+ DRAGGING = 0,
54	+ DRAG_SUCCESS,
55	+ DRAG_REVERT,
56	+ DRAG_ABORT
57	+} drag_result_t;
58
59	/**
60	* This function grabs your pointer and keyboard and lets you drag stuff around