Merge pull request #4249 from wlhlm/issue-3889-v2

Only swallow windows once v2

4 files changed, 79 insertions, 0 deletions

diff --git a/RELEASE-NOTES-next b/RELEASE-NOTES-next
index f8b6c65c..56e31bee 100644
--- a/RELEASE-NOTES-next
+++ b/RELEASE-NOTES-next
@@ -52,3 +52,4 @@ working. Please reach out to us in that case!
   • fix Xorg memory leak with i3bar
   • fix named workspace assignments on output changes
   • fix named workspace assignment precedence on workspace renames
+  • fix windows getting swallowed more than once
diff --git a/include/data.h b/include/data.h
index d794909f..e411129f 100644
--- a/include/data.h
+++ b/include/data.h
@@ -476,6 +476,9 @@ struct Window {
     /* Time when the window became managed. Used to determine whether a window
      * should be swallowed after initial management. */
     time_t managed_since;
+
+    /* The window has been swallowed. */
+    bool swallowed;
 };
 
 /**
diff --git a/src/manage.c b/src/manage.c
index 13e67d94..da23ab3e 100644
--- a/src/manage.c
+++ b/src/manage.c
@@ -270,6 +270,7 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki
     DLOG("Initial geometry: (%d, %d, %d, %d)\n", geom->x, geom->y, geom->width, geom->height);
 
     /* See if any container swallows this new window */
+    cwindow->swallowed = false;
     Match *match = NULL;
     Con *nc = con_for_window(search_at, cwindow, &match);
     const bool match_from_restart_mode = (match && match->restart_mode);
@@ -358,6 +359,8 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki
             match_free(match);
             FREE(match);
         }
+
+        cwindow->swallowed = true;
     }
 
     DLOG("new container = %p\n", nc);
@@ -695,6 +698,11 @@ out:
  *
  */
 Con *remanage_window(Con *con) {
+    /* Make sure this windows hasn't already been swallowed. */
+    if (con->window->swallowed) {
+        run_assignments(con->window);
+        return con;
+    }
     Match *match;
     Con *nc = con_for_window(croot, con->window, &match);
     if (nc == NULL || nc->window == NULL || nc->window == con->window) {
@@ -740,5 +748,6 @@ Con *remanage_window(Con *con) {
         ewmh_update_wm_desktop();
     }
 
+    nc->window->swallowed = true;
     return nc;
 }
diff --git a/testcases/t/542-layout-restore-remanage.t b/testcases/t/542-layout-restore-remanage.t
index 26b50835..82ec84d0 100644
--- a/testcases/t/542-layout-restore-remanage.t
+++ b/testcases/t/542-layout-restore-remanage.t
@@ -83,4 +83,70 @@ is($nodes[0]->{name}, 'different_title', 'test window got swallowed');
 
 close($fh);
 
+############################################################
+# Make sure window only gets swallowed once
+############################################################
+# Regression, issue #3888
+$ws = fresh_workspace;
+
+($fh, $filename) = tempfile(UNLINK => 1);
+print $fh <<'EOT';
+// vim:ts=4:sw=4:et
+{
+    // splith split container with 2 children
+    "layout": "splith",
+    "type": "con",
+    "nodes": [
+        {
+            "type": "con",
+            "swallows": [
+               {
+               "class": "^foo$"
+               }
+            ]
+        },
+        {
+            // splitv split container with 2 children
+            "layout": "splitv",
+            "type": "con",
+            "nodes": [
+                {
+                    "type": "con",
+                    "swallows": [
+                       {
+                        "class": "^foo$"
+                       }
+                    ]
+                },
+                {
+                    "type": "con",
+                    "swallows": [
+                       {
+                        "class": "^foo$"
+                       }
+                    ]
+                }
+            ]
+        }
+    ]
+}
+EOT
+$fh->flush;
+cmd "append_layout $filename";
+
+$window = open_window wm_class => 'foo';
+
+# Changing an unrelated window property originally resulted in the window
+# getting remanaged and swallowd by a different placeholder, even though the
+# matching property (class for the layout above) didn't change.
+change_window_title($window, "different_title");
+
+@content = @{get_ws_content($ws)};
+
+subtest 'regression test that window gets only swallowed once', sub {
+    is($content[0]->{nodes}[0]->{window}, $window->id, 'first placeholder swallowed window');
+    isnt($content[0]->{nodes}[1]->{nodes}[0]->{window}, $window->id, 'second placeholder did not swallow window');
+    isnt($content[0]->{nodes}[1]->{nodes}[1]->{window}, $window->id, 'thid placeholder did not swallow window');
+};
+
 done_testing;