Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cursor: process layer subsurfaces in cursor_button_press() and fix a layer-surface bug in get_cursor_context() #1594

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

johanmalm
Copy link
Collaborator

...to give keyboard focus to layer-shell clients if exclusive or on-demand interactivity is set, so that menu popups can be navigated with the keyboard. This still only works if the client is in top (or overlay) layers. Support for bottom and background to be done as a separate patch set.

Revert 06b19f0 to process layer-shell subsurfaces in cursor_button_press(), but only when their parent layer-shell surface has keyboard interactivity.

In get_cursor_context() process layer-surfaces and layer-subsurfaces from surfaces rather than nodes.

Background:

Commit 06b19f0 (issue #1131) disabled processing of layer-shell subsurfaces in cursor_button_press() because when pressing a task in Waybar (Gtk panel using layer-shell subsurfaces) the foreign-toplevel minimize-raise action did not work correctly as the action logic relied on the recipient window being activated and by clicking on the panel, the panel itself was both surface-focusd and activated (and thus the window de-activated).

The un-intended consequence was that by not responding to layer-subsurface cursor buttons presses, layer-shell clients (such as panels) were not given keyboard focus if they indeed wanted it by setting exclusive or on-demand keyboard interactivity.

The good news is that that following @jlindgren90's refactoring (various) the only place where we call view_set_actived() is in focus_change_notify() in seat.c and we now only do it for views (bb8f0bc).

Another side-effect (positive) of 06b19f0 was that a Waybar dnd bug was fixed (pointer-serial-number validation failure).

Have tested with sfwbar, waybar and tint (test-panel) the following results:

  • Minimize-raise works even when on-demand keyboard interactivity is set
  • Keyboard interactivity is given popup-menus (sfwbar and tint) when the panels are in the top layer (support for bottom will be as a separate patch set)
  • Waybar dnd still works (even when hard-coding keyboard-interactivity)

References:

Fixes: #1572

@stefonarch
Copy link
Contributor

stefonarch commented Mar 7, 2024

Tested with lxqt-panel (after changing KeyboardInteractivityNone to KeyboardInteractivityOnDemand) and pcmanfm-qt --desktop (this worked with the patch) - looks working fine.

Thanks!

@Tamaranch
Copy link

Tamaranch commented Mar 7, 2024

Looks like it would also fix #1554 :)

@Tamaranch
Copy link

Tamaranch commented Mar 7, 2024

I can also confirm that it does allow you to navigate xfce4-panel menus via the keyboard (internal plugins in the panel surface, or external plugins on their own layer-shell surface in overlay), and that it also solves that old problem of not being able to close the menu via the Escape key (we must have talked about this somewhere, but I can't remember where :)).

...to give keyboard focus to layer-shell clients if exclusive or on-demand
interactivity is set, so that menu popups can be navigated with the
keyboard. This still only works if the client is in top (or overlay)
layers. Support for bottom and background to be done as a separate patch
set.

Revert 06b19f0 to process layer-shell subsurfaces in
`cursor_button_press()`, but only when their parent layer-shell surface
has keyboard interactivity.

Fix bug in `get_cursor_context()` which resulted in layer-surfaces not
being detected correctly.

Background:

Commit 06b19f0 (issue labwc#1131) disabled processing of layer-shell
subsurfaces in cursor_button_press() because when pressing a task in
Waybar (Gtk panel using layer-shell subsurfaces) the foreign-toplevel
minimize-raise action did not work correctly as the action logic relied on
the recipient window being activated and by clicking on the panel, the
panel itself was both surface-focusd and activated (and thus the window
de-activated).

The un-intended consequence was that by not responding to layer-subsurface
cursor buttons presses, layer-shell clients (such as panels) were not
given keyboard focus if they indeed wanted it by setting exclusive or
on-demand keyboard interactivity.

The good news is that that following @jlindgren90's refactoring (various)
the only place where we call `view_set_actived()` is in
`focus_change_notify()` in `seat.c` and we now only do it for views
(bb8f0bc).

Another side-effect (positive) of 06b19f0 was that a Waybar dnd bug was
fixed (pointer-serial-number validation failure).

Have tested with sfwbar, waybar and tint (test-panel) the following
results:
- Minimize-raise works even when on-demand keyboard interactivity is set
- Keyboard interactivity is given popup-menus (sfwbar and tint) when the
  panels are in the top layer (support for bottom will be as a separate
  patch set)
- Waybar dnd still works (even when hard-coding keyboard-interactivity)

References:
- labwc@bb8f0bc
- https://github.com/labwc/labwc/blob/40ce95a68cf19796dd67b0527fddfdbe46181805/src/seat.c#L481-L483
- https://github.com/labwc/labwc/blob/40ce95a68cf19796dd67b0527fddfdbe46181805/src/dnd.c#L24
- https://github.com/johanmalm/tint

Fixes: labwc#1572
@johanmalm johanmalm force-pushed the layer-interactivity-subsurfaces branch from ffe3dcd to e4046c5 Compare March 8, 2024 19:28
@johanmalm
Copy link
Collaborator Author

Thanks both for testing.
Have refactored a bit following conversation with @Consolatis
Ready for final review + merge.

@johanmalm johanmalm changed the title cursor: process layer subsurfaces in cursor_button_press() cursor: process layer subsurfaces in cursor_button_press() and fix a layer-surface bug in get_cursor_context() Mar 8, 2024
@Consolatis
Copy link
Member

Consolatis commented Mar 8, 2024

Had a quick test nested and it improves things quite a bit, thanks for your work on this :)

@Consolatis Consolatis merged commit a5fcbfa into labwc:master Mar 8, 2024
5 checks passed
@johanmalm johanmalm deleted the layer-interactivity-subsurfaces branch March 9, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyboard focus on panel menus
4 participants