Bugfix: Properly ignore UnmapNotify events (especially for floating windows)

This fixes the bug which caused floating windows to be visible even when
switching to a different workspace.

Instead of ignoring a specific sequence, we now set an ignore_unmap counter for
each container. (So, should containers be closed too early or stay open even if
they should be closed, we probably need to have a closer look at the counter.
At the moment, it is increased by one on reparenting and unmapping (for
workspace changes) and decremented by one on each UnmapNotify event).

This system is better because a sequence does not describe a single unmap or
reparent request but a request to X11 on the network layer -- which can contain
multiple requests.

diff --git a/include/data.h b/include/data.h
index cc9fca1db7a9f82912bd846881c14efdd5aefd8c..c7fcb4a5cc9bcfbf3420109a73fa6d9b3c5f3e95 100644 (file)
--- a/include/data.h
+++ b/include/data.h
@@ -338,6 +338,12 @@ struct Con {
         FLOATING_USER_ON = 3
     } floating;
 
+    /** This counter contains the number of UnmapNotify events for this
+     * container (or, more precisely, for its ->frame) which should be ignored.
+     * UnmapNotify events need to be ignored when they are caused by i3 itself,
+     * for example when reparenting or when unmapping the window on a workspace
+     * change. */
+    uint8_t ignore_unmap;
 
     TAILQ_ENTRY(Con) nodes;
     TAILQ_ENTRY(Con) focused;

