From 0845d7b264fdd8aa65b0d56c100ed716db06c63a Mon Sep 17 00:00:00 2001 From: izzel <34325651+izzel@users.noreply.github.com> Date: Tue, 13 Aug 2019 02:50:48 -0400 Subject: [PATCH] Remanage window after property updates (like titles) (#3759) --- docs/layout-saving | 24 ----- include/con.h | 8 ++ include/data.h | 4 + include/manage.h | 7 ++ include/startup.h | 6 ++ include/window.h | 8 +- src/con.c | 58 ++++++++---- src/handlers.c | 16 +++- src/manage.c | 107 ++++++++++++++++++---- src/startup.c | 19 ++++ src/window.c | 20 +--- testcases/t/542-layout-restore-remanage.t | 86 +++++++++++++++++ 12 files changed, 279 insertions(+), 84 deletions(-) create mode 100644 testcases/t/542-layout-restore-remanage.t diff --git a/docs/layout-saving b/docs/layout-saving index f31b5e21..4f0ffccf 100644 --- a/docs/layout-saving +++ b/docs/layout-saving @@ -261,27 +261,3 @@ container: ] } -------------------------------------------------------------------------------- - -=== Placeholders using window title matches don't swallow the window - -If you use the +title+ attribute to match a window and find that it doesn't -work or only works sometimes, the reason might be that the application sets the -title only after making the window visible. This will be especially true for -programs running inside terminal emulators, e.g., +urxvt -e irssi+ when -matching on +title: "irssi"+. - -One way to deal with this is to not rely on the title, but instead use, e.g., -the +instance+ attribute and running the program to set this window instance to -that value: - --------------------------------------------------------------------------------- -# Run irssi via -# urxvt -name "irssi-container" -e irssi - -"swallows": [ - { - "class": "URxvt", - "instance": "irssi-container" - } -] --------------------------------------------------------------------------------- diff --git a/include/con.h b/include/con.h index 09ec7b44..2d843eeb 100644 --- a/include/con.h +++ b/include/con.h @@ -533,3 +533,11 @@ bool con_swap(Con *first, Con *second); * */ uint32_t con_rect_size_in_orientation(Con *con); + +/** + * Merges container specific data that should move with the window (e.g. marks, + * title format, and the window itself) into another container, and closes the + * old container. + * + */ +void con_merge_into(Con *old, Con *new); diff --git a/include/data.h b/include/data.h index c3cada37..4c6ecb8c 100644 --- a/include/data.h +++ b/include/data.h @@ -489,6 +489,10 @@ struct Window { bool shaped; /** The window has a nonrectangular input shape. */ bool input_shaped; + + /* Time when the window became managed. Used to determine whether a window + * should be swallowed after initial management. */ + time_t managed_since; }; /** diff --git a/include/manage.h b/include/manage.h index 55b0a85b..22fbe527 100644 --- a/include/manage.h +++ b/include/manage.h @@ -37,3 +37,10 @@ void restore_geometry(void); void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cookie, bool needs_to_be_mapped); + +/** + * Remanages a window: performs a swallow check and runs assignments. + * Returns con for the window regardless if it updated. + * + */ +Con *remanage_window(Con *con); diff --git a/include/startup.h b/include/startup.h index feece575..0001a77d 100644 --- a/include/startup.h +++ b/include/startup.h @@ -69,3 +69,9 @@ struct Startup_Sequence *startup_sequence_get(i3Window *cwindow, * */ char *startup_workspace_for_window(i3Window *cwindow, xcb_get_property_reply_t *startup_id_reply); + +/** + * Deletes the startup sequence for a window if it exists. + * + */ +void startup_sequence_delete_by_window(i3Window *win); diff --git a/include/window.h b/include/window.h index b03f9c14..6673e835 100644 --- a/include/window.h +++ b/include/window.h @@ -22,14 +22,14 @@ void window_free(i3Window *win); * given window. * */ -void window_update_class(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt); +void window_update_class(i3Window *win, xcb_get_property_reply_t *prop); /** * Updates the name by using _NET_WM_NAME (encoded in UTF-8) for the given * window. Further updates using window_update_name_legacy will be ignored. * */ -void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt); +void window_update_name(i3Window *win, xcb_get_property_reply_t *prop); /** * Updates the name by using WM_NAME (encoded in COMPOUND_TEXT). We do not @@ -38,7 +38,7 @@ void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool befo * window_update_name()). * */ -void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt); +void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop); /** * Updates the CLIENT_LEADER (logical parent window). @@ -62,7 +62,7 @@ void window_update_strut_partial(i3Window *win, xcb_get_property_reply_t *prop); * Updates the WM_WINDOW_ROLE * */ -void window_update_role(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt); +void window_update_role(i3Window *win, xcb_get_property_reply_t *prop); /** * Updates the _NET_WM_WINDOW_TYPE property. diff --git a/src/con.c b/src/con.c index 10afb4cc..52c78d0c 100644 --- a/src/con.c +++ b/src/con.c @@ -1258,34 +1258,17 @@ static bool _con_move_to_con(Con *con, Con *target, bool behind_focused, bool fi /* 8. If anything within the container is associated with a startup sequence, * delete it so child windows won't be created on the old workspace. */ - struct Startup_Sequence *sequence; - xcb_get_property_cookie_t cookie; - xcb_get_property_reply_t *startup_id_reply; - if (!con_is_leaf(con)) { Con *child; TAILQ_FOREACH(child, &(con->nodes_head), nodes) { if (!child->window) continue; - - cookie = xcb_get_property(conn, false, child->window->id, - A__NET_STARTUP_ID, XCB_GET_PROPERTY_TYPE_ANY, 0, 512); - startup_id_reply = xcb_get_property_reply(conn, cookie, NULL); - - sequence = startup_sequence_get(child->window, startup_id_reply, true); - if (sequence != NULL) - startup_sequence_delete(sequence); + startup_sequence_delete_by_window(child->window); } } if (con->window) { - cookie = xcb_get_property(conn, false, con->window->id, - A__NET_STARTUP_ID, XCB_GET_PROPERTY_TYPE_ANY, 0, 512); - startup_id_reply = xcb_get_property_reply(conn, cookie, NULL); - - sequence = startup_sequence_get(con->window, startup_id_reply, true); - if (sequence != NULL) - startup_sequence_delete(sequence); + startup_sequence_delete_by_window(con->window); } /* 9. If the container was marked urgent, move the urgency hint. */ @@ -2401,3 +2384,40 @@ bool con_swap(Con *first, Con *second) { uint32_t con_rect_size_in_orientation(Con *con) { return (con_orientation(con) == HORIZ ? con->rect.width : con->rect.height); } + +/* + * Merges container specific data that should move with the window (e.g. marks, + * title format, and the window itself) into another container, and closes the + * old container. + * + */ +void con_merge_into(Con *old, Con *new) { + new->window = old->window; + old->window = NULL; + + if (old->title_format) { + FREE(new->title_format); + new->title_format = old->title_format; + old->title_format = NULL; + } + + if (old->sticky_group) { + FREE(new->sticky_group); + new->sticky_group = old->sticky_group; + old->sticky_group = NULL; + } + + new->sticky = old->sticky; + + con_set_urgency(new, old->urgent); + + mark_t *mark; + TAILQ_FOREACH(mark, &(old->marks_head), marks) { + TAILQ_INSERT_TAIL(&(new->marks_head), mark, marks); + ipc_send_window_event("mark", new); + } + new->mark_changed = (TAILQ_FIRST(&(old->marks_head)) != NULL); + TAILQ_INIT(&(old->marks_head)); + + tree_close_internal(old, DONT_KILL_WINDOW, false); +} diff --git a/src/handlers.c b/src/handlers.c index ae42b82e..7a334978 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -565,7 +565,9 @@ static bool handle_windowname_change(void *data, xcb_connection_t *conn, uint8_t char *old_name = (con->window->name != NULL ? sstrdup(i3string_as_utf8(con->window->name)) : NULL); - window_update_name(con->window, prop, false); + window_update_name(con->window, prop); + + con = remanage_window(con); x_push_changes(croot); @@ -590,7 +592,9 @@ static bool handle_windowname_change_legacy(void *data, xcb_connection_t *conn, char *old_name = (con->window->name != NULL ? sstrdup(i3string_as_utf8(con->window->name)) : NULL); - window_update_name_legacy(con->window, prop, false); + window_update_name_legacy(con->window, prop); + + con = remanage_window(con); x_push_changes(croot); @@ -612,7 +616,9 @@ static bool handle_windowrole_change(void *data, xcb_connection_t *conn, uint8_t if ((con = con_by_window_id(window)) == NULL || con->window == NULL) return false; - window_update_role(con->window, prop, false); + window_update_role(con->window, prop); + + con = remanage_window(con); return true; } @@ -1158,7 +1164,9 @@ static bool handle_class_change(void *data, xcb_connection_t *conn, uint8_t stat return false; } - window_update_class(con->window, prop, false); + window_update_class(con->window, prop); + + con = remanage_window(con); return true; } diff --git a/src/manage.c b/src/manage.c index 80faa167..df0cd934 100644 --- a/src/manage.c +++ b/src/manage.c @@ -13,6 +13,34 @@ #include +/* + * Match frame and window depth. This is needed because X will refuse to reparent a + * window whose background is ParentRelative under a window with a different depth. + * + */ +static xcb_window_t _match_depth(i3Window *win, Con *con) { + xcb_window_t old_frame = XCB_NONE; + if (con->depth != win->depth) { + old_frame = con->frame.id; + con->depth = win->depth; + x_con_reframe(con); + } + return old_frame; +} + +/* + * Remove all match criteria, the first swallowed window wins. + * + */ +static void _remove_matches(Con *con) { + while (!TAILQ_EMPTY(&(con->swallow_head))) { + Match *first = TAILQ_FIRST(&(con->swallow_head)); + TAILQ_REMOVE(&(con->swallow_head), first, matches); + match_free(first); + free(first); + } +} + /* * Go through all existing windows (if the window manager is restarted) and manage them * @@ -174,13 +202,13 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki FREE(buttons); /* update as much information as possible so far (some replies may be NULL) */ - window_update_class(cwindow, xcb_get_property_reply(conn, class_cookie, NULL), true); - window_update_name_legacy(cwindow, xcb_get_property_reply(conn, title_cookie, NULL), true); - window_update_name(cwindow, xcb_get_property_reply(conn, utf8_title_cookie, NULL), true); + window_update_class(cwindow, xcb_get_property_reply(conn, class_cookie, NULL)); + window_update_name_legacy(cwindow, xcb_get_property_reply(conn, title_cookie, NULL)); + window_update_name(cwindow, xcb_get_property_reply(conn, utf8_title_cookie, NULL)); window_update_leader(cwindow, xcb_get_property_reply(conn, leader_cookie, NULL)); window_update_transient_for(cwindow, xcb_get_property_reply(conn, transient_cookie, NULL)); window_update_strut_partial(cwindow, xcb_get_property_reply(conn, strut_cookie, NULL)); - window_update_role(cwindow, xcb_get_property_reply(conn, role_cookie, NULL), true); + window_update_role(cwindow, xcb_get_property_reply(conn, role_cookie, NULL)); bool urgency_hint; window_update_hints(cwindow, xcb_get_property_reply(conn, wm_hints_cookie, NULL), &urgency_hint); border_style_t motif_border_style = BS_NORMAL; @@ -341,24 +369,13 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki DLOG("Uh?! Container without a placeholder, but with a window, has swallowed this to-be-managed window?!\n"); } else { /* Remove remaining criteria, the first swallowed window wins. */ - while (!TAILQ_EMPTY(&(nc->swallow_head))) { - Match *first = TAILQ_FIRST(&(nc->swallow_head)); - TAILQ_REMOVE(&(nc->swallow_head), first, matches); - match_free(first); - free(first); - } + _remove_matches(nc); } } xcb_window_t old_frame = XCB_NONE; if (nc->window != cwindow && nc->window != NULL) { window_free(nc->window); - /* Match frame and window depth. This is needed because X will refuse to reparent a - * window whose background is ParentRelative under a window with a different depth. */ - if (nc->depth != cwindow->depth) { - old_frame = nc->frame.id; - nc->depth = cwindow->depth; - x_con_reframe(nc); - } + old_frame = _match_depth(cwindow, nc); } nc->window = cwindow; x_reinit(nc); @@ -594,6 +611,8 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki } render_con(croot); + cwindow->managed_since = time(NULL); + /* Send an event about window creation */ ipc_send_window_event("new", nc); @@ -670,3 +689,57 @@ geom_out: out: free(attr); } + +/* + * Remanages a window: performs a swallow check and runs assignments. + * Returns con for the window regardless if it updated. + * + */ +Con *remanage_window(Con *con) { + Match *match; + Con *nc = con_for_window(croot, con->window, &match); + if (nc == NULL || nc->window == con->window) { + run_assignments(con->window); + return con; + } + /* Make sure the placeholder that wants to swallow this window didn't spawn + * after the window to follow current behavior: adding a placeholder won't + * swallow windows currently managed. */ + if (nc->window->managed_since > con->window->managed_since) { + run_assignments(con->window); + return con; + } + + if (!restore_kill_placeholder(nc->window->id)) { + DLOG("Uh?! Container without a placeholder, but with a window, has swallowed this managed window?!\n"); + } else { + _remove_matches(nc); + } + window_free(nc->window); + + xcb_window_t old_frame = _match_depth(con->window, nc); + + x_reparent_child(nc, con); + + bool moved_workpaces = (con_get_workspace(nc) != con_get_workspace(con)); + + con_merge_into(con, nc); + + /* Destroy the old frame if we had to reframe the container. This needs to be done + * after rendering in order to prevent the background from flickering in its place. */ + if (old_frame != XCB_NONE) { + xcb_destroy_window(conn, old_frame); + } + + run_assignments(nc->window); + + if (moved_workpaces) { + /* If the window is associated with a startup sequence, delete it so + * child windows won't be created on the old workspace. */ + startup_sequence_delete_by_window(nc->window); + + ewmh_update_wm_desktop(); + } + + return nc; +} diff --git a/src/startup.c b/src/startup.c index c5b7ad5d..45d4673e 100644 --- a/src/startup.c +++ b/src/startup.c @@ -365,3 +365,22 @@ char *startup_workspace_for_window(i3Window *cwindow, xcb_get_property_reply_t * return sequence->workspace; } + +/* + * Deletes the startup sequence for a window if it exists. + * + */ +void startup_sequence_delete_by_window(i3Window *win) { + struct Startup_Sequence *sequence; + xcb_get_property_cookie_t cookie; + xcb_get_property_reply_t *startup_id_reply; + + cookie = xcb_get_property(conn, false, win->id, A__NET_STARTUP_ID, + XCB_GET_PROPERTY_TYPE_ANY, 0, 512); + startup_id_reply = xcb_get_property_reply(conn, cookie, NULL); + + sequence = startup_sequence_get(win, startup_id_reply, true); + if (sequence != NULL) { + startup_sequence_delete(sequence); + } +} diff --git a/src/window.c b/src/window.c index 8c3ae850..369aaa96 100644 --- a/src/window.c +++ b/src/window.c @@ -26,7 +26,7 @@ void window_free(i3Window *win) { * given window. * */ -void window_update_class(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt) { +void window_update_class(i3Window *win, xcb_get_property_reply_t *prop) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("WM_CLASS not set.\n"); FREE(prop); @@ -52,9 +52,6 @@ void window_update_class(i3Window *win, xcb_get_property_reply_t *prop, bool bef win->class_instance, win->class_class); free(prop); - if (!before_mgmt) { - run_assignments(win); - } } /* @@ -62,7 +59,7 @@ void window_update_class(i3Window *win, xcb_get_property_reply_t *prop, bool bef * window. Further updates using window_update_name_legacy will be ignored. * */ -void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt) { +void window_update_name(i3Window *win, xcb_get_property_reply_t *prop) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("_NET_WM_NAME not specified, not changing\n"); FREE(prop); @@ -89,9 +86,6 @@ void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool befo win->uses_net_wm_name = true; free(prop); - if (!before_mgmt) { - run_assignments(win); - } } /* @@ -101,7 +95,7 @@ void window_update_name(i3Window *win, xcb_get_property_reply_t *prop, bool befo * window_update_name()). * */ -void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt) { +void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("WM_NAME not set (_NET_WM_NAME is what you want anyways).\n"); FREE(prop); @@ -134,9 +128,6 @@ void window_update_name_legacy(i3Window *win, xcb_get_property_reply_t *prop, bo win->name_x_changed = true; free(prop); - if (!before_mgmt) { - run_assignments(win); - } } /* @@ -218,7 +209,7 @@ void window_update_strut_partial(i3Window *win, xcb_get_property_reply_t *prop) * Updates the WM_WINDOW_ROLE * */ -void window_update_role(i3Window *win, xcb_get_property_reply_t *prop, bool before_mgmt) { +void window_update_role(i3Window *win, xcb_get_property_reply_t *prop) { if (prop == NULL || xcb_get_property_value_length(prop) == 0) { DLOG("WM_WINDOW_ROLE not set.\n"); FREE(prop); @@ -233,9 +224,6 @@ void window_update_role(i3Window *win, xcb_get_property_reply_t *prop, bool befo LOG("WM_WINDOW_ROLE changed to \"%s\"\n", win->role); free(prop); - if (!before_mgmt) { - run_assignments(win); - } } /* diff --git a/testcases/t/542-layout-restore-remanage.t b/testcases/t/542-layout-restore-remanage.t new file mode 100644 index 00000000..26b50835 --- /dev/null +++ b/testcases/t/542-layout-restore-remanage.t @@ -0,0 +1,86 @@ +#!perl +# vim:ts=4:sw=4:expandtab +# +# Please read the following documents before working on tests: +# • https://build.i3wm.org/docs/testsuite.html +# (or docs/testsuite) +# +# • https://build.i3wm.org/docs/lib-i3test.html +# (alternatively: perldoc ./testcases/lib/i3test.pm) +# +# • https://build.i3wm.org/docs/ipc.html +# (or docs/ipc) +# +# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf +# (unless you are already familiar with Perl) +# +# Tests that swallowing still works after a window gets managed and its property +# updated. +use i3test; +use File::Temp qw(tempfile); +use IO::Handle; +use X11::XCB qw(PROP_MODE_REPLACE); + +sub change_window_title { + my ($window, $title, $length) = @_; + my $atomname = $x->atom(name => '_NET_WM_NAME'); + my $atomtype = $x->atom(name => 'UTF8_STRING'); + $length ||= length($title) + 1; + $x->change_property( + PROP_MODE_REPLACE, + $window->id, + $atomname->id, + $atomtype->id, + 8, + $length, + $title + ); + sync_with_i3; +} + +my $ws = fresh_workspace; + +my @content = @{get_ws_content($ws)}; +is(@content, 0, 'no nodes on the new workspace yet'); + +my ($fh, $filename) = tempfile(UNLINK => 1); +print $fh <flush; +cmd "append_layout $filename"; + +@content = @{get_ws_content($ws)}; +is(@content, 1, 'one node on the workspace now'); + +my $window = open_window( + name => 'original_title', + wm_class => 'a', +); + +@content = @{get_ws_content($ws)}; +is(@content, 2, 'two nodes on the workspace now'); + +change_window_title($window, "different_title"); + +does_i3_live; + +@content = @{get_ws_content($ws)}; +my @nodes = @{$content[0]->{nodes}}; +is(@content, 1, 'only one node on the workspace now'); +is($nodes[0]->{name}, 'different_title', 'test window got swallowed'); + +close($fh); + +done_testing;