Keyboard focus on panel menus

[stefonarch]

In v. 0.6.5 at least up/down arrows were working in lxqt-panel (wayland_taskbar PR) menus, as seen in KDE Neon. Only compositor where this is working is kwin_wayland atm.

• impossible to type in search fields
• no keyboard navigation

[spl237]

I've had to work around this in the panel we use on Raspberry Pi - it seems to be a common problem with Wayland compositors. Wayfire is the same.

The crude fix I have used in the panel code is to apply keyboard focus to the panel immediately before opening a menu, and then to remove focus from the panel once the menu closes - menus popped up from a panel with keyboard focus also get keyboard focus. I couldn't find any other way to do this.

https://github.com/raspberrypi-ui/wf-panel-pi/blob/master/src/panel/lxutils.c#L85

[johanmalm]

-- EDIT -- Will keep updating this as the thinking develops.

Okay, I think we've got a few things to research and get right with layer-shell surfaces. It'll probably be easiest to tackle them in small chunks, and probably from top to bottom. However, I thought it would be good to post my current thinking here in one hit so that others can help shape the direction.

• [x] 1. Process sub-surfaces on cursor-button-press so that focus is given to client if on-demand/exclusive is set.

  • 1594

• [ ] 2. Grant exclusive/on-demand interactivity regardless of layer. We currently only grant it top/overlay layers. I think for panels/desktop, the background/bottom are just as important in this regard. The protocol says that "For the bottom and background layers, the compositor is allowed to use normal focus semantics." I don't think we even do that, so need to fix. Shouldn't be hard.

• [ ] 3. Make process_keyboard_interactivity() public and call it from src/input/cursor.c

4. Should keyboard-focus be given to layer-shell popups on creation. I'd never thought about that, and the protocol doesn't specify. Based on what you've said about Qt6 on other compositors, I don't suspect anyone else does it either. Feels like we should though. Can't think of any cases where a popup does not want keyboard-interactivity (although if there is, we might be breaking user-space). I suppose the 'status-quo' alternative is to leave as is and 'demand' the interactivity when opening popups, but it'd be nice if clients didn't have to. I've had a quick play with labwc to grab keyboard focus on popup first commit, but can't get it to work. Can draft PR if anyone wants to help. See: https://github.com/labwc/labwc/issues/1572#issuecomment-1981815408

• [ ] 5. When interactivity is taken (e.g. by bemenu) should we close open popups? For example, with a GTK3 panel popup menu open (with force exclusive interactivity), if you open bemenu the popup remains open and focus stays with the panel-popup. If you click anywhere else, popups close so it seems odd that when we really DO want to take keyboard-focus for a layer-shell exclusive client, that we let a popup remain open.

• [ ] 6. When dealing with multiple interactive layer-shell surfaces (from different clients), when the currently focused one unmaps/destroys we (and sway I think) currently just unset focus. We should probably iterate over all excl/on-demand clients and give it to the one with highest precedence (e.g. excl-overlay-to-background, then on-demand-overlay-to-background).

• [ ] 7. Support giving the Desktop (background layer-shell client with on-demand interactivity?) keyboard focus once they close all windows or switches to a workspace without any open window (as in X11). Currently Desktop needs to be clicked first to get focus.

[Consolatis]

Might update this post later with more thoughts.

For on-demand I think we should only ever give focus automatically during map to keep it the same as for usual xdg and xwayland surfaces. In my understanding on-demand just means "apply usual focus semantics", e.g. focus on map and on cursor button press. So for 4. excl-overlay-to-background sounds fine but I am not sure about on-demand-overlay-to-background. If there is no exclusive candidate, the next focus should either be on some remaining layer surface with on-demand set or on some xdg / xwayland surface. But we don't have a combined history of the last keyboard focused layershell / xdg / xwayland surface so not sure how to handle this properly.

[stefonarch]

2) Should keyboard-focus be given to layer-shell popups on creation. I'd never thought about that, and the protocol doesn't specify. Based on what you've said about Qt6 on other compositors, I don't suspect anyone else does it either.

Only with Hyprland and kwin keyboard navigation in lxqt-panel's menus is working atm.

[Consolatis]

That might also be relevant here: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/main/stable/xdg-shell/xdg-shell.xml?ref_type=heads#L1244 As well as this: https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/blob/master/unstable/wlr-layer-shell-unstable-v1.xml?ref_type=heads#L275

[johanmalm]

As well as this: https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/blob/master/unstable/wlr-layer-shell-unstable-v1.xml?ref_type=heads#L275

Good spot. That links says "This setting is inherited by child surfaces set by the get_popup request." I read that as the client should ask for interactivity when opening the popup.

