Merge pull request #2941 from orestisf1993/issue-2938

Fix focus order in floating_disable & floating_enable for unfocused windows
next
Ingo Bürk 2018-03-31 20:34:17 +02:00 committed by GitHub
commit 666aa9e0dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 286 additions and 90 deletions

View File

@ -17,3 +17,13 @@
*
*/
void tree_move(Con *con, int direction);
typedef enum { BEFORE,
AFTER } position_t;
/**
* This function detaches 'con' from its parent and inserts it either before or
* after 'target'.
*
*/
void insert_con_into(Con *con, Con *target, position_t position);

View File

@ -1436,40 +1436,6 @@ orientation_t con_orientation(Con *con) {
*
*/
Con *con_next_focused(Con *con) {
Con *next;
/* floating containers are attached to a workspace, so we focus either the
* next floating container (if any) or the workspace itself. */
if (con->type == CT_FLOATING_CON) {
DLOG("selecting next for CT_FLOATING_CON\n");
next = TAILQ_NEXT(con, floating_windows);
DLOG("next = %p\n", next);
if (!next) {
next = TAILQ_PREV(con, floating_head, floating_windows);
DLOG("using prev, next = %p\n", next);
}
if (!next) {
Con *ws = con_get_workspace(con);
next = ws;
DLOG("no more floating containers for next = %p, restoring workspace focus\n", next);
while (next != TAILQ_END(&(ws->focus_head)) && !TAILQ_EMPTY(&(next->focus_head))) {
next = TAILQ_FIRST(&(next->focus_head));
if (next == con) {
DLOG("skipping container itself, we want the next client\n");
next = TAILQ_NEXT(next, focused);
}
}
if (next == TAILQ_END(&(ws->focus_head))) {
DLOG("Focus list empty, returning ws\n");
next = ws;
}
} else {
/* Instead of returning the next CT_FLOATING_CON, we descend it to
* get an actual window to focus. */
next = con_descend_focused(next);
}
return next;
}
/* dock clients cannot be focused, so we focus the workspace instead */
if (con->parent->type == CT_DOCKAREA) {
DLOG("selecting workspace for dock client\n");
@ -1478,10 +1444,9 @@ Con *con_next_focused(Con *con) {
/* if 'con' is not the first entry in the focus stack, use the first one as
* its currently focused already */
Con *first = TAILQ_FIRST(&(con->parent->focus_head));
if (first != con) {
DLOG("Using first entry %p\n", first);
next = first;
Con *next = TAILQ_FIRST(&(con->parent->focus_head));
if (next != con) {
DLOG("Using first entry %p\n", next);
} else {
/* try to focus the next container on the same level as this one or fall
* back to its parent */
@ -1496,6 +1461,10 @@ Con *con_next_focused(Con *con) {
next = TAILQ_FIRST(&(next->focus_head));
}
if (con->type == CT_FLOATING_CON && next != con->parent) {
next = con_descend_focused(next);
}
return next;
}

View File

@ -176,11 +176,36 @@ void floating_enable(Con *con, bool automatic) {
return;
}
Con *focus_head_placeholder = NULL;
bool focus_before_parent = true;
if (!set_focus) {
/* Find recursively the ancestor container which is a child of our workspace.
* We need to reuse its focus position later. */
Con *ancestor = con;
while (ancestor->parent->type != CT_WORKSPACE) {
focus_before_parent &= TAILQ_FIRST(&(ancestor->parent->focus_head)) == ancestor;
ancestor = ancestor->parent;
}
/* Consider the part of the focus stack of our current workspace:
* [ ... S_{i-1} S_{i} S_{i+1} ... ]
* Where S_{x} is a container tree and the container 'con' that is beeing switched to
* floating belongs in S_{i}. The new floating container, 'nc', will have the
* workspace as its parent so it needs to be placed in this stack. If C was focused
* we just need to call con_focus(). Otherwise, nc must be placed before or after S_{i}.
* We should avoid using the S_{i} container for our operations since it might get
* killed if it has no other children. So, the two possible positions are after S_{i-1}
* or before S_{i+1}.
*/
if (focus_before_parent) {
focus_head_placeholder = TAILQ_PREV(ancestor, focus_head, focused);
} else {
focus_head_placeholder = TAILQ_NEXT(ancestor, focused);
}
}
/* 1: detach the container from its parent */
/* TODO: refactor this with tree_close_internal() */
TAILQ_REMOVE(&(con->parent->nodes_head), con, nodes);
TAILQ_REMOVE(&(con->parent->focus_head), con, focused);
con_detach(con);
con_fix_percent(con->parent);
/* 2: create a new container to render the decoration on, add
@ -196,12 +221,23 @@ 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 its not. */
if (set_focus) {
TAILQ_INSERT_TAIL(&(ws->floating_head), nc, floating_windows);
TAILQ_INSERT_HEAD(&(ws->floating_head), nc, floating_windows);
struct focus_head *fh = &(ws->focus_head);
if (focus_before_parent) {
if (focus_head_placeholder) {
TAILQ_INSERT_AFTER(fh, focus_head_placeholder, nc, focused);
} else {
TAILQ_INSERT_HEAD(fh, nc, focused);
}
} else {
TAILQ_INSERT_HEAD(&(ws->floating_head), nc, floating_windows);
if (focus_head_placeholder) {
TAILQ_INSERT_BEFORE(focus_head_placeholder, nc, focused);
} else {
/* Also used for the set_focus case */
TAILQ_INSERT_TAIL(fh, nc, focused);
}
}
TAILQ_INSERT_TAIL(&(ws->focus_head), nc, focused);
/* check if the parent container is empty and close it if so */
if ((con->parent->type == CT_CON || con->parent->type == CT_FLOATING_CON) &&
@ -329,45 +365,22 @@ void floating_disable(Con *con, bool automatic) {
return;
}
const bool set_focus = (con == focused);
Con *ws = con_get_workspace(con);
Con *parent = con->parent;
Con *tiling_focused = con_descend_tiling_focused(ws);
/* 1: detach from parent container */
TAILQ_REMOVE(&(con->parent->nodes_head), con, nodes);
TAILQ_REMOVE(&(con->parent->focus_head), con, focused);
/* 2: kill parent container */
TAILQ_REMOVE(&(con->parent->parent->floating_head), con->parent, floating_windows);
TAILQ_REMOVE(&(con->parent->parent->focus_head), con->parent, focused);
/* clear the pointer before calling tree_close_internal in which the memory is freed */
con->parent = NULL;
tree_close_internal(parent, DONT_KILL_WINDOW, true, false);
/* 3: re-attach to the parent of the currently focused con on the workspace
* this floating con was on */
Con *focused = con_descend_tiling_focused(ws);
/* if there is no other container on this workspace, focused will be the
* workspace itself */
if (focused->type == CT_WORKSPACE)
con->parent = focused;
else
con->parent = focused->parent;
/* con_fix_percent will adjust the percent value */
con->percent = 0.0;
if (tiling_focused->type == CT_WORKSPACE) {
Con *parent = con->parent;
con_detach(con);
con->parent = NULL;
tree_close_internal(parent, DONT_KILL_WINDOW, true, false);
con_attach(con, tiling_focused, false);
con->percent = 0.0;
con_fix_percent(con->parent);
} else {
insert_con_into(con, tiling_focused, AFTER);
}
con->floating = FLOATING_USER_OFF;
con_attach(con, con->parent, false);
con_fix_percent(con->parent);
if (set_focus)
con_activate(con);
floating_set_hint_atom(con, false);
ipc_send_window_event("floating", con);
}

View File

@ -9,9 +9,6 @@
*/
#include "all.h"
typedef enum { BEFORE,
AFTER } position_t;
/*
* Returns the lowest container in the tree that has both a and b as descendants.
*
@ -65,7 +62,7 @@ static bool is_focused_descendant(Con *con, Con *ancestor) {
* after 'target'.
*
*/
static void insert_con_into(Con *con, Con *target, position_t position) {
void insert_con_into(Con *con, Con *target, position_t position) {
Con *parent = target->parent;
/* We need to preserve the old con->parent. While it might still be used to
* insert the entry before/after it, we call the on_remove_child callback

View File

@ -105,18 +105,14 @@ cmd 'split v';
cmd 'layout stacked';
$second = open_window({ background_color => '#00ff00' }); # window 6
$third = open_window({ background_color => '#0000ff' }); # window 7
is($x->input_focus, $third->id, 'last container focused');
cmd 'floating enable';
cmd '[id="' . $second->id . '"] focus';
is($x->input_focus, $second->id, 'second con focused');
cmd 'floating enable';
cmd '[id="' . $third->id . '"] floating enable';
sync_with_i3;
is($x->input_focus, $second->id, 'second con focused');
# now kill the second one. focus should fall back to the third one, which is
# also floating
@ -240,4 +236,184 @@ $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');
#############################################################################
# 9: verify that disabling / enabling floating for a window from a different
# workspace maintains the correct focus order.
#############################################################################
sub open_window_helper {
my $floating = shift if @_;
if ($floating){
return open_floating_window;
}
else {
return open_window;
}
}
for my $floating (0, 1){
$tmp = fresh_workspace;
$first = open_window;
$second = open_window_helper($floating);
is($x->input_focus, $second->id, "second window focused");
fresh_workspace;
cmd "[id=" . $second->id . "] floating toggle";
cmd "workspace $tmp";
sync_with_i3;
my $workspace = get_ws($tmp);
is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $second->id, 'second window on first workspace, floating') unless $floating;
is($workspace->{nodes}->[1]->{window}, $second->id, 'second window on first workspace, right') unless !$floating;
is($x->input_focus, $second->id, 'second window still focused');
}
#############################################################################
# 10: verify that toggling floating for an unfocused window on another
# workspace doesn't make it focused.
#############################################################################
for my $floating (0, 1){
$tmp = fresh_workspace;
$first = open_window_helper($floating);
$second = open_window;
is($x->input_focus, $second->id, 'second (tiling) window focused');
fresh_workspace;
cmd "[id=" . $first->id . "] floating toggle";
cmd "workspace $tmp";
sync_with_i3;
my $workspace = get_ws($tmp);
is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $first->id, 'first window on first workspace, floating') unless $floating;
is($workspace->{nodes}->[1]->{window}, $first->id, 'first window on first workspace, right') unless !$floating;
is($x->input_focus, $second->id, 'second window still focused');
}
#############################################################################
# 11: verify that toggling floating for a focused window on another workspace
# which has another, unfocused floating window maintains the focus of the
# first window.
#############################################################################
for my $floating (0, 1){
$tmp = fresh_workspace;
$first = open_window;
$second = open_floating_window;
is($x->input_focus, $second->id, 'second (floating) window focused');
$third = open_window_helper($floating);
is($x->input_focus, $third->id, "third (floating = $floating) window focused");
fresh_workspace;
cmd "[id=" . $third->id . "] floating toggle";
cmd "workspace $tmp";
sync_with_i3;
my $workspace = get_ws($tmp);
is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $third->id, 'third window on first workspace, floating') unless $floating;
is($workspace->{nodes}->[1]->{window}, $third->id, 'third window on first workspace, right') unless !$floating;
is($x->input_focus, $third->id, 'third window still focused');
}
#############################################################################
# 12: verify that toggling floating for an unfocused window on another
# workspace which has another, focused floating window doesn't change focus.
#############################################################################
for my $floating (0, 1){
$tmp = fresh_workspace;
$first = open_window;
$second = open_window_helper($floating);
is($x->input_focus, $second->id, "second (floating = $floating) window focused");
$third = open_floating_window;
is($x->input_focus, $third->id, 'third (floating) window focused');
fresh_workspace;
cmd "[id=" . $second->id . "] floating toggle";
cmd "workspace $tmp";
sync_with_i3;
my $workspace = get_ws($tmp);
is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $second->id, 'second window on first workspace, floating') unless $floating;
is($workspace->{nodes}->[1]->{window}, $second->id, 'second window on first workspace, right') unless !$floating;
is($x->input_focus, $third->id, 'third window still focused');
}
#############################################################################
# 13: For layout [H1 [A V1[ B F ] ] ] verify that toggling F's floating
# mode maintains its focus.
#############################################################################
for my $floating (0, 1){
$tmp = fresh_workspace;
$first = open_window;
$second = open_window;
cmd "split v";
sync_with_i3;
is($x->input_focus, $second->id, "second (floating = $floating) window focused");
$third = open_window_helper($floating);
is($x->input_focus, $third->id, 'third (floating) window focused');
fresh_workspace;
cmd "[id=" . $third->id . "] floating toggle";
cmd "workspace $tmp";
sync_with_i3;
my $workspace = get_ws($tmp);
is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $third->id, 'third window on first workspace, floating') unless $floating;
is($workspace->{nodes}->[1]->{nodes}->[1]->{window}, $third->id, 'third window on first workspace') unless !$floating;
is($x->input_focus, $third->id, 'third window still focused');
}
#############################################################################
# 14: For layout [H1 [A V1[ H2 [B H2 [ C V2 [ F D ] ] ] ] ] ] verify that
# toggling F's floating mode maintains its focus.
#############################################################################
sub kill_and_confirm_focus {
my $focus = shift;
my $msg = shift;
cmd "kill";
sync_with_i3;
is($x->input_focus, $focus, $msg);
}
$tmp = fresh_workspace;
my $A = open_window;
my $B = open_window;
cmd "split v";
my $C = open_window;
cmd "split h";
my $F = open_window;
cmd "split v";
my $D = open_window;
is($x->input_focus, $D->id, "D is focused");
sync_with_i3;
my $workspace = get_ws($tmp);
is($workspace->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{nodes}->[0]->{window}, $F->id, 'F opened in its expected position');
fresh_workspace;
cmd "[id=" . $F->id . "] floating enable";
cmd "workspace $tmp";
sync_with_i3;
$workspace = get_ws($tmp);
is($workspace->{floating_nodes}->[0]->{nodes}->[0]->{window}, $F->id, 'F on first workspace, floating');
is($workspace->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{nodes}->[0]->{window}, $D->id, 'D where F used to be');
is($x->input_focus, $D->id, 'D still focused');
fresh_workspace;
cmd "[id=" . $F->id . "] floating disable";
cmd "workspace $tmp";
sync_with_i3;
$workspace = get_ws($tmp);
is($workspace->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{nodes}->[1]->{window}, $F->id, 'F where D used to be');
is($x->input_focus, $D->id, 'D still focused');
kill_and_confirm_focus($F->id, 'F focused after D is killed');
kill_and_confirm_focus($C->id, 'C focused after F is killed');
kill_and_confirm_focus($B->id, 'B focused after C is killed');
kill_and_confirm_focus($A->id, 'A focused after B is killed');
done_testing;

View File

@ -176,4 +176,35 @@ cmd '[id=' . $windows[2]->id . '] move to workspace ' . $ws;
cmd '[id=' . $windows[1]->id . '] move to workspace ' . $ws;
confirm_focus('\'move to workspace\' focus order when moving containers from other workspace');
######################################################################
# Test focus order with floating and tiling windows.
# See issue: 1975
######################################################################
fresh_workspace;
$windows[2] = open_window;
$windows[0] = open_window;
$windows[3] = open_floating_window;
$windows[1] = open_floating_window;
focus_windows;
confirm_focus('mix of floating and tiling windows');
######################################################################
# Same but an unfocused tiling window is killed first.
######################################################################
fresh_workspace;
$windows[2] = open_window;
$windows[0] = open_window;
$windows[3] = open_floating_window;
$windows[1] = open_floating_window;
focus_windows;
cmd '[id=' . $windows[1]->id . '] focus';
cmd '[id=' . $windows[0]->id . '] kill';
kill_and_confirm_focus($windows[2]->id, 'window 2 focused after tiling killed');
kill_and_confirm_focus($windows[3]->id, 'window 3 focused after tiling killed');
done_testing;