If the compositor (rootston) handles the escape-key (either by
canceling a keyboard/pointer grab like a popup, or canceling a
move/resize/rotate) the user probably doesn't desire that the
escape-key is propagated to any focust client window.
---

Notes:

  This is kind of a strawman patch. For example, it might be better
  for the called methods to return true/false if they had any effect.
  And, perhaps the original behavior was intentional... But I'm
  curious to see what people think.

  Here are some problematic cases that this patch fixes:

  [Case A]
    0. Open gnome-terminal.
    1. Right-click to open popup.
    2. Press Esc [should close popup].
    3. Type a letter.
    4. Would expect that the letter is input. But instead, the key
       combination <escape>+<letter> was input, which you can see more
       clearly by starting cat(1) in gnome-terminal in step 0.

  [Case B]
    0. Open VMPK.
    1. Click "Help" -> "About Translation".
    2. Right-click in the child pop-up to open a grandchild popup.
    3. Press Esc.
    4. Would expect that just the grandchild popup is closed. But
       instead, the child popup is closed as well.

  [Case C]
    Same as "Case A", but replace step-1 with "Start to drag the
    window somewhere". In this case, step-2 (press Esc), will cancel
    the drag. Unfortunately, it also sends the Esc-key to
    gnome-terminal again.

 rootston/keyboard.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/rootston/keyboard.c b/rootston/keyboard.c
index b5a8093b..5e5a0113 100644
--- a/rootston/keyboard.c
+++ b/rootston/keyboard.c
@@ -205,9 +205,17 @@ static bool keyboard_execute_compositor_binding(struct roots_keyboard *keyboard,
 	}

 	if (keysym == XKB_KEY_Escape) {
-		wlr_seat_pointer_end_grab(keyboard->seat->seat);
-		wlr_seat_keyboard_end_grab(keyboard->seat->seat);
+		struct wlr_seat *wlr_seat = keyboard->seat->seat;
+		bool handled_escape =
+			wlr_seat->pointer_state.grab != wlr_seat->pointer_state.default_grab ||
+			wlr_seat->keyboard_state.grab != wlr_seat->keyboard_state.default_grab ||
+			keyboard->seat->cursor->mode != ROOTS_CURSOR_PASSTHROUGH;
+		wlr_seat_pointer_end_grab(wlr_seat);
+		wlr_seat_keyboard_end_grab(wlr_seat);
 		roots_seat_end_compositor_grab(keyboard->seat);
+		if (handled_escape) {
+			return true;
+		}
 	}

 	return false;
--
2.18.0

On 2018-09-06  8:08 PM, Genki Sky wrote:
>   This is kind of a strawman patch. For example, it might be better
>   for the called methods to return true/false if they had any effect.

Yes

If the compositor (rootston) handles the escape-key (either by canceling
a keyboard/pointer grab like a popup, or canceling the motion of a
move/resize) the user probably doesn't desire that the escape-key is
propagated to any focust client window.
---

Changes from v1:

  Change the relevant methods to return a bool based on whether they
  had any effect, and use that to determine if the Escape key was
  handled.

 include/rootston/seat.h        |  2 +-
 include/wlr/types/wlr_seat.h   |  8 ++++----
 rootston/keyboard.c            | 10 +++++++---
 rootston/seat.c                |  7 +++++--
 types/seat/wlr_seat_keyboard.c |  4 +++-
 types/seat/wlr_seat_pointer.c  |  4 +++-
 6 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/include/rootston/seat.h b/include/rootston/seat.h
index 3ddb97c5..6706a127 100644
--- a/include/rootston/seat.h
+++ b/include/rootston/seat.h
@@ -151,7 +151,7 @@ void roots_seat_begin_resize(struct roots_seat *seat, struct roots_view *view,

 void roots_seat_begin_rotate(struct roots_seat *seat, struct roots_view *view);

-void roots_seat_end_compositor_grab(struct roots_seat *seat);
+bool roots_seat_end_compositor_grab(struct roots_seat *seat);

 struct roots_seat_view *roots_seat_view_from_view( struct roots_seat *seat,
 	struct roots_view *view);
diff --git a/include/wlr/types/wlr_seat.h b/include/wlr/types/wlr_seat.h
index b3c02cf2..10ff4a65 100644
--- a/include/wlr/types/wlr_seat.h
+++ b/include/wlr/types/wlr_seat.h
@@ -321,9 +321,9 @@ void wlr_seat_pointer_start_grab(struct wlr_seat *wlr_seat,

 /**
  * End the grab of the pointer of this seat. This reverts the grab back to the
- * default grab for the pointer.
+ * default grab for the pointer. Return true iff it had any effect.
  */
-void wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat);
+bool wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat);

 /**
  * Notify the seat of a pointer enter event to the given surface and request it
@@ -378,9 +378,9 @@ void wlr_seat_keyboard_start_grab(struct wlr_seat *wlr_seat,

 /**
  * End the grab of the keyboard of this seat. This reverts the grab back to the
- * default grab for the keyboard.
+ * default grab for the keyboard. Return true iff it had any effect.
  */
