[PATCH] xwm: Fix wrong input offset for certain clients

[Pekka Paalanen]

On Sun, 18 Mar 2018 12:22:17 -0600
Scott Moreau <oreaus at gmail.com> wrote:

> Some windows might get a create_notify event without the
> override redirect flag set and then get a confiure_notify
> event before map_request is received. This means that when
> weston_wm_window_get_child_position is called in response
> to configure_notify, the wrong offsets are computed resulting
> in wrong input offsets for some clients like steam.

Hi,

do I understand correctly that the window is never set as
override-redirect?

> ---
>  xwayland/window-manager.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index c307e19..2307f1a 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -711,10 +711,12 @@ weston_wm_handle_configure_request(struct weston_wm *wm, xcb_generic_event_t *ev
>  	if (configure_request->value_mask & XCB_CONFIG_WINDOW_HEIGHT)
>  		window->height = configure_request->height;
>  
> -	if (window->frame)
> +	if (window->frame) {
>  		frame_resize_inside(window->frame, window->width, window->height);
> +		weston_wm_window_get_child_position(window, &x, &y);
> +	} else
> +		x = y = 0;

The else branch should have braces as well because the other branch
does.

>  
> -	weston_wm_window_get_child_position(window, &x, &y);
>  	values[i++] = x;
>  	values[i++] = y;
>  	values[i++] = window->width;

For a not yet mapped X11 window, this change looks correct. If there is
no 'window->frame', then the app window has not been reparented either,
which means there is no offset we could apply. In that case, the x, y
will be relative to the root window. We can set them to any number,
e.g. zero, because the window has not been mapped yet. Only when we get
MapRequest and content, the Wayland window manager may actually pick a
position and then we move the X11 window stack to its appropriate
coordinates.

However, if we get a ConfigureRequest for a window that has already
been mapped, and if that window happens to not have a frame, would this
not smash the window to the top-left corner of the screen?

That raises an interesting question: can we have a window that does not
have a frame and also is not override-redirect (O-R means we never see
the ConfigureRequest)?

If XWM sees the MapRequest, it practically always creates the frame. So
the only way to hit that corner case is, if the X11 client creates the
window as a normal window, changes it to O-R, maps it, changes it to
non-O-R, and then attempts to move. Maybe we can live with that, seems
like a pretty deep hole the X11 client is digging?

But if we have your patch to handle O-R state changes, then I suppose
we wouldn't have this corner-case at all, because it would create the
frame, right?

I have to say I don't quite understand how the wrong offsets this patch
fixes can confuse an X11 client's input. Nevertheless, I think I can
give:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

unless something I said here is not true.


I still wonder... the Wayland window manager picks the window position
when it maps it, so could this patch be just a bandaid for a very
specific case, and the real bug is actually somewhere in the Wayland
window manager side for forgetting to call ->send_position() in this
specific case. If that's true, then sending the zeroes like this patch
does is just accidentally correct for the app you tested with.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180329/2f1dd65a/attachment.sig>