Properly focus container when it changes parent

[WhyNotHugo]

When a parented container changes parent, sway will render a sibling non-focused container.

Revert deletion of code which introduced this regression.

Fixes: 5e18ed3
Fixes: #8292

[WhyNotHugo]

Kind bump? This is a really annoying regression since 1.10.

[Nefsen402]

It seems this behavior is intended by the "regression" PR, which claims that this behavior is correct for i3.

[WhyNotHugo]

This was not reproducible on i3 last I checked.

Have you reproduced the bug yourself? When moving windows through a tabbed container, sway will at some point render another window, while indicating that the one being moved is still focused.

The commit that broke this says:

My code archaeology isn't good enough to determine what this is here for, but it isn't correct.

I've found why this code is here, if fixes this particular issue. Maybe i3 doesn't need it because this corner case is handled elsewhere, I have no idea.

[WhyNotHugo]

Here's a video reproducing the behaviour:

https://mirror.whynothugo.nl/t/sway-issue-8292.mkv

I start out with a tabbed container with:

• One terminal (1)
• A horizontal split with a single terminal (2)
• One terminal (3)
• One terminal (4)
• One terminal (wf-record)

I focus terminal (4), and move it left two times. It is now inside the horizontal split, sway marks the horizontal split as focused, but renders the surface from the wf-record window.

I then type swaymsg move left. You can't see me type this, but it gets typed into the window that isn't being rendered (e.g.: it's not visible but still has keyboard focus). You can see it appear again after I move it left two more times. The first time it is moved to the left side of the horizontal split, the second time it is moved out of the split.

[Nefsen402]

Huh, yeah this is definitely a real issue. I find it strange that it doesn't render anything at all, focusing/unfocusing shouldn't affect rendering to this extent.

[kennylevinsen]

Huh, yeah this is definitely a real issue. I find it strange that it doesn't render anything at all, focusing/unfocusing shouldn't affect rendering to this extent.

When you just move plain left, no focus or render state is changed - only the parent's title updates. The issue is that a container is moved from being a direct child of its parent to being a grandchild.

sway/sway/commands/move.c

Lines 125 to 134 in 962e1e7

	sway_log(SWAY_DEBUG, "Promoting to sibling of cousin");
	int offset =
	move_dir == WLR_DIRECTION_LEFT || move_dir == WLR_DIRECTION_UP;
	int index = container_sibling_index(destination) + offset;
	if (destination->pending.parent) {
	container_insert_child(destination->pending.parent, container, index);
	} else {
	workspace_insert_tiling(destination->pending.workspace,
	container, index);
	}

container_insert_child will amongst other things detach the container from the current parent, which causes the parent to pick its last child to render instead. It does not do anything to make the inserted container visible.

What we need to do in this case is change the container state to match the new parent, instead of being reset to the random child. This would also have to apply to the two reparenting scenarios below this one.

I'm not sure if i3 has any caveats if, say, you're moving unfocused containers - maybe visibility only changes if the container moved was initially visible or focused. The commit removing the workaround suggests we were doing too much refocusing compared to i3 at least. It's better to be functional than to be correct though.

[WhyNotHugo]

container_insert_child will amongst other things detach the container from the current parent, which causes the parent to pick its last child to render instead. It does not do anything to make the inserted container visible.

What determines which child is chosen to render? The focus seems to remain correct (I can still keyboard interact with the non-visible window).

I considered forcing focus for the new parent if the child was already focused, but we don't even have a seat in scope.