@stefonarch I'll take a look at what hyprland have done later.

[johanmalm]

I've got it working with tint test panel by setting keyboard-interactivity to on-demand and by applying the patch below.

@Consolatis There is something weird going on here. ctx->type is never LAB_SSD_LAYER_SURFACE but always LAB_SSD_LAYER_SUBSURFACE. Have even tried with a home made thing that doesn't do sub-surfaces just to make sure it's not a Gtk/Qt thing.

I feel I want to understand and fix this first before patching src/layers.c

diff --git a/src/input/cursor.c b/src/input/cursor.c
index b06a9ca..d34b29e 100644
--- a/src/input/cursor.c
+++ b/src/input/cursor.c
@@ -964,7 +964,7 @@ cursor_button_press(struct seat *seat, uint32_t button,
     * Action processing does not run for these surfaces and thus
     * the Focus action (used for normal views) does not work.
     */
-   if (ctx.type == LAB_SSD_LAYER_SURFACE) {
+   if (ctx.type == LAB_SSD_LAYER_SUBSURFACE) {
        struct wlr_layer_surface_v1 *layer =
            wlr_layer_surface_v1_try_from_wlr_surface(ctx.surface);
        if (layer && layer->current.keyboard_interactive) {

[spl237]

It might be worth checking my original report of this on the Wayfire repo at https://github.com/WayfireWM/wayfire/issues/1779

I found that setting interactivity to on_demand didn't completely fix the problem - it did mean that you got keyboard control of menus if they were opened with the mouse, but you didn't if they were opened by a keyboard shortcut; it was a mouse click which made the necessary demand, and if you didn't mouse-click to open the menu, you still didn't get keyboard control while in it - does your patch fix that?

[stefonarch]

@johanmalm Applied your patch, with lxqt-panel no diff, still unable to use the search in menu or any keyboard navigation. Compiling the panel should be easier in some days when basic stuff is merged.

[stefonarch]

Oh - there is a diff: on the desktop keyboard navigation, delete button and tooltips are working now. Tooltips appear only if desktop is clicked once first. Also here a PR for pcmanfm-qt wayland should come soon.

[johanmalm]

I found that setting interactivity to on_demand didn't completely fix the problem - it did mean that you got keyboard control of menus if they were opened with the mouse, but you didn't if they were opened by a keyboard shortcut; it was a mouse click which made the necessary demand, and if you didn't mouse-click to open the menu, you still didn't get keyboard control while in it - does your patch fix that?

I hadn't thought about the keyboard driven approach. No, the patch doesn't address that.

Three potential options:

1. Activation via xdg-desktop-portal (although that might be the wrong approach because I think we want keyboard-focus rather than activation - and taking activation away for toplevel window may have unintended side effects).
2. Add a FocusLayerClient action - but that's not an elegant solutions.

Hmm... that one needs thinking about. Not obvious to me how we best solve that. Will focus on the other focus semantics for the minute.

[johanmalm]

1594 is a step on the journey. Testing with the various panels/setup you have would be appreciated. Note that it doesn't address clients in the bottom/background layers yet. Will do that as a follow-up.

[tsujan]

I tried https://github.com/labwc/labwc/issues/1572#issuecomment-1981905534 for pcmanfm-qt's desktop + https://github.com/lxqt/pcmanfm-qt/pull/1876 (don't have lxqt-panel). With that patch, Desktop gets keyboard focus once clicked — keyboard navigation and activation work fine. For some reason, labwc's title-bar doesn't reflect the focus change on clicking Desktop but, as far as Qt is concerned, the focus is changed (as expected).

The only problem is that, the user expects that Desktop should get the keyboard focus once he closes all windows or switches to a workspace without any open window (as in X11), but that doesn't happen: Desktop needs to be clicked first to get focus.

[johanmalm]

Thanks for testing.

The only problem is that, the user expects that Desktop should get the keyboard focus once he closes all windows or switches to a workspace without any open window (as in X11), but that doesn't happen: Desktop needs to be clicked first to get focus.

Interesting point. I'd never thought about that. This probably stems from the lack a _NET_WM_WINDOW_TYPE_DESKTOP on Wayland.

One solution would be to give the first background layer-shell client with on-demand keyboard interactivity the focus when there are no toplevels left. Probably here: https://github.com/labwc/labwc/blob/be37f9a564f23e562b990394a12620297c7ce378/src/desktop.c#L171

[johanmalm]

@tsujan I've done a quick 'idea-PR'. Have just pushed it so we don't forget + RFC. https://github.com/labwc/labwc/pull/1598

[tsujan]

Maybe I did it too soon (considering your comment about "Need couple of other patches..."), but I compiled your branch https://github.com/labwc/labwc/pull/1598 after applying the patch at https://github.com/labwc/labwc/issues/1572#issuecomment-1981905534. Desktop wasn't focused on closing all windows or switching to a workspace without window.

[johanmalm]

Maybe I did it too soon (considering your comment about "Need couple of other patches..."), but I compiled your branch #1598 after applying the patch at #1572 (comment). Desktop wasn't focused on closing all windows or switching to a workspace without window.

:smile: Yes, conceptual only at this stage. Need #1594 and #1599 first.

[Consolatis]

Leaving this open until all the pieces are in place.

One thing I recognized is that the exclusive layershell setting may work against the user to a point where the whole compositor can become unusable. The only way to "fix" that was to A-F4 the terminal that still was set as the active view (even though didn't have keyboard focus anymore). When active view was switched to something else however there is basically nothing a user can do short of changing VT or using ssh from a remote system to get back (keyboard) control of the compositor.

[stefonarch]

• 1. Process sub-surfaces on cursor-button-press so that focus is given to client if on-demand/exclusive is set.

We have still the issue that the panel takes focus (especially annoying when "follow mouse" cursor settings are used). With Qt 6.8 and mLayerWindow->setKeyboardInteractivity(LayerShellQt::Window::KeyboardInteractivityNone); menus get focus nevertheless on kwin_wayland, Hyprland and wayfire, but not on labwc, sway, river and niri. We're considering a patch, but maybe labwc could give focus to menus too also if the main layer surface doesn't ask for it.

https://github.com/lxqt/lxqt-panel/pull/2153

[tsujan]

Let me explain the issue with more details:

A good panel shouldn't accept keyboard focus, whether on Wayland or X11, because otherwise, any mouse button press on it will steal the focus from the active window.

Now, under Wayland, we can use the Qt shell parameter KeyboardInteractivityNone for that. The problem is that, in this case, labwc makes all panel popups non-focusable too. That's bad because, for example, Main Menu and especially Fancy Menu need keyboard focus to work fine.

kwin_wayland, wayfire and hyprland don't have this problem.

As a workaround, we set KeyboardInteractivityNone only for the above compositors and tolerate a focusable panel under the others, including labwc.

[Consolatis]

You could keep it at None and set it to either OnDemand (not sure if that works at this stage) or to Exclusive before opening the menu. That should then make the menu inherit the keyboard setting. So basically what was suggested by @spl237 above.

I don't know how labwc could implement it otherwise without violating the protocol.

[tsujan]

You could keep it at None and set it to ...

Even if that works without extra problems (which I really doubt), it isn't practical: a lot of dirty codes.

[spl237]

In case it helps, what I have had to do for Pi is to create a custom menu opening function in the panel. This uses the layer-shell command to set keyboard interactivity for the panel to be true. I then added a custom signal to GTK which calls back to the panel when the interactivity change has been commited. The callback then actually pops up the menu, which now inherits the keyboard interactivity from the panel, and I set another callback on the hide signal on the menu which restores the keyboard interactivity for the panel back to false when the menu closes.

It is incredibly clunky and ugly, but it works - but it really shouldn't be necessary to do this. Menus should automatically have keyboard interactivity; I can't think of any circumstances in which you wouldn't want them to.

[tsujan]

but it really shouldn't be necessary to do this. Menus should automatically have keyboard interactivity.

I completely agree.

EDIT: It isn't just about menus but all child popups — at least with Qt.

[johanmalm]

A good panel shouldn't accept keyboard focus, whether on Wayland or X11, because otherwise, any mouse button press on it will steal the focus from the active window.

Interesting. I had never considered this. Last time I played with it I concluded that OnDemand was the right thing to do.

Have just played with

1. MS Windows
2. Ubuntu
3. xfce4-panel on X11
4. lxqt-panel on X11

Interestingly, MS Windows behaves like OnDemand whereas the rest behave like None.

All test cases give keyboard-focus on popups.

So, that makes quite a compelling argument for trying to "fix" this.

We'd have to square away the fact that the protocol says that "[the interactivity] setting is inherited by child surfaces set by the get_popup request" - potentially by making it configurable - but the first step is to work out how to do it.

I've had a play with src/layers.c but can't work out how to do it, so welcome help here.

[tsujan]

@johanmalm Thanks for considering it!

After we made the panel unfocusable in kwin_wayland, wayfire and hyprland, we realized that several problems, that didn't seem to be related to this, were automatically fixed for those compositors.