Wlroots: X11 drag and drop sometimes drops "through" overlapping windows with Chromium/Electron

[Xyene]

Given something like T[slack* discord], and initiating a drag in Slack:

• the drag image does not respond to movement, and is frozen a couple of pixels from where it started
• releasing the drag causes Discord to receive it, making it request urgent status

Video example:

https://user-images.githubusercontent.com/1403503/112929915-6027d380-90e7-11eb-8c68-3b66dc76ea31.mp4

The logs around this time look like:

14:50:42.736 [DEBUG] [wlr] [types/seat/wlr_seat_pointer.c:384] button_count=1 grab_serial=66649 serial=66651
14:50:42.737 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 401 (_NET_WM_USER_TIME) for window 20971527
14:50:42.968 [DEBUG] [sway/desktop/xwayland.c:761] New xwayland unmanaged surface
14:50:42.968 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 360 (CHROMIUM_TIMESTAMP) for window 20971528
14:50:42.968 [DEBUG] [wlr] [xwayland/selection/incoming.c:487] XCB_XFIXES_SELECTION_NOTIFY (selection=299, owner=20971621)
14:50:42.968 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 380 (XdndActionList) for window 20971527
14:50:42.968 [DEBUG] [sway/desktop/xwayland.c:761] New xwayland unmanaged surface
14:50:42.968 [DEBUG] [wlr] [xwayland/selection/incoming.c:453] XCB_SELECTION_NOTIFY (selection=299, property=282, target=283)
14:50:42.970 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 6291468
14:50:42.970 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 6291468
14:50:42.970 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 6291468
14:50:42.970 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 6291468
14:50:47.954 [DEBUG] [wlr] [types/seat/wlr_seat_pointer.c:384] button_count=0 grab_serial=66651 serial=66652
14:50:47.955 [DEBUG] [wlr] [xwayland/selection/incoming.c:487] XCB_XFIXES_SELECTION_NOTIFY (selection=299, owner=0)
14:50:47.955 [ERROR] [wlr] [xwayland/xwm.c:1386] xcb error: op 18:0, code 3, sequence 18214, value 20971622
14:50:48.804 [DEBUG] [sway/input/cursor.c:893] denying request to set cursor from unfocused client
14:50:49.025 [DEBUG] [sway/input/cursor.c:893] denying request to set cursor from unfocused client
14:50:49.026 [DEBUG] [sway/input/cursor.c:893] denying request to set cursor from unfocused client

--- releasing the drag here ---

14:50:51.119 [DEBUG] [sway/tree/arrange.c:264] Usable area for ws: 1440x2530@0,30
14:50:51.119 [DEBUG] [sway/tree/arrange.c:294] Arranging workspace '2' at 0.000000, 30.000000
14:50:51.119 [DEBUG] [sway/tree/arrange.c:77] Arranging 0x7fffb6e5b2b0 horizontally
14:50:51.119 [DEBUG] [sway/tree/arrange.c:156] Arranging 0x7fffb6e5b240 vertically
14:50:51.119 [DEBUG] [sway/tree/arrange.c:156] Arranging 0x7fffb6e5b160 vertically
14:50:51.119 [DEBUG] [sway/desktop/transaction.c:399] Transaction 0x559e6c8fb040 committing with 10 instructions
14:50:51.119 [DEBUG] [sway/desktop/transaction.c:294] Applying transaction 0x559e6c8fb040
14:50:51.119 [DEBUG] [wlr] [types/seat/wlr_seat_pointer.c:384] button_count=1 grab_serial=66651 serial=66662

In a floating layout like the below, with Chromium partially obscured by Slack and dragging from Slack:

...the drag image position updates with mouse movement until it overlaps with the Chromium window underneath, at which point it freezes.

At the time of intersection, the

14:55:23.241 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 35651585
14:55:23.242 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 35651585
14:55:23.242 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 35651585
14:55:23.242 [DEBUG] [wlr] [xwayland/xwm.c:759] unhandled X11 property 385 (_CHROMIUM_DRAG_RECEIVER) for window 35651585

lines get printed, but nothing else appears in the log.

[Xyene]

I spent a while looking into this last week, figured I'd at least write down my findings before I forget...

• This issue is not reproducible in GNOME, so it's unlikely to be an Xwayland issue.

• The windows involved are both X11. This means that we don't invoke the DnD handlers in wlroots.

• I've found a good test case is Slack on top, and GDK_BACKEND=x11 gedit overlapping behind it. Dragging selected text from Slack over the intersection region between Slack and gedit causes gedit to render a green border around its edit buffer (a visual indicator of the drop region, I imagine), which makes it easy to see what's going on and when.

  A regular Chromium works in place of Slack here, too. Oddly, qutebrowser running under X11 doesn't. Neither does Epiphany.

