Containers nested in tabbed/stacked containers do not function properly.

agausmann commented 6 years ago

Steps to reproduce

(I have done these steps multiple times with various applications)

Open a blank workspace, set it to tabbed or stacked mode $mod+w/$mod+s - this will be called "parent container".
Open two windows, they should open as tabs
Create a container ("child container") from one of the windows by selecting it and choosing vertical/horizontal tiling, tabs, or stack $mod+v/$mod+b/$mod+w/$mod+s
(OPTIONAL) Open another window in the child container.

Problems

After performing the steps above, clicking on the child container's tab has no effect, and clicking in the title bar regions of the child container's windows (even when the child container is not in focus) will switch focus to that window.

I have also confirmed that this occurs with any number of tabs in the parent container. There can be any number of "child containers", they all will have this issue. The example above demonstrates the issue with more than 1 window in the child container, but 1 window also works fine.

Additional information

I am running Arch Linux Linux exams 4.13.12-1-ARCH #1 SMP PREEMPT Wed Nov 8 11:54:06 CET 2017 x86_64 GNU/Linux, using the officially distributed package for sway 0.15.0-1.

.config/sway/config

# Default config for sway
#
# Copy this to ~/.config/sway/config and edit it to your liking.
#
# Read `man 5 sway` for a complete reference.

### Variables
#
# Logo key. Use Mod1 for Alt.
set $mod Mod4
# Home row direction keys, like vim
set $left h
set $down j
set $up k
set $right l
# Your preferred terminal emulator
set $term termite
# Your preferred application launcher
set $menu dmenu_run
# Lock dialog
set $lock swaylock -c 000000

### Output configuration
#
# Default wallpaper (more resolutions are available in /usr/share/backgrounds/sway/)
output * bg /usr/share/backgrounds/sway/Sway_Wallpaper_Blue_1920x1080.png fill
#
# Example configuration:
#
#   output HDMI-A-1 resolution 1920x1080 position 1920,0
#
# You can get the names of your outputs by running: swaymsg -t get_outputs

### Input configuration
#
# Example configuration:
#
#   input "2:14:SynPS/2_Synaptics_TouchPad" {
#       dwt enabled
#       tap enabled
#       natural_scroll enabled
#       middle_emulation enabled
#   }
#
# You can get the names of your inputs by running: swaymsg -t get_inputs
# Read `man 5 sway-input` for more information about this section.
input "1267:12313:ELAN0501:00_04F3:3019_To" {
    accel_profile adaptive
    click_method clickfinger
    dwt enabled
    natural_scroll disabled
    scroll_method two_finger
    tap enabled
}

input "1578:22808:MOSART_Semi._2.4G_Keyboard" {
    accel_profile flat
    natural_scroll disabled
}

### Key bindings
#
# Basics:
#
    # start a terminal
    bindsym $mod+Return exec $term

    # kill focused window
    bindsym $mod+Shift+q kill

    # start your launcher
    bindsym $mod+d exec $menu

    # lock the screen
    bindsym $mod+Shift+p exec $lock

    # Drag floating windows by holding down $mod and left mouse button.
    # Resize them with right mouse button + $mod.
    # Despite the name, also works for non-floating windows.
    # Change normal to inverse to use left mouse button for resizing and right
    # mouse button for dragging.
    floating_modifier $mod normal

    # reload the configuration file
    bindsym $mod+Shift+c reload

    # exit sway (logs you out of your wayland session)
    bindsym $mod+Shift+e exit
#
# Moving around:
#
    # Move your focus around
    bindsym $mod+$left focus left
    bindsym $mod+$down focus down
    bindsym $mod+$up focus up
    bindsym $mod+$right focus right
    # or use $mod+[up|down|left|right]
    bindsym $mod+Left focus left
    bindsym $mod+Down focus down
    bindsym $mod+Up focus up
    bindsym $mod+Right focus right

    # _move_ the focused window with the same, but add Shift
    bindsym $mod+Shift+$left move left
    bindsym $mod+Shift+$down move down
    bindsym $mod+Shift+$up move up
    bindsym $mod+Shift+$right move right
    # ditto, with arrow keys
    bindsym $mod+Shift+Left move left
    bindsym $mod+Shift+Down move down
    bindsym $mod+Shift+Up move up
    bindsym $mod+Shift+Right move right
#
# Workspaces:
#
    # switch to workspace
    bindsym $mod+1 workspace 1
    bindsym $mod+2 workspace 2
    bindsym $mod+3 workspace 3
    bindsym $mod+4 workspace 4
    bindsym $mod+5 workspace 5
    bindsym $mod+6 workspace 6
    bindsym $mod+7 workspace 7
    bindsym $mod+8 workspace 8
    bindsym $mod+9 workspace 9
    bindsym $mod+0 workspace 10
    # move focused container to workspace
    bindsym $mod+Shift+1 move container to workspace 1
    bindsym $mod+Shift+2 move container to workspace 2
    bindsym $mod+Shift+3 move container to workspace 3
    bindsym $mod+Shift+4 move container to workspace 4
    bindsym $mod+Shift+5 move container to workspace 5
    bindsym $mod+Shift+6 move container to workspace 6
    bindsym $mod+Shift+7 move container to workspace 7
    bindsym $mod+Shift+8 move container to workspace 8
    bindsym $mod+Shift+9 move container to workspace 9
    bindsym $mod+Shift+0 move container to workspace 10
    # Note: workspaces can have any name you want, not just numbers.
    # We just use 1-10 as the default.
