4
2
[RFC PATCH wlroots] rootston: Don't propagate a handled Escape-key
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
[PATCH v2 wlroots] rootston: Don't propagate a handled Escape-key
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
Re: [PATCH v2 wlroots] rootston: Don't propagate a handled Escape-key
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"
[PATCH v3 wlroots] rootston: Don't propagate a handled Escape-key
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