-void wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat);
+bool wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat);

 /**
  * Send the keyboard key to focused keyboard resources. Compositors should use
diff --git a/rootston/keyboard.c b/rootston/keyboard.c
index b5a8093b..5a158eb9 100644
--- a/rootston/keyboard.c
+++ b/rootston/keyboard.c
@@ -205,9 +205,13 @@ static bool keyboard_execute_compositor_binding(struct roots_keyboard *keyboard,
 	}

 	if (keysym == XKB_KEY_Escape) {
-		wlr_seat_pointer_end_grab(keyboard->seat->seat);
-		wlr_seat_keyboard_end_grab(keyboard->seat->seat);
-		roots_seat_end_compositor_grab(keyboard->seat);
+		struct wlr_seat *wlr_seat = keyboard->seat->seat;
+		bool handled_escape = wlr_seat_pointer_end_grab(wlr_seat);
+		handled_escape |= wlr_seat_keyboard_end_grab(wlr_seat);
+		handled_escape |= roots_seat_end_compositor_grab(keyboard->seat);
+		if (handled_escape) {
+			return true;
+		}
 	}

 	return false;
diff --git a/rootston/seat.c b/rootston/seat.c
index c11ff017..73962a0b 100644
--- a/rootston/seat.c
+++ b/rootston/seat.c
@@ -1342,14 +1342,15 @@ void roots_seat_begin_rotate(struct roots_seat *seat, struct roots_view *view) {
 		ROOTS_XCURSOR_ROTATE, seat->cursor->cursor);
 }

-void roots_seat_end_compositor_grab(struct roots_seat *seat) {
+bool roots_seat_end_compositor_grab(struct roots_seat *seat) {
 	struct roots_cursor *cursor = seat->cursor;
 	struct roots_view *view = roots_seat_get_focus(seat);

 	if (view == NULL) {
-		return;
+		return false;
 	}

+	bool had_effect = true;
 	switch(cursor->mode) {
 		case ROOTS_CURSOR_MOVE:
 			view_move(view, cursor->view_x, cursor->view_y);
@@ -1361,10 +1362,12 @@ void roots_seat_end_compositor_grab(struct roots_seat *seat) {
 			view->rotation = cursor->view_rotation;
 			break;
 		case ROOTS_CURSOR_PASSTHROUGH:
+			had_effect = false;
 			break;
 	}

 	cursor->mode = ROOTS_CURSOR_PASSTHROUGH;
+	return had_effect;
 }

 struct roots_seat *input_last_active_seat(struct roots_input *input) {
diff --git a/types/seat/wlr_seat_keyboard.c b/types/seat/wlr_seat_keyboard.c
index e8ea300e..f8a28e2c 100644
--- a/types/seat/wlr_seat_keyboard.c
+++ b/types/seat/wlr_seat_keyboard.c
@@ -169,7 +169,7 @@ void wlr_seat_keyboard_start_grab(struct wlr_seat *wlr_seat,
 	wlr_signal_emit_safe(&wlr_seat->events.keyboard_grab_begin, grab);
 }

-void wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat) {
+bool wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat) {
 	struct wlr_seat_keyboard_grab *grab = wlr_seat->keyboard_state.grab;

 	if (grab != wlr_seat->keyboard_state.default_grab) {
@@ -177,8 +177,10 @@ void wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat) {
 		wlr_signal_emit_safe(&wlr_seat->events.keyboard_grab_end, grab);
 		if (grab->interface->cancel) {
 			grab->interface->cancel(grab);
+			return true;
 		}
 	}
+	return false;
 }

 static void seat_keyboard_handle_surface_destroy(struct wl_listener *listener,
diff --git a/types/seat/wlr_seat_pointer.c b/types/seat/wlr_seat_pointer.c
index 899fb64f..93412b97 100644
--- a/types/seat/wlr_seat_pointer.c
+++ b/types/seat/wlr_seat_pointer.c
@@ -273,15 +273,17 @@ void wlr_seat_pointer_start_grab(struct wlr_seat *wlr_seat,
 	wlr_signal_emit_safe(&wlr_seat->events.pointer_grab_begin, grab);
 }

-void wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat) {
+bool wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat) {
 	struct wlr_seat_pointer_grab *grab = wlr_seat->pointer_state.grab;
 	if (grab != wlr_seat->pointer_state.default_grab) {
 		wlr_seat->pointer_state.grab = wlr_seat->pointer_state.default_grab;
 		wlr_signal_emit_safe(&wlr_seat->events.pointer_grab_end, grab);
 		if (grab->interface->cancel) {
 			grab->interface->cancel(grab);
+			return true;
 		}
 	}
+	return false;
 }

 void wlr_seat_pointer_notify_enter(struct wlr_seat *wlr_seat,
--
2.18.0

On 2018-09-08  3:16 PM, Genki Sky wrote:
> --- a/include/rootston/seat.h
> +++ b/include/rootston/seat.h
> @@ -378,9 +378,9 @@ void wlr_seat_keyboard_start_grab(struct wlr_seat *wlr_seat,
>  /**
>   * End the grab of the keyboard of this seat. This reverts the grab back to the
> - * default grab for the keyboard.
> + * default grab for the keyboard. Return true iff it had any effect.
>   */

