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

skip keyboard focus for layer shell surfaces not... #818

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

cmeissl
Copy link
Contributor

@cmeissl cmeissl commented Nov 19, 2024

...requesting keyboard interactivity

I have seen clients reacting weird when receiving keyboard focus unexpectedly.
The last time it was about implicit focus for popups in firefox, which immediately dismissed the popup.
Looks like the same applies to layer surfaces not requesting keyboard interactivity.

fixes #311

@YaLTeR
Copy link
Owner

YaLTeR commented Nov 21, 2024

Hmm, so what is actually causing the issue here? xdg-popup grab docs say (emphasis mine):

During a popup grab, the client owning the grab will receive pointer and touch events for all their surfaces as normal (similar to an "owner-events" grab in X11 parlance), while the top most grabbing popup will always have keyboard focus.

It seems that currently, keyboard navigation does work in Waybar popup menus on niri. Though, on sway it doesn't—keyboard events keep going to the window instead.

While the grab is active, niri will keep setting keyboard focus to the root layer surface, which PopupKeyboardGrab will ignore. When the popup is dismissed, niri sets the keyboard focus straight to something else, i.e. the layout.

(in fact, AFAICT, with this PR, niri will set the keyboard focus to the root layer surface in update_keyboard_focus() with no grab to stop it, so I'm not sure why it helps)

Could it be that the problem is caused by PopupKeyboardGrab restoring the keyboard focus to the root layer surface briefly when the popup is dismissed, like through here or elsewhere? (I'm not sure I entirely understand what code runs when dismissing the popup by pressing Escape, which also sometimes triggers the issue.) Though, I'm not sure why the problem only happens sometimes.

@cmeissl cmeissl force-pushed the fix/popup_grab_keyboard_focus branch from e7b7f00 to 8f6c912 Compare November 24, 2024 19:12
@cmeissl
Copy link
Contributor Author

cmeissl commented Nov 24, 2024

Hmm, so what is actually causing the issue here? xdg-popup grab docs say (emphasis mine):

During a popup grab, the client owning the grab will receive pointer and touch events for all their surfaces as normal (similar to an "owner-events" grab in X11 parlance), while the top most grabbing popup will always have keyboard focus.

It seems that currently, keyboard navigation does work in Waybar popup menus on niri. Though, on sway it doesn't—keyboard events keep going to the window instead.

Yeah, because the layer keyboard interactivity is set to None. Out of waybar, ironbar and lxqt-panel which I tested only lxqt-panel requested keyboard interactivity.

While the grab is active, niri will keep setting keyboard focus to the root layer surface, which PopupKeyboardGrab will ignore. When the popup is dismissed, niri sets the keyboard focus straight to something else, i.e. the layout.

imo it should also exclude layer surfaces not requesting keyboard interactivity here. I updated the PR to do that, so now it behaves the same as in sway.

(in fact, AFAICT, with this PR, niri will set the keyboard focus to the root layer surface in update_keyboard_focus() with no grab to stop it, so I'm not sure why it helps)

Could it be that the problem is caused by PopupKeyboardGrab restoring the keyboard focus to the root layer surface briefly when the popup is dismissed, like through here or elsewhere? (I'm not sure I entirely understand what code runs when dismissing the popup by pressing Escape, which also sometimes triggers the issue.) Though, I'm not sure why the problem only happens sometimes.

No, it has nothing to do with restoring the focus imo. imo it just worked by chance. But it seems the issue is worse when the popup unexpectedly receives keyboard focus then when only the layer surface receives it. (Which happened in the previous version of this PR which only ignored the keyboard grab).

@cmeissl cmeissl force-pushed the fix/popup_grab_keyboard_focus branch from 8f6c912 to 08255b2 Compare November 24, 2024 19:19
@calops
Copy link
Contributor

calops commented Dec 18, 2024

This PR seems to fix a few issues I have, thanks! Is there anything I can do to help move it forward? Is anything missing that's preventing a merge?

@YaLTeR
Copy link
Owner

YaLTeR commented Dec 18, 2024

I'm currently deep in the floating branch, I'll get to this PR afterwards. I want to do some more testing and digging on my own here too.

On that note, what other issues does it fix for you? This will be useful

@calops
Copy link
Contributor

calops commented Dec 20, 2024

I was mistaken, I found no other issues related to this 👍

@quadbyte
Copy link

I can confirm this fixed #311 on my machine. Thanks! it was a daily pain to deal with this bug.

@quadbyte
Copy link

Now that floating has been merged, would be nice if this could be revisited.

@YaLTeR
Copy link
Owner

YaLTeR commented Dec 31, 2024

Don't worry I'll get to it. After the y'know new year's eve and all

a grab is requested for an unmapped popup,
delay focusing the popup until the first keyboard
interaction
...requesting keyboard interactivity
@cmeissl cmeissl force-pushed the fix/popup_grab_keyboard_focus branch from 08255b2 to 71ce04c Compare December 31, 2024 16:43
@cmeissl
Copy link
Contributor Author

cmeissl commented Dec 31, 2024

rebased on current HEAD

@YaLTeR
Copy link
Owner

YaLTeR commented Jan 2, 2025

I'm investigating this a bit. So, with the current niri code, if I click on an icon to open a menu, this is sent:

[ 171146.346] {Default Queue} wl_keyboard#25.enter(13491, wl_surface#39, array[4])
[ 171146.350] {Default Queue} wl_keyboard#25.modifiers(13491, 64, 0, 0, 0)

The keyboard enters the pop-up menu surface. Next, when I click outside the menu, this happens:

[ 171919.435] {Default Queue} wl_keyboard#25.leave(13760, nil)
[ 171919.444] {Default Queue} wl_keyboard#25.enter(13760, wl_surface#34, array[4])
[ 171919.450] {Default Queue} wl_keyboard#25.modifiers(13760, 64, 0, 0, 0)
[ 171920.788] {Default Queue} wl_keyboard#25.leave(13761, wl_surface#34)

The keyboard leaves the closed (nil) pop-up menu surface, then enters and immediately leaves the bar wl_surface with no keyboard interactivity. I suspect this is the grab "restoring" the keyboard focus. However, this sequence of events doesn't cause the bug: the menu keeps working.

Now, if instead I open a menu, then click again without moving my mouse, then the menu will close, but there will be no further events. So it's just the first two enter and modifiers on the pop-up menu surface. No leave or anything else.

This is when the bug happens; subsequent clicks on the same icon keep sending enter and modifiers for the same pop-up surface (which apparently still exists, despite being invisible?). In this state, clicking on a different icon will bring up its menu and reset whatever issue occurred.

@YaLTeR
Copy link
Owner

YaLTeR commented Jan 2, 2025

Well anyway, the keyboard focus code in niri probably needs some reworking, and this does seem to fix the issue, so let's merge it and maybe I'll come up with something better later on.

@YaLTeR YaLTeR merged commit b16d7ab into YaLTeR:main Jan 2, 2025
10 checks passed
@YaLTeR
Copy link
Owner

YaLTeR commented Jan 2, 2025

Thanks!

@cmeissl cmeissl deleted the fix/popup_grab_keyboard_focus branch January 2, 2025 19:15
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.

Cannot open Waybar system tray popup more than once
4 participants