xwm: Handle changing override redirect flag

Submitted by Scott Moreau on March 18, 2018, 6:22 p.m.

Details

Message ID 1521397337-19298-1-git-send-email-oreaus@gmail.com
State New
Headers show
Series "xwm: Handle changing override redirect flag" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Scott Moreau March 18, 2018, 6:22 p.m.
Xwayland windows might be created with a different override redirect
flag than is given on map or configure notify. This causes confusion
about whether a window should be treated as override redirect or not.
Here we handle the changing override redirect flag in relevant notify
handlers so windows are treated appropriately. In particular, this
fixes positioning menus in clients like VLC, though it is not a
complete fix. If the window is moved, the menus are still positioned
as if the window wasn't moved.
---
 xwayland/window-manager.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index c307e19..353bcbb 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -754,6 +754,9 @@  our_resource(struct weston_wm *wm, uint32_t id)
 }
 
 static void
+weston_wm_window_create_frame(struct weston_wm_window *window);
+
+static void
 weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t *event)
 {
 	xcb_configure_notify_event_t *configure_notify =
@@ -775,6 +778,14 @@  weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t *eve
 	window->y = configure_notify->y;
 	window->pos_dirty = false;
 
+	if (window->override_redirect != configure_notify->override_redirect) {
+		if (window->override_redirect)
+			weston_wm_window_create_frame(window);
+		else
+			wm_log("%s: Unhandled override redirect flag change\n", __func__);
+		window->override_redirect = configure_notify->override_redirect;
+	}
+
 	if (window->override_redirect) {
 		window->width = configure_notify->width;
 		window->height = configure_notify->height;
@@ -987,6 +998,9 @@  weston_wm_window_create_frame(struct weston_wm_window *window)
 	int x, y, width, height;
 	int buttons = FRAME_BUTTON_CLOSE;
 
+	if (window->frame_id != XCB_WINDOW_NONE)
+		return;
+
 	if (window->decorate & MWM_DECOR_MAXIMIZE)
 		buttons |= FRAME_BUTTON_MAXIMIZE;
 
@@ -1060,6 +1074,8 @@  weston_wm_window_create_frame(struct weston_wm_window *window)
 			     &rect);
 
 	hash_table_insert(wm->window_hash, window->frame_id, window);
+
+	assert(window->frame_id != XCB_WINDOW_NONE);
 }
 
 /*
@@ -1120,9 +1136,7 @@  weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event)
 	window->map_request_x = window->x;
 	window->map_request_y = window->y;
 
-	if (window->frame_id == XCB_WINDOW_NONE)
-		weston_wm_window_create_frame(window); /* sets frame_id */
-	assert(window->frame_id != XCB_WINDOW_NONE);
+	weston_wm_window_create_frame(window); /* sets frame_id */
 
 	wm_log("XCB_MAP_REQUEST (window %d, %p, frame %d, %dx%d @ %d,%d)\n",
 	       window->id, window, window->frame_id,
@@ -1153,6 +1167,7 @@  static void
 weston_wm_handle_map_notify(struct weston_wm *wm, xcb_generic_event_t *event)
 {
 	xcb_map_notify_event_t *map_notify = (xcb_map_notify_event_t *) event;
+	struct weston_wm_window *window;
 
 	if (our_resource(wm, map_notify->window)) {
 		wm_log("XCB_MAP_NOTIFY (window %d, ours)\n",
@@ -1162,6 +1177,17 @@  weston_wm_handle_map_notify(struct weston_wm *wm, xcb_generic_event_t *event)
 
 	wm_log("XCB_MAP_NOTIFY (window %d%s)\n", map_notify->window,
 	       map_notify->override_redirect ? ", override" : "");
+
+	if (!wm_lookup_window(wm, map_notify->window, &window))
+		return;
+
+	if (window->override_redirect != map_notify->override_redirect) {
+		if (window->override_redirect)
+			weston_wm_window_create_frame(window);
+		else
+			wm_log("%s: Unhandled override redirect flag change\n", __func__);
+		window->override_redirect = map_notify->override_redirect;
+	}
 }
 
 static void
@@ -1333,11 +1359,12 @@  weston_wm_window_schedule_repaint(struct weston_wm_window *window)
 	struct weston_wm *wm = window->wm;
 
 	if (window->frame_id == XCB_WINDOW_NONE) {
-		/* Override-redirect windows go through here, but we
-		 * cannot assert(window->override_redirect); because
-		 * we do not deal with changing OR flag yet.
-		 * XXX: handle OR flag changes in message handlers
-		 */
+		/* Ignore if the window isn't mapped */
+		if (!window->shsurf)
+			return;
+
+		assert(window->override_redirect);
+
 		weston_wm_window_set_pending_state_OR(window);
 		return;
 	}

Comments

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

> Xwayland windows might be created with a different override redirect
> flag than is given on map or configure notify. This causes confusion
> about whether a window should be treated as override redirect or not.
> Here we handle the changing override redirect flag in relevant notify
> handlers so windows are treated appropriately. In particular, this
> fixes positioning menus in clients like VLC, though it is not a
> complete fix. If the window is moved, the menus are still positioned
> as if the window wasn't moved.
> ---
>  xwayland/window-manager.c | 43 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 8 deletions(-)

Hi,

this patch looks big enough that I'd consider it more a new feature
than just a bug fix, so I'd like to postpone this after the 4.0.0
release.

I've marked this as deferred in Patchwork.


Thanks,
pq
This patch is an addition to https://patchwork.freedesktop.org/patch/211161/.

Pekka Paalanen (1):
  xwm: improve override-redirect debug messages

 xwayland/window-manager.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)
Hi,

I think un-reparenting would involve reparenting to the whatever the parent
was before reparenting in the first place. I haven't given this too much
thought and I don't know if this case ever happens in practice but figured
I'd comment about it since Pekka mentioned possibly reviewing it soon.

Thanks,
Scott

On Thu, Mar 29, 2018 at 5:17 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Sun, 18 Mar 2018 12:22:15 -0600
> Scott Moreau <oreaus@gmail.com> wrote:
>
> > Xwayland windows might be created with a different override redirect
> > flag than is given on map or configure notify. This causes confusion
> > about whether a window should be treated as override redirect or not.
> > Here we handle the changing override redirect flag in relevant notify
> > handlers so windows are treated appropriately. In particular, this
> > fixes positioning menus in clients like VLC, though it is not a
> > complete fix. If the window is moved, the menus are still positioned
> > as if the window wasn't moved.
> > ---
> >  xwayland/window-manager.c | 43 ++++++++++++++++++++++++++++++
> +++++--------
> >  1 file changed, 35 insertions(+), 8 deletions(-)
>
> Hi,
>
> this patch looks big enough that I'd consider it more a new feature
> than just a bug fix, so I'd like to postpone this after the 4.0.0
> release.
>
> I've marked this as deferred in Patchwork.
>
>
> Thanks,
> pq
>