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

xwayland: listen to request_activate event #6764

Merged
merged 1 commit into from Jan 31, 2022
Merged

xwayland: listen to request_activate event #6764

merged 1 commit into from Jan 31, 2022

Conversation

yrhki
Copy link
Contributor

@yrhki yrhki commented Jan 17, 2022

When REAPER submenu is closed XCB_CLIENT_MESSAGE with type
NET_ACTIVE_WINDOW is sent to set focus to parent menu.

Closes: #6324

@emersion
Copy link
Member

I don't understand why re-focusing is necessary here.

@emersion
Copy link
Member

Giving focus to an unmapped surface doesn't make sense, the surface isn't shown on screen.

@yrhki yrhki changed the title xwayland: re-focus on pointer focused surface on unmap xwayland: listen to request_activate event Jan 27, 2022
@yrhki
Copy link
Contributor Author

yrhki commented Jan 27, 2022

@emersion I looked deeper into this and this seems to be proper solution.

@Xyene
Copy link
Member

Xyene commented Jan 27, 2022

This looks like a proper fix, thanks for looking into it!

Out of curiosity, does this remove the need to do

if (seat->wlr_seat->keyboard_state.focused_surface == xsurface->surface) {
// This simply returns focus to the parent surface if there's one available.
// This seems to handle JetBrains issues.
if (xsurface->parent && xsurface->parent->surface
&& wlr_xwayland_or_surface_wants_focus(xsurface->parent)) {
seat_set_focus_surface(seat, xsurface->parent->surface, false);
return;
}
// Restore focus
struct sway_node *previous = seat_get_focus_inactive(seat, &root->node);
if (previous) {
// Hack to get seat to re-focus the return value of get_focus
seat_set_focus(seat, NULL);
seat_set_focus(seat, previous);
}
? (In case it does, not asking you to make the change in this PR, just wondering if it does.)

@emersion
Copy link
Member

We should make sure this doesn't allow an X11 app to steal focus from a Wayland app. IIRC we already have some kind of client matching elsewhere, maybe we can do something similar?

@yrhki
Copy link
Contributor Author

yrhki commented Jan 27, 2022

@Xyene Removing

if (xsurface->parent && xsurface->parent->surface
&& wlr_xwayland_or_surface_wants_focus(xsurface->parent)) {
seat_set_focus_surface(seat, xsurface->parent->surface, false);
return;
}
breaks menus in IntelliJ IDEA.

Also found that in IDEA set_hints event is called on sub-menu before unmap happens, but sway_xwayland_unmanaged dosen't have it. It would be helpful if wlr_signal_emit_safe would log when nothing is called.

@Xyene
Copy link
Member

Xyene commented Jan 28, 2022

Thanks for testing.

Yeah, I think that doesn't sound unreasonable for a debug-level message, it seems likely this issue would have been found quicker if we had it.

@yrhki
Copy link
Contributor Author

yrhki commented Jan 29, 2022

I think this will do it. Doesn't limiting to the same process apply here as well similar to xwm_handle_focus_in?

@emersion
Copy link
Member

Doesn't limiting to the same process apply here as well similar to xwm_handle_focus_in?

Yup, that would be nice.

sway/desktop/xwayland.c Outdated Show resolved Hide resolved
sway/desktop/xwayland.c Outdated Show resolved Hide resolved
When REAPER submenu is closed `XCB_CLIENT_MESSAGE` with type
`NET_ACTIVE_WINDOW` is sent to set focus to parent menu.

Closes: #6324
@yrhki yrhki requested a review from emersion January 31, 2022 07:09
Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@emersion emersion merged commit 69b4302 into swaywm:master Jan 31, 2022
@emersion emersion added this to the 1.7.1 milestone Jan 31, 2022
@yrhki yrhki deleted the reaper-focus branch January 31, 2022 10:33
@salkin-mada
Copy link
Contributor

Fantastic! Thanks so much @yrhki

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

mouse and dropdown issues in Reaper after sway 1.6 [wlroots 13] in XWayland.
4 participants