• Running GDK_BACKEND=x11 xtrace gedit | grep -i dnd shows that the gedit window gets XdndEnter, XdndPosition, and XdndLeave events when over the intersecting region.

  As far as I can tell, it's not wlroots sending these events; Xwayland is doing this internally. I thought it might have to do with Sway's real window stacking order not matching the stacking order communicated to Xwayland, but I added some logging around here (and an extra xcb_flush, just in case) and it does seem like we're requesting XCB_STACK_MODE_ABOVE whenever an X11 window gets focused.

  I have not found a reference for who gets XdndPosition events, so maybe XCB_STACK_MODE_ABOVE is not sufficient? This doesn't seem very likely, since a quick reading of i3 makes it seem like that's all it's doing. This comment is a bit concerning.

  (Note: I'm pretty sure this logic is wrong anyway with windows overlapping over the edges of workspace outputs, since it doesn't match what is actually used for hit-testing, where all floating windows on one of the outputs will stack above all from the other, even if not focused. But it's not the bug at play here.)

• There might be a way to dump X11's stacking order to verify it matches our expectations, but I didn't find anything in a cursory search.

/cc @emersion, @psychon -- any thoughts or ideas?

[Xyene]

By chance I came across https://github.com/swaywm/sway/issues/5692, which I'm pretty confident is the same underlying bug.

The reproduction steps are identical; reordering Chromium tabs only goes crazy if there's another X11 app underlapping them. This is not clear from the issue description of https://github.com/swaywm/sway/issues/5692, but I've tested it to be the case.

[psychon]

@psychon -- any thoughts or ideas?

Thanks a lot for asking :-)

I thought it might have to do with Sway's real window stacking order not matching the stacking order communicated to Xwayland,

Yup, that sounds a lot like it.

There might be a way to dump X11's stacking order

QueryTree does!

From https://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#requests:QueryTree

The children are listed in bottom-to-top stacking order.

I just tried xwininfo -root -tree here and... I am not sure about its output order. I guess I get confused because of different workspaces, but I would expect this to just print the order returned by QueryTree....

I have not found a reference for who gets XdndPosition events, so maybe XCB_STACK_MODE_ABOVE is not sufficient?

Since this X11, it is of course X11 clients sending that to each other. ;-)

https://www.freedesktop.org/wiki/Specifications/XDND/

(I never looked at this protocol. I only know it exists. https://www.freedesktop.org/wiki/Specifications/XDND/#examplewalk-through seems helpful.)

I tried finding a small-ish example implementation that the above page promises, but didn't find it. So, Gtk it is.

This is the function that seems to turn a mouse position into a window: https://github.com/GNOME/gtk/blob/77f32a69c0a35ffd3a1a542a6057c1c6d13c793d/gdk/x11/gdkdnd-x11.c#L2126
This uses get_client_window_at_coords to find the window... but at that point it already has a list of windows to check.

That list comes from here via gdk_screen_get_toplevel_windows: https://github.com/GNOME/gtk/blob/77f32a69c0a35ffd3a1a542a6057c1c6d13c793d/gdk/x11/gdkdnd-x11.c#L591

Sigh. That found just returns an already existing list. I failed at finding how it gets that list.

Edit: Well, no, I read the if for trusted_client wrong. The code selects SubstructureNotifyMask on the root window and uses those events to update its cache. The "real data" comes from QueryTree via _gdk_x11_get_window_child_info. So, that should be in stacking order...

This part didn't lead anywhere

So, random guess would be the _NET_CLIENT_LIST_STACKING property on the root window. Seems like sway/wlroots do not support that property. Anyway, git grep found gdk_x11_screen_get_window_stack in gdk/x11/gdkscreen-x11.c. That function.... does not do anything with the WM does not support this property. Hm... but it also seems like gtk does not use that anywhere internally. git grep get_window_stack looks like it.

Coming back to get_client_window_at_coords: This function recurses into the window tree, looking for windows with WM_STATE set that contain the mouse cursor. Seems sensible.

it does seem like we're requesting XCB_STACK_MODE_ABOVE whenever an X11 window gets focused.

Random guess would be X11 clients reordering themselves because they feel important. But xwm_handle_configure_request blocks anything not size-related. Hm...

Looking a bit at xwm.c, there are three places where STACK_MODE appears:

• When a window is focused, it gets put at the top
• When a window is mapped, it is put at the top
• There is wlr_xwayland_surface_restack which allows other code to cause restacking. However, I couldn't find any callers in wlroots nor sway.

So, I guess the stacking order is based on "how recently was a window mapped or last focsed"? The currently focused window should definitely be at top...

[Xyene]

Since this X11, it is of course X11 clients sending that to each other. ;-)