"iff"

If the compositor (rootston) handles the escape-key (either by canceling
a keyboard/pointer grab like a popup, or canceling the motion of a
move/resize) the user probably doesn't desire that the escape-key is
propagated to any focust client window.
---

Changes from v2:

  - Change the return-true case in *_end_grab() to be unconditional on
    whether the non-default grab had custom cancel() logic.
  - "iff" -> "if". I was using the abbreviation of "if and only if",
    but I guess it's unnecessary.

 include/rootston/seat.h        |  2 +-
 include/wlr/types/wlr_seat.h   |  8 ++++----
 rootston/keyboard.c            | 10 +++++++---
 rootston/seat.c                |  7 +++++--
 types/seat/wlr_seat_keyboard.c |  4 +++-
 types/seat/wlr_seat_pointer.c  |  4 +++-
 6 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/include/rootston/seat.h b/include/rootston/seat.h
index 3ddb97c5..6706a127 100644
--- a/include/rootston/seat.h
+++ b/include/rootston/seat.h
@@ -151,7 +151,7 @@ void roots_seat_begin_resize(struct roots_seat *seat, struct roots_view *view,

 void roots_seat_begin_rotate(struct roots_seat *seat, struct roots_view *view);

-void roots_seat_end_compositor_grab(struct roots_seat *seat);
+bool roots_seat_end_compositor_grab(struct roots_seat *seat);

 struct roots_seat_view *roots_seat_view_from_view( struct roots_seat *seat,
 	struct roots_view *view);
diff --git a/include/wlr/types/wlr_seat.h b/include/wlr/types/wlr_seat.h
index b3c02cf2..4ddb6334 100644
--- a/include/wlr/types/wlr_seat.h
+++ b/include/wlr/types/wlr_seat.h
@@ -321,9 +321,9 @@ void wlr_seat_pointer_start_grab(struct wlr_seat *wlr_seat,

 /**
  * End the grab of the pointer of this seat. This reverts the grab back to the
- * default grab for the pointer.
+ * default grab for the pointer. Return true if it had any effect.
  */
-void wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat);
+bool wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat);

 /**
  * Notify the seat of a pointer enter event to the given surface and request it
@@ -378,9 +378,9 @@ void wlr_seat_keyboard_start_grab(struct wlr_seat *wlr_seat,

 /**
  * End the grab of the keyboard of this seat. This reverts the grab back to the
- * default grab for the keyboard.
+ * default grab for the keyboard. Return true if it had any effect.
  */