#
# Layout stuff:
#
    # You can "split" the current object of your focus with
    # $mod+b or $mod+v, for horizontal and vertical splits
    # respectively.
    bindsym $mod+b splith
    bindsym $mod+v splitv

    # Switch the current container between different layout styles
    bindsym $mod+s layout stacking
    bindsym $mod+w layout tabbed
    bindsym $mod+e layout toggle split

    # Make the current focus fullscreen
    bindsym $mod+f fullscreen

    # Toggle the current focus between tiling and floating mode
    bindsym $mod+Shift+space floating toggle

    # Swap focus between the tiling area and the floating area
    bindsym $mod+space focus mode_toggle

    # move focus to the parent container
    bindsym $mod+a focus parent
#
# Scratchpad:
#
    # Sway has a "scratchpad", which is a bag of holding for windows.
    # You can send windows there and get them back later.

    # Move the currently focused window to the scratchpad
    bindsym $mod+Shift+minus move scratchpad

    # Show the next scratchpad window or hide the focused scratchpad window.
    # If there are multiple scratchpad windows, this command cycles through them.
    bindsym $mod+minus scratchpad show
#
# Resizing containers:
#
mode "resize" {
    # left will shrink the containers width
    # right will grow the containers width
    # up will shrink the containers height
    # down will grow the containers height
    bindsym $left resize shrink width 10 px or 10 ppt
    bindsym $down resize grow height 10 px or 10 ppt
    bindsym $up resize shrink height 10 px or 10 ppt
    bindsym $right resize grow width 10 px or 10 ppt

    # ditto, with arrow keys
    bindsym Left resize shrink width 10 px or 10 ppt
    bindsym Down resize grow height 10 px or 10 ppt
    bindsym Up resize shrink height 10 px or 10 ppt
    bindsym Right resize grow width 10 px or 10 ppt

    # return to default mode
    bindsym Return mode "default"
    bindsym Escape mode "default"
}
bindsym $mod+r mode "resize"

#
# Status Bar:
#
# Read `man 5 sway-bar` for more information about this section.
bar {
    position bottom
    status_command i3status
    colors {
        statusline #ffffff
        background #323232
        inactive_workspace #32323200 #32323200 #5c5c5c
    }
}

include /etc/sway/config.d/*

agausmann commented 6 years ago

Unfortunately, I don't have enough knowledge of the code structure to track down where this is, and attempting to use git bisect reveals that my current version of wlc is not backwards-compatible... If someone else could help me find the offending lines, I would greatly appreciate it!

My guess is that either sway_container.visible is not checked in these circumstances, or it is not updated properly when the container goes out of focus inside of a stacked/tabbed container.

agausmann commented 6 years ago

I've spent some time studying the code and I think I found the place causing part of this issue. In the functionswayc_border_check(*c, *origin) (passed into container_find() by handle_pointer_button() where mouse clicks are used to update focus), there is no check to make sure c->visible == True.

UPDATE: I just looked at the code where focus changes based on mouse motion (which does work in the given scenario). It is handled by the following series of calls: handle_mouse_motion() -> pointer_position_set() -> container_under_pointer(). The last function there is actually what determines which container will receive focus, where it handles stacked and tabbed containers as special cases. It does not use the visible field at all, nor does it need to. It just unwraps the tab that is currently selected inside the container and continues execution.

Now I'm not sure whether visible is the correct field to use, however, I don't see any reason why it shouldn't be used in this case. Setting it for containers within tabs and stacks may actually solve additional issues that have not been found.

ddevault commented 6 years ago

Sounds like you're ready to submit a patch?

agausmann commented 6 years ago

Working on the patch now. Notes in the meantime:

I was mistaken, visible was already set correctly in the case of tabs by update_visibility(). Simply adding a check in swayc_border_check() resolves that issue.
Additional details on the other erroneus behavior that I had found: Tabbed containers with a container as a child will handle their entire tab bar as the tab of the focused window, whether it be a view or a container. Clicking anywhere on the tab bar only attempts to focus the currently focused child. Additionally, when focusing the child container, the container is focused and all children are updated to match (blue title bar). The parent container's tab color does not update to match its focused state. I suspect these issues have to do with the title bar's geometry set by update_geometry()

agausmann commented 6 years ago

Turns out the issue is far more complicated than a single line patch. Tabs which are out of focus have their visible flag set to false, meaning that clicking their title bars/tabs does not work. Title bar visibility is not the same as full container visibility. This may be fixed by checking the parent container's visibility instead.

agausmann commented 6 years ago

Okay, I'm submitting what I have right now; however, containers are far more broken than I've outlined here. The title bars are honestly completely screwed up, and I don't even know where to start with those.

ddevault commented 6 years ago

Yeah we'll be rewriting the whole title bar thing soon.

ddevault commented 6 years ago

I'm closing issues that are resolved by the yet-unreleased sway 1.0.

swaywm / sway