Browse Source

fix: dnd for some wm that does not set _NET_CLIENT_LIST_STACKING (#29272)

* fix: dnd for some wm that does not set _NET_CLIENT_LIST_STACKING

Backports https://chromium-review.googlesource.com/c/chromium/src/+/2844104

* chore: update patches

Co-authored-by: deepak1556 <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
trop[bot] 3 years ago
parent
commit
5cbfa5f975

+ 1 - 0
patches/chromium/.patches

@@ -129,3 +129,4 @@ reland_views_handle_deletion_when_toggling_fullscreen.patch
 media_feeds_disable_media_feeds_and_related_features.patch
 remove_tabs_and_line_breaks_from_the_middle_of_app_names_when.patch
 autofill_fixed_refill_of_changed_form.patch
+x11_fix_window_enumeration_order_when_wm_doesn_t_set.patch

+ 95 - 0
patches/chromium/x11_fix_window_enumeration_order_when_wm_doesn_t_set.patch

@@ -0,0 +1,95 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Tudor Brindus <[email protected]>
+Date: Tue, 4 May 2021 07:17:23 +0000
+Subject: x11: Fix window enumeration order when WM doesn't set
+ |_NET_CLIENT_LIST_STACKING|
+
+Chrome needs to know what window to send Xdnd events to during a
+drag-and-drop operation.
+
+It does so through |X11TopmostWindowFinder::FindWindowAt|, which calls
+into |EnumerateWindows| when the WM has not set
+|_NET_CLIENT_LIST_STACKING|. GNOME/mutter set this property, so this is
+a little-tested code path.
+
+However, setting this property is not mandated by the spec, and in
+practice wlroots/Sway do not currently set it.
+
+Chrome's current |EnumerateWindows| fallback path is incorrect,
+resulting in downstream bugs like
+https://github.com/swaywm/sway/issues/5692 and
+https://github.com/swaywm/wlroots/issues/2889.
+
+|EnumerateWindows| ends up calling |EnumerateChildren|, which uses
+|XQueryTree| to determine the stacking order. |XQueryTree| returns
+windows in bottom-to-top order, so the list has to be reverse-iterated
+to get a top-down order that |FindWindowAt| expects (and to match the
+order reported by |_NET_CLIENT_LIST_STACKING|).
+
+The original code introduced in da11eed did so; however, it regressed in
+3c64537 -- currently, the code comments are inconsistent with the actual
+logic.
+
+This commit switches |EnumerateChildren| to use a reverse iterator. It
+is not used anywhere but as a fallback when |_NET_CLIENT_LIST_STACKING|
+is not present, so this should be a safe change.
+
+I have not touched the iteration order of Chrome's X11 menus. I suspect
+that these are in the right order already.
+
+TEST=manually tested on Sway 1.6, with Chromium overlapping an X11 gedit
+     window
+
+Change-Id: I8f777aa566db1e8d0614187fa4b3d156caa1e0f9
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2844104
+Reviewed-by: Maksim Sisov <[email protected]>
+Commit-Queue: Maksim Sisov <[email protected]>
+Cr-Commit-Position: refs/heads/master@{#878767}
+
+diff --git a/AUTHORS b/AUTHORS
+index 296c220888df0c421e1659d08c546fdde11e60af..8c81cc30b347b36eacb4586c713a25f771cbd574 100644
+--- a/AUTHORS
++++ b/AUTHORS
+@@ -1060,6 +1060,7 @@ Toshihito Kikuchi <[email protected]>
+ Trent Willis <[email protected]>
+ Trevor Perrin <[email protected]>
+ Tripta Gupta <[email protected]>
++Tudor Brindus <[email protected]>
+ Tuukka Toivonen <[email protected]>
+ U. Artie Eoff <[email protected]>
+ Umar Hansa <[email protected]>
+diff --git a/ui/base/x/x11_util.cc b/ui/base/x/x11_util.cc
+index 4c0f799f3f06edf17acff3b7f4672866729c7e7d..a07367a0140ab5e8846006a984e007f98a3a488b 100644
+--- a/ui/base/x/x11_util.cc
++++ b/ui/base/x/x11_util.cc
+@@ -703,10 +703,10 @@ bool EnumerateChildren(EnumerateWindowsDelegate* delegate,
+     return false;
+ 
+   std::vector<x11::Window> windows;
+-  std::vector<x11::Window>::iterator iter;
+   if (depth == 0) {
+     XMenuList::GetInstance()->InsertMenuWindows(&windows);
+     // Enumerate the menus first.
++    std::vector<x11::Window>::iterator iter;
+     for (iter = windows.begin(); iter != windows.end(); iter++) {
+       if (delegate->ShouldStopIterating(*iter))
+         return true;
+@@ -721,7 +721,8 @@ bool EnumerateChildren(EnumerateWindowsDelegate* delegate,
+ 
+   // 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++) {
++  std::vector<x11::Window>::reverse_iterator iter;
++  for (iter = windows.rbegin(); iter != windows.rend(); iter++) {
+     if (IsWindowNamed(*iter) && delegate->ShouldStopIterating(*iter))
+       return true;
+   }
+@@ -731,7 +732,7 @@ bool EnumerateChildren(EnumerateWindowsDelegate* delegate,
+   // loop because the recursion and call to XQueryTree are expensive and is only
+   // needed for a small number of cases.
+   if (++depth <= max_depth) {
+-    for (iter = windows.begin(); iter != windows.end(); iter++) {
++    for (iter = windows.rbegin(); iter != windows.rend(); iter++) {
+       if (EnumerateChildren(delegate, *iter, max_depth, depth))
+         return true;
+     }