Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

tinywl: fix crash if there is no keyboard #3152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions tinywl/tinywl.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ static void focus_view(struct tinywl_view *view, struct wlr_surface *surface) {
seat->keyboard_state.focused_surface);
wlr_xdg_toplevel_set_activated(previous, false);
}
struct wlr_keyboard *keyboard = wlr_seat_get_keyboard(seat);
/* Move the view to the front */
wl_list_remove(&view->link);
wl_list_insert(&server->views, &view->link);
Expand All @@ -125,8 +124,13 @@ static void focus_view(struct tinywl_view *view, struct wlr_surface *surface) {
* track of this and automatically send key events to the appropriate
* clients without additional work on your part.
*/
wlr_seat_keyboard_notify_enter(seat, view->xdg_surface->surface,
keyboard->keycodes, keyboard->num_keycodes, &keyboard->modifiers);
struct wlr_keyboard *keyboard = wlr_seat_get_keyboard(seat);
if (keyboard != NULL) {
wlr_seat_keyboard_notify_enter(seat, view->xdg_surface->surface,
keyboard->keycodes, keyboard->num_keycodes, &keyboard->modifiers);
} else {
wlr_seat_keyboard_notify_enter(seat, surface, NULL, 0, NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I wonder if it really makes sense to do this…?

Sway may do it because of data device related things (copy/paste relies on keyboard focus).

}
Comment on lines +127 to +133
Copy link
Member Author

@ifreund ifreund Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kinda makes me wonder why wlroots makes compositors do this check themselves, instead wlr_seat_keyboard_notify_enter() could just take a possibly NULL pointer to a wlr_keyboard. Or even just call wlr_seat_get_keyboard() itself internally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yes, that's a good point. At some point we wanted to get rid of wlr_seat_set_keyboard and de-couple wlr_keyboard from wlr_seat (just like wlr_cursor/wlr_pointer is decoupled from wlr_seat), but stopped halfway through. Ref #803

}

static void keyboard_handle_modifiers(
Expand Down