diff --git a/src/handlers.c b/src/handlers.c
index e8c626e1a6339958ee3df6a03bdcdfec494b99dc..f5966db921e40e5c55d9e97e48a354271c730ea1 100644 (file)
--- a/src/handlers.c
+++ b/src/handlers.c
@@ -444,8 +444,6 @@ int handle_screen_change(void *prophs, xcb_connection_t *conn,
  */
 int handle_unmap_notify_event(void *data, xcb_connection_t *conn, xcb_unmap_notify_event_t *event) {
 
-    bool ignored = event_is_ignored(event->sequence);
-
     /* FIXME: we cannot ignore this sequence because more UnmapNotifys with the same sequence
      * numbers but different window IDs may follow */
     /* we need to ignore EnterNotify events which will be generated because a
@@ -453,14 +451,24 @@ int handle_unmap_notify_event(void *data, xcb_connection_t *conn, xcb_unmap_noti
     //add_ignore_event(event->sequence);
 
     DLOG("UnmapNotify for 0x%08x (received from 0x%08x), serial %d\n", event->window, event->event, event->sequence);
-    if (ignored) {
-        DLOG("Ignoring UnmapNotify (generated by reparenting)\n");
-        unignore_event(event->sequence);
-        return 1;
-    }
     Con *con = con_by_window_id(event->window);
     if (con == NULL) {
-        LOG("Not a managed window, ignoring\n");
+        /* This could also be an UnmapNotify for the frame. We need to
+         * decrement the ignore_unmap counter. */
+        con = con_by_frame_id(event->window);
+        if (con == NULL) {
+            LOG("Not a managed window, ignoring UnmapNotify event\n");
+            return 1;
+        }
+        if (con->ignore_unmap > 0)
+            con->ignore_unmap--;
+        DLOG("ignore_unmap = %d for frame of container %p\n", con->ignore_unmap, con);
+        return 1;
+    }
+
+    if (con->ignore_unmap > 0) {
+        DLOG("ignore_unmap = %d, dec\n", con->ignore_unmap);
+        con->ignore_unmap--;
         return 1;
     }
 

diff --git a/src/tree.c b/src/tree.c
index 7d12cab1799368d89c052e5a5f385b1a66839451..dac949ba09b3087733637a6d2517dc08ee65b056 100644 (file)
--- a/src/tree.c
+++ b/src/tree.c
@@ -291,10 +291,21 @@ void level_down() {
 
 static void mark_unmapped(Con *con) {
     Con *current;
+    DLOG("marking container %p unmapped\n", con);
 
     con->mapped = false;
     TAILQ_FOREACH(current, &(con->nodes_head), nodes)
         mark_unmapped(current);
+    if (con->type == CT_WORKSPACE) {
+        TAILQ_FOREACH(current, &(con->floating_head), floating_windows) {
+            DLOG("Marking unmapped for floating %p\n", current);
+            current->mapped = false;
+            Con *child = TAILQ_FIRST(&(current->nodes_head));
+            DLOG("  unmapping floating child %p\n", child);
+            child->mapped = false;
+        }
+    }
+    DLOG("mark_unmapped done\n");
 }
 
 /*

diff --git a/src/x.c b/src/x.c
index 1ee23e64ab45b4b73a306c4530eaf2f129f83505..0de6c65a8ea6bddcff217c94edbd64633f04f7f5 100644 (file)
--- a/src/x.c
+++ b/src/x.c
@@ -425,6 +425,10 @@ static void x_push_node(Con *con) {
 
         state->old_frame = XCB_NONE;
         state->need_reparent = false;
+
+        con->ignore_unmap++;
+        DLOG("ignore_unmap for reparenting of con %p (win 0x%08x) is now %d\n",
+                con, con->window->id, con->ignore_unmap);
     }
 
     /* map/unmap if map state changed, also ensure that the child window
@@ -442,6 +446,11 @@ static void x_push_node(Con *con) {
 
             cookie = xcb_unmap_window(conn, con->frame);
             LOG("unmapping container (serial %d)\n", cookie.sequence);
+            /* we need to increase ignore_unmap for this container (if it contains a window) and for every window "under" this one which contains a window */
+            if (con->window != NULL) {
+                con->ignore_unmap++;
+                DLOG("ignore_unmap for con %p (frame 0x%08x) now %d\n", con, con->frame, con->ignore_unmap);
+            }
             /* Ignore enter_notifies which are generated when unmapping */
             add_ignore_event(cookie.sequence);
         } else {

diff --git a/testcases/t/36-floating-unmap.t b/testcases/t/36-floating-unmap.t
new file mode 100644 (file)
index 0000000..9d122fa
--- /dev/null
+++ b/testcases/t/36-floating-unmap.t
@@ -0,0 +1,64 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+# Regression test: Floating windows were not correctly unmapped when switching
+# to a different workspace.
+
+use i3test tests => 5;
+use X11::XCB qw(:all);
+use Time::HiRes qw(sleep);
+
+BEGIN {
+    use_ok('X11::XCB::Window');
+}
+
+my $i3 = i3("/tmp/nestedcons");
+
+my $tmp = get_unused_workspace();
+$i3->command("workspace $tmp")->recv;
+
+#############################################################################
+# 1: open a floating window, get it mapped
+#############################################################################
+
+my $x = X11::XCB::Connection->new;
+
+# FIXME: we open a tiling container as long as t/37* is not done (should swap positions with t/36* then)
+my $twindow = $x->root->create_child(
+    class => WINDOW_CLASS_INPUT_OUTPUT,
+    rect => [ 0, 0, 30, 30],
+    background_color => '#C0C0C0',
+);
+
+isa_ok($twindow, 'X11::XCB::Window');
+
+$twindow->map;
+
+sleep 0.25;
+
+# Create a floating window which is smaller than the minimum enforced size of i3
+my $window = $x->root->create_child(
+    class => WINDOW_CLASS_INPUT_OUTPUT,
+    rect => [ 0, 0, 30, 30],
+    background_color => '#C0C0C0',
+    # replace the type with 'utility' as soon as the coercion works again in X11::XCB
+    window_type => $x->atom(name => '_NET_WM_WINDOW_TYPE_UTILITY'),
+);
+
+isa_ok($window, 'X11::XCB::Window');
+
+$window->map;
+
+sleep 0.25;
+
+ok($window->mapped, 'Window is mapped');
+
+# switch to a different workspace, see if the window is still mapped?
+
+my $otmp = get_unused_workspace();
+$i3->command("workspace $otmp")->recv;
+
+sleep 0.25;
+
+ok(!$window->mapped, 'Window is not mapped after switching ws');
+
+$i3->command("nop testcase done")->recv;