Which results in:
ERROR: AddressSanitizer: heap-use-after-free on address …
READ of size 8 at 0x614000001f48 thread T0
#0 0x5563df6e73a8 in init_ws_for_output i3/src/randr.c:468
#1 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
#2 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450
… is located 264 bytes inside of 448-byte region …
freed by thread T0 here:
#1 0x5563df634b0a in con_free i3/src/con.c:96
#2 0x5563df7151e6 in tree_close_internal i3/src/tree.c:344
#3 0x5563df7280fe in workspace_show i3/src/workspace.c:499
#4 0x5563df6e7315 in init_ws_for_output i3/src/randr.c:457
#5 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
#6 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450
tree_append_json: Allow strings that are not valid UTF8
Fixes #3156.
I couldn't reproduce the problem in a "natural" way so I cheated:
1. Start i3 with gdb
2. Set breakpoing on tree_restore
3. Run, open window, i3-msg restart
5. Open the file in *path with a hex editor
6. Edit the "name" field of the window and insert bytes that are not
valid UTF8
7. Continue
After parsing fails, all nodes including croot are incomplete, meaning
they have to be deleted. We can't recover in any reasonable way so we
have to allow non-UTF8 characters to avoid this situation altogether.
Orestis Floros [Thu, 23 Aug 2018 19:09:52 +0000 (22:09 +0300)]
Make cmd_resize_tiling_direction work with pixels
Introduces resize_neighboring_cons in resize.c which is also used by
resize_graphical_handler.
Co-authored-by: Andrew Laucius <andrewla@gmail.com>
Authored original code and tests in #3240. I rewrote most of the
resizing code and fixed the failing tests.
Orestis Floros [Wed, 22 Aug 2018 11:02:27 +0000 (14:02 +0300)]
Introduce con_get_fullscreen_covering_ws
This commit will also fix the following bugs:
1. click.c: Users could drag global fullscreen floating containers.
2. render.c: Floating containers would get rendered with a global fullscreen container in another
workspace.
Orestis Floros [Wed, 22 Aug 2018 00:40:51 +0000 (03:40 +0300)]
render_root: fix popup_during_fullscreen logic
The first issue is that there seems to be a typo: fullscreen->window
should have been child->window. The corrected check is redundant since
the while loop checks if the transient_con has a window.
The second issue is that popup_during_fullscreen is never checked even
though the behaviour should be exclusive to the "smart" option.
Orestis Floros [Tue, 21 Aug 2018 18:10:02 +0000 (21:10 +0300)]
Call dragloop callback on DRAG_SUCCESS
A race condition is possible. For example, if we first receive a
XCB_MOTION_NOTIFY event and then, while drain_drag_events is still
running, a XCB_BUTTON_RELEASE event the first event is never handled
because we return.
Orestis Floros [Tue, 21 Aug 2018 18:06:00 +0000 (21:06 +0300)]
floating_drag_window: return on DRAG_REVERT
Right now tree_render() is called twice on DRAG_REVERT since
floating_reposition calls it.
Also, on DRAG_REVERT the scratchpad state shouldn't change since the
user canceled the action.
Kill misbehaving subscribed clients instead of hanging
This change only affects clients that are subscribed to events, which
should be the main cause of our problems.
In the common case (no buffered data) the behaviour doesn't change at
all: the message is sent directly, no ev_io / ev_timeout callback is
enabled. Once a write to a client's socket is not completed fully
(returns with EAGAIN error), we put the message in the tail of a queue
and init an ev_io callback and a corresponding timer. If the timer is
triggered first, the socket is closed and the client connection is
removed. If the socket becomes writeable before the timeout we either
reset the timer if we couldn't push all the buffered data or completely
remove it if everything was pushed.
We could also replace ipc_send_message() for all client connections in
i3, not just those subscribed to events.
Furthermore, we could limit the amount of messages stored and increase
the timeout (or use multiple timeouts): eg it's ok if a client is not
reading for 10 seconds and we are only holding 5KB of messages for them
but it is not ok if they are inactive for 5 seconds and we have 30MB of
messages held.
This was introduced in 252db3b8c (#3245). That commit moved the
con_activate line inside the loop, meaning it wouldn't always be called.
When the mouse moved after a rename with focus_follows_mouse enabled,
check_crossing_screen_boundary (src/handlers.c:111) called
con_descend_focused that used the wrong focus order.
I also change con_activate to con_focus since we don't really want to
raise floating containers there.
Orestis Floros [Wed, 28 Mar 2018 01:05:48 +0000 (04:05 +0300)]
Use get_assigned_output for numbers
This prohibits the usage of workspaces assigned to other outputs in
create_workspace_on_output.
Eg, with config:
workspace 1 output fake-0
workspace 2 output fake-0
and 2 screens workspace 2 would be used for the second screen even
though it is assigned to the first one.
Also introduces a test for workspace assignments that includes the case
described above and some tests that don't fail in the next branch.
Orestis Floros [Wed, 28 Mar 2018 00:35:40 +0000 (03:35 +0300)]
Introduce get_assigned_output
This also replaces code in create_workspace_on_output() that is
theoretically more efficient but:
1. It isn't a huge difference since it depends on the number of outputs,
that shouldn't be high.
2. get_assigned_output will be modified and used for #555, then its
logic should be followed in create_workspace_on_output() too.
Another note for create_workspace_on_output: if assigned is not NULL the
condition (assigned != output->con) should never be false, ie if there
is an assigned output to this name, it isn't the current one. This
happens because the current callers check for assignments before calling
create_workspace_on_output().
Detectable autorepeat should only affect --release bindings. Currently,
when a user keeps a key pressed, we get multiple KeyPress and KeyRelease
events. With this change, we still get multiple KeyPress events, which
means that you can still keep a key pressed to repeatedly execute a
normal binding, but only one KeyRelease event when the key is physically
released.
Unfortunately, this change is not currently testable because detectable
autorepeat doesn't seem to work under Xephyr. AwesomeWM experienced the
same problem:
https://github.com/awesomeWM/awesome/commit/6f2424e90170be4acaa1d140f966ab6bb8a4d217
Orestis Floros [Sat, 2 Jun 2018 14:58:10 +0000 (17:58 +0300)]
docs: link workspace_auto_back_and_forth from workspace command
The current text is confusing. '--no-auto-back-and-forth' doesn't
disable the 'workspace back_and_forth' command, the flag is not even
valid for that command.
Dan Elkouby [Fri, 1 Jun 2018 15:55:35 +0000 (18:55 +0300)]
Reframe swallowed windows if depth doesn't match
X will not allow a window with ParentRelative background to be created
or reparented under a window with mismatching color depth.
Deal with this by destroying the container frame and creating a new one
with the right depth upon swallowing.
Defer destruction of the frame window until after the updated tree has
been rendered to avoid some distracting flickering.
Orestis Floros [Tue, 8 May 2018 14:05:46 +0000 (17:05 +0300)]
Replace strncpy call with memcpy when result is not NUL-terminated
This fixes a new warning from GCC 8.1, -Wstringop-truncation:
https://gcc.gnu.org/gcc-8/changes.html
https://gcc.gnu.org/onlinedocs/gcc-8.1.0/gcc/Warning-Options.html#index-Wstringop-truncation
Replacing with memcpy is what gcc suggests:
> As another example, the following call to strncpy results in copying
> to d just the characters preceding the terminating NUL, without
> appending the NUL to the end. Assuming the result of strncpy is
> necessarily a NUL-terminated string is a common mistake, and so the
> call is diagnosed. To avoid the warning when the result is not
> expected to be NUL-terminated, call memcpy instead.
> void copy (char *d, const char *s)
> {
> strncpy (d, s, strlen (s));
> }
Dan Elkouby [Sat, 5 May 2018 10:30:48 +0000 (13:30 +0300)]
Activate the focused child when scrolling over tab/stack decorations
fbce834b introduced a bug where scrolling over the decoration while
another container is focused would not focus the tabbed/stacked
container itself, but would instead move focus through the currently
focused container.