Merge pull request #2978 from orestisf1993/issue-2572

Issue 2572

src/click.c

@@ -262,8 +262,6 @@ static int route_click(Con *con, xcb_button_press_event_t *event, const bool mod
      * We will skip handling events on floating cons in fullscreen mode */
     Con *fs = (ws ? con_get_fullscreen_con(ws, CF_OUTPUT) : NULL);
     if (floatingcon != NULL && fs != con) {
-        floating_raise_con(floatingcon);
-
         /* 4: floating_modifier plus left mouse button drags */
         if (mod_pressed && event->detail == XCB_BUTTON_CLICK_LEFT) {
             floating_drag_window(floatingcon, event);

src/con.c

@@ -243,6 +243,13 @@ void con_focus(Con *con) {
         workspace_update_urgent_flag(con_get_workspace(con));
         ipc_send_window_event("urgent", con);
     }
+
+    /* Focusing a container with a floating parent should raise it to the top. Since
+     * con_focus is called recursively for each parent we don't need to use
+     * con_inside_floating(). */
+    if (con->type == CT_FLOATING_CON) {
+        floating_raise_con(con);
+    }
 }
 
 /*

src/floating.c

@@ -196,7 +196,11 @@ void floating_enable(Con *con, bool automatic) {
     /* We insert nc already, even though its rect is not yet calculated. This
      * is necessary because otherwise the workspace might be empty (and get
      * closed in tree_close_internal()) even though it’s not. */
-    TAILQ_INSERT_TAIL(&(ws->floating_head), nc, floating_windows);
+    if (set_focus) {
+        TAILQ_INSERT_TAIL(&(ws->floating_head), nc, floating_windows);
+    } else {
+        TAILQ_INSERT_HEAD(&(ws->floating_head), nc, floating_windows);
+    }
     TAILQ_INSERT_TAIL(&(ws->focus_head), nc, focused);
 
     /* check if the parent container is empty and close it if so */

src/handlers.c

@@ -1221,7 +1221,9 @@ static void handle_focus_in(xcb_focus_in_event_t *event) {
         return;
     }
 
-    if (focused_id == event->event) {
+    /* Floating windows should be refocused to ensure that they are on top of
+     * other windows. */
+    if (focused_id == event->event && !con_inside_floating(con)) {
         DLOG("focus matches the currently focused window, not doing anything\n");
         return;
     }
@@ -1232,7 +1234,7 @@ static void handle_focus_in(xcb_focus_in_event_t *event) {
         return;
     }
 
-    DLOG("focus is different, updating decorations\n");
+    DLOG("focus is different / refocusing floating window: updating decorations\n");
 
     /* Get the currently focused workspace to check if the focus change also
      * involves changing workspaces. If so, we need to call workspace_show() to
@@ -1244,7 +1246,7 @@ static void handle_focus_in(xcb_focus_in_event_t *event) {
     con_focus(con);
     /* We update focused_id because we don’t need to set focus again */
     focused_id = event->event;
-    x_push_changes(croot);
+    tree_render();
     return;
 }
 

testcases/t/135-floating-focus.t

@@ -216,4 +216,28 @@ cmd 'focus child';
 
 is($x->input_focus, $floating->id, 'floating window focused');
 
+#############################################################################
+# 8: verify that focusing a floating window raises it to the top.
+# This test can't verify that the floating container is visually on top, just
+# that it is placed on the tail of the floating_head.
+# See issue: 2572
+#############################################################################
+
+$tmp = fresh_workspace;
+
+$first = open_floating_window;
+$second = open_floating_window;
+
+is($x->input_focus, $second->id, 'second floating window focused');
+my $ws = get_ws($tmp);
+is($ws->{floating_nodes}->[1]->{nodes}->[0]->{window}, $second->id, 'second on top');
+is($ws->{floating_nodes}->[0]->{nodes}->[0]->{window}, $first->id, 'first behind');
+
+cmd '[id=' . $first->id . '] focus';
+
+is($x->input_focus, $first->id, 'first floating window focused');
+$ws = get_ws($tmp);
+is($ws->{floating_nodes}->[1]->{nodes}->[0]->{window}, $first->id, 'first on top');
+is($ws->{floating_nodes}->[0]->{nodes}->[0]->{window}, $second->id, 'second behind');
+
 done_testing;