The gift that keeps on giving :')

I just tried xwininfo -root -tree here and... I am not sure about its output order.

I think it's top-to-bottom ordering; I played around with watch -n.1 -d xwininfo -root -tree while playing with focusing and the order updated as expected. In particular, the ordering matched that when doing the same in GNOME.

This part didn't lead anywhere

I think you're onto something with _NET_CLIENT_LIST_STACKING! GDK may not do anything special with it, but GDK as the drag _source_ was never buggy -- only Chromium-based things are. Also, GNOME _does_ specify _NET_CLIENT_LIST_STACKING, which is probably why things work there.

Chromium uses FindWindowFor within ProcessMouseMove to determine who to send Xdnd events to. That delegates, I think, to X11TopmostWindowFinder::FindWindowAt, which calls into EnumerateWindows:

void EnumerateTopLevelWindows(ui::EnumerateWindowsDelegate* delegate) {
  std::vector<x11::Window> stack;
  if (!ui::GetXWindowStack(ui::GetX11RootWindow(), &stack)) {
    // Window Manager doesn't support _NET_CLIENT_LIST_STACKING, so fall back
    // to old school enumeration of all X windows.  Some WMs parent 'top-level'
    // windows in unnamed actual top-level windows (ion WM), so extend the
    // search depth to all children of top-level windows.
    const int kMaxSearchDepth = 1;
    ui::EnumerateAllWindows(delegate, kMaxSearchDepth);
    return;
  }
  XMenuList::GetInstance()->InsertMenuWindows(&stack);

  std::vector<x11::Window>::iterator iter;
  for (iter = stack.begin(); iter != stack.end(); iter++) {
    if (delegate->ShouldStopIterating(*iter))
      return;
  }
}

The definition of EnumerateAllWindows is trivial...

bool EnumerateAllWindows(EnumerateWindowsDelegate* delegate, int max_depth) {
  x11::Window root = GetX11RootWindow();
  return EnumerateChildren(delegate, root, max_depth, 0);
}

...but EnumerateChildren is a fair bit longer and hard to run through "mentally".

But wait. This comment is just straight up wrong:

  // XQueryTree returns the children of |window| in bottom-to-top order, so
  // reverse-iterate the list to check the windows from top-to-bottom.
  for (iter = windows.begin(); iter != windows.end(); iter++) {
    if (IsWindowNamed(*iter) && delegate->ShouldStopIterating(*iter))
      return true;
  }

This was originally rbegin and rend when introduced 12 years ago in https://github.com/chromium/chromium/commit/da11eed5cd62f052f6b335f21ebb451ec2b3e510. But this regressed 10 years ago in https://github.com/chromium/chromium/commit/3c64537927e5a31388c8fac8e04e575a12f6ff81 (titled "Do not show notifications when in fullscreen or screensaver mode.", seemingly an unrelated change):

  // XQueryTree returns the children of |window| in bottom-to-top order, so
  // reverse-iterate the list to check the windows from top-to-bottom.
-  std::set<XID>::reverse_iterator iter;
-  for (iter = windows.rbegin(); iter != windows.rend(); iter++) {
+  std::vector<XID>::iterator iter;
+  for (iter = windows.begin(); iter != windows.end(); iter++) {
    if (IsWindowNamed(*iter) && delegate->ShouldStopIterating(*iter))
      return true;
  }

So this is a Chromium bug, that happens to not happen in GNOME because GNOME supports _NET_CLIENT_LIST_STACKING.

Moving to wlroots and marking as a feature-request for _NET_CLIENT_LIST_STACKING, which would "fix" the problem.

[psychon]

This was originally rbegin and rend when introduced 12 years ago in chromium/chromium@da11eed. But this regressed 10 years ago in chromium/chromium@3c64537

Sounds like a very important code path that everyone runs all the time. 10 years. That's only, like yesterday!

(Feel free to mark this comment as off-topic, because it is, but) does anyone know why _NET_CLIENT_LIST_STACKING exists? XQueryTree provides the same information, but perhaps in a less-easy-to-consume form (and a lot harder to watch for changes).

[Xyene]

Chromium patch is up: https://chromium-review.googlesource.com/c/chromium/src/+/2844104

Thanks @psychon for your help in figuring this out :)

does anyone know why _NET_CLIENT_LIST_STACKING exists?

I tried tracking it down when you first mentioned it, but I couldn't find anything. Ergonomics and performance are likely reasons; at the very least Chrome has extra logic to avoid a call to XQueryTree and a comment saying that it's "expensive".