search

lxqt / lxqt-panel

The LXQt desktop panel
https://lxqt-project.org
GNU Lesser General Public License v2.1
193 stars 135 forks source link

Regression in desktop switcher with dynamic desktops of i3wm #2173

Closed ssipos90 closed 1 day ago

ssipos90 commented 2 days ago

Hi, guys.

Expected Behavior

The desktop switcher should reflect the current desktop.

Current Behavior

Since upgrading to version 2.1.1, the desktop switcher is wonky and doesn't reflect the current desktop in all situations, especially when moving away from an empty desktop. It seems that something's off with the dynamic desktops detection that i3wm uses.

Made a short video using screenkey showing the behaviour. I'm moving from desktop 1 (browser) to 2 (empty) then to 3 (terminal), but it doesn't reflect desktop 3. https://github.com/user-attachments/assets/55860edc-aabd-4985-9490-1299b40f32b9

Possible Solution

This is a regression, as it worked up until and including 2.0.1.

Steps to Reproduce (for bugs)

I think it's enough to have i3wm declare more than 5 desktops.

System Information
stefonarch commented 2 days ago

I never used i3, only sway sometimes. But I think I can confirm this issue. I see that I can only create one empty workspace. If I open an app on 1, another on 2 and then create 3 and open an app there and close the one on 2 when cycling the workspaces the switcher shows only 1 2 when I'm actually on 3 (omitting the empty 2 and 3 will be shown as 2).

There was a refactoring needed for wayland plugins, probably something has changed, other WM are fine. @gfgit any idea?

tsujan commented 2 days ago

But I think I can confirm this issue

What about other X11 WMs? Do they show this problem?

tsujan commented 2 days ago

other WM are fine.

But we don't treat WMs separately; we use KF6 for all. How can this be a new issue?

stefonarch commented 2 days ago

Afaik no other WM uses dynamic workspaces. I can't see this issue in openbox or xfwm4. @ssipos90 How can I declare 5 workspaces?

Could it be a KF6 update?

tsujan commented 2 days ago

What I wanted to know is just this: Did our switcher worked fine with dynamic workspaces before? If the answer is "yes", it gives motivation (to me, for example) for checking whether there's a real difference between our new and old X11 codes, because the last time I checked, there wasn't.

tsujan commented 2 days ago

After a quick comparison, I found a regression: the function DesktopSwitch::onDesktopNamesChanged() is never called, because we missed a connection to KX11Extras::desktopNamesChanged in lxqtwmbackend_x11.cpp.

I'll add that connection tomorrow, but it's about changing the desktop names, and I don't know if it can have any relation to this issue.

tsujan commented 2 days ago

I'm moving from desktop 1 (browser) to 2 (empty) then to 3 (terminal), but it doesn't reflect desktop 3.

Your video shows a different thing:

There's nothing wrong.

@stefonarch Your description doesn't show any problem either. You said, "omitting the empty 2 and 3 will be shown as 2". Well, it should because now it's the second desktop.

ssipos90 commented 1 day ago

I'll install the previous version and record a short video with the previous behaviour.

edit:

(desktop switcher is set to names, not numbers)

On v2.0.1 entering an empty/non existent workspace creates it on the fly, while leaving it removes it - this is what I mean by dynamic workspaces.

https://github.com/user-attachments/assets/15d8ade1-38a8-40e4-82b9-a151b4bdce5a

I'm going: 1 (browser) 2 (empty/non existent) 3 (terminal) 4 (empty/non existent) 5 (OBS) and 1, 2, 3 again

And this is how it behaves on v2.1.1:

https://github.com/user-attachments/assets/6cf0f81b-b1d7-49ee-b25a-f784692f3064

Minimal i3 config for this issue (@stefonarch here is how i3 declares workspaces):

set $mod Mod4 # set mod to super key
bindsym $mod+Return       exec --no-startup-id i3-sensible-terminal # super+enter starts a terminal

bindsym $mod+Shift+BackSpace   kill  #  super + shift + backspace kills what's in focus

bindsym $mod+d                 exec --no-startup-id rofi -show run # launch rofi run
bindsym $mod+f                 exec --no-startup-id rofi -show drun -show-icons # launch rofi drun

# declare workspace names
set $ws_browser  1
set $ws_browser2 2
set $ws_term     3
set $ws_aux      4
set $ws_aux2     5
set $ws_personal 6
set $ws7 7
set $ws8 8

# switch to workspace using super+1..8
bindsym $mod+1 workspace $ws_browser
bindsym $mod+2 workspace $ws_browser2
bindsym $mod+3 workspace $ws_term
bindsym $mod+4 workspace $ws_aux
bindsym $mod+5 workspace $ws_aux2
bindsym $mod+6 workspace $ws_personal
bindsym $mod+7 workspace $ws7
bindsym $mod+8 workspace $ws8
ssipos90 commented 1 day ago

Your video shows a different thing:

There were 5 desktops at first.

@tsujan unfortunately, no. The new panel version thinks that, but it's not the case as i3 deletes empty workspaces.

ssipos90 commented 1 day ago

Here is what $ xprop -root returns, compared to the panel toolbar.

lxqt-panel-3

tsujan commented 1 day ago

If i3wm explicitly sets the name of the second desktop to "3" after "2" is removed, then https://github.com/lxqt/lxqt-panel/pull/2174 should fix this.

tsujan commented 1 day ago

Oh, there's no regression, but just an understandable misunderstanding :)

I installed i3-wm from Arch and saw it (with https://github.com/lxqt/lxqt-panel/pull/2174, but I don't think it's relevant). There was a commit that "fixed" the value of an enum. Since V2.1.0, the switcher shows pure numbers by default. Just change "Desktop labels" to "Names", and everything will be OK.

ssipos90 commented 1 day ago

I'll install the previous version and record a short video with the previous behaviour.

edit:

(desktop switcher is set to names, not numbers)

I mentioned I'm using names, not numbers. It's not that.

edit: I get why you'd think that and it's because I followed the issue template asking for a minimal reproducible config of sorts, so I removed the icons for that. But I added reverted the config later on when I provided the screenshot of xprop.

tsujan commented 1 day ago

As I said, I tested it (with https://github.com/lxqt/lxqt-panel/pull/2174), and it was OK.

ssipos90 commented 1 day ago

Ah ok. I didn't properly follow the latest comments.

I just cloned and build from source from that branch and it works great.

Thank you!!

tsujan commented 1 day ago

Then I also misunderstood it, in a way.

You mean that only after using https://github.com/lxqt/lxqt-panel/pull/2174 the problem was fixed?

ssipos90 commented 1 day ago

yes. v2.1.1 doesn't work properly. I always, since day 1 had "names" set since I use icons.

tsujan commented 1 day ago

OK. So, actually fixing of the missing signal workspaceNameChanged did it, although I didn't hope that it would be relevant to this report.

Thanks for reporting and testing!

ssipos90 commented 1 day ago

you too for fixing!

tsujan commented 1 day ago

@stefonarch Do you think this needs a point release? If you say "yes", I'll make a point release for lxqt-wayland-session too — you know why ;)

stefonarch commented 1 day ago

Yes :)

tsujan commented 1 day ago

:)

Will make them tomorrow.