-void wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat);
+bool wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat);

 /**
  * Send the keyboard key to focused keyboard resources. Compositors should use
diff --git a/rootston/keyboard.c b/rootston/keyboard.c
index b5a8093b..5a158eb9 100644
--- a/rootston/keyboard.c
+++ b/rootston/keyboard.c
@@ -205,9 +205,13 @@ static bool keyboard_execute_compositor_binding(struct roots_keyboard *keyboard,
 	}

 	if (keysym == XKB_KEY_Escape) {
-		wlr_seat_pointer_end_grab(keyboard->seat->seat);
-		wlr_seat_keyboard_end_grab(keyboard->seat->seat);
-		roots_seat_end_compositor_grab(keyboard->seat);
+		struct wlr_seat *wlr_seat = keyboard->seat->seat;
+		bool handled_escape = wlr_seat_pointer_end_grab(wlr_seat);
+		handled_escape |= wlr_seat_keyboard_end_grab(wlr_seat);
+		handled_escape |= roots_seat_end_compositor_grab(keyboard->seat);
+		if (handled_escape) {
+			return true;
+		}
 	}

 	return false;
diff --git a/rootston/seat.c b/rootston/seat.c
index c11ff017..73962a0b 100644
--- a/rootston/seat.c
+++ b/rootston/seat.c
@@ -1342,14 +1342,15 @@ void roots_seat_begin_rotate(struct roots_seat *seat, struct roots_view *view) {
 		ROOTS_XCURSOR_ROTATE, seat->cursor->cursor);
 }

-void roots_seat_end_compositor_grab(struct roots_seat *seat) {
+bool roots_seat_end_compositor_grab(struct roots_seat *seat) {
 	struct roots_cursor *cursor = seat->cursor;
 	struct roots_view *view = roots_seat_get_focus(seat);

 	if (view == NULL) {
-		return;
+		return false;
 	}

+	bool had_effect = true;
 	switch(cursor->mode) {
 		case ROOTS_CURSOR_MOVE:
 			view_move(view, cursor->view_x, cursor->view_y);
@@ -1361,10 +1362,12 @@ void roots_seat_end_compositor_grab(struct roots_seat *seat) {
 			view->rotation = cursor->view_rotation;
 			break;
 		case ROOTS_CURSOR_PASSTHROUGH:
+			had_effect = false;
 			break;
 	}

 	cursor->mode = ROOTS_CURSOR_PASSTHROUGH;
+	return had_effect;
 }

 struct roots_seat *input_last_active_seat(struct roots_input *input) {
diff --git a/types/seat/wlr_seat_keyboard.c b/types/seat/wlr_seat_keyboard.c
index e8ea300e..435a7fc7 100644
--- a/types/seat/wlr_seat_keyboard.c
+++ b/types/seat/wlr_seat_keyboard.c
@@ -169,7 +169,7 @@ void wlr_seat_keyboard_start_grab(struct wlr_seat *wlr_seat,
 	wlr_signal_emit_safe(&wlr_seat->events.keyboard_grab_begin, grab);
 }

-void wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat) {
+bool wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat) {
 	struct wlr_seat_keyboard_grab *grab = wlr_seat->keyboard_state.grab;

 	if (grab != wlr_seat->keyboard_state.default_grab) {
@@ -178,7 +178,9 @@ void wlr_seat_keyboard_end_grab(struct wlr_seat *wlr_seat) {
 		if (grab->interface->cancel) {
 			grab->interface->cancel(grab);
 		}
+		return true;
 	}
+	return false;
 }

 static void seat_keyboard_handle_surface_destroy(struct wl_listener *listener,
diff --git a/types/seat/wlr_seat_pointer.c b/types/seat/wlr_seat_pointer.c
index 899fb64f..9680becc 100644
--- a/types/seat/wlr_seat_pointer.c
+++ b/types/seat/wlr_seat_pointer.c
@@ -273,7 +273,7 @@ void wlr_seat_pointer_start_grab(struct wlr_seat *wlr_seat,
 	wlr_signal_emit_safe(&wlr_seat->events.pointer_grab_begin, grab);
 }

-void wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat) {
+bool wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat) {
 	struct wlr_seat_pointer_grab *grab = wlr_seat->pointer_state.grab;
 	if (grab != wlr_seat->pointer_state.default_grab) {
 		wlr_seat->pointer_state.grab = wlr_seat->pointer_state.default_grab;
@@ -281,7 +281,9 @@ void wlr_seat_pointer_end_grab(struct wlr_seat *wlr_seat) {
 		if (grab->interface->cancel) {
 			grab->interface->cancel(grab);
 		}
+		return true;
 	}
+	return false;
 }

 void wlr_seat_pointer_notify_enter(struct wlr_seat *wlr_seat,
--
2.18.0