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

follow=keyboard: Fix regression where we don't fall back to mouse #1062

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

cdown
Copy link
Contributor

@cdown cdown commented Apr 7, 2022

Commit e8fc45d ("follow=keyboard: Fall back to follow=mouse instead
of XDefaultScreen()", PR#708) introduced the behaviour that we fall
back to follow=mouse if there is no focused client in order to
accommodate for window managers where sitting on the root window is
normalised, like dwm.

Commmit ebcd20d ("Fix process of gettign the active monitor",
PR#809) added support for multiple X screens on one display. However, it
broke the functionality introduced in the previous PR, because
explicitly focusing the root window results in XGetInputFocus()
returning the root window, not PointerRoot.

Fix this by explicitly checking if the focused window is the root
window.

Commit e8fc45d ("follow=keyboard: Fall back to follow=mouse instead
of XDefaultScreen()", PR#708) introduced the behaviour that we fall
back to follow=mouse if there is no focused client in order to
accommodate for window managers where sitting on the root window is
normalised, like dwm.

Commmit ebcd20d ("Fix process of gettign the active monitor",
PR#809) added support for multiple X screens on one display. However, it
broke the functionality introduced in the previous PR, because
explicitly focusing the root window results in XGetInputFocus()
returning the root window, not PointerRoot.

Fix this by explicitly checking if the focused window is the root
window.
@cdown
Copy link
Contributor Author

cdown commented Apr 7, 2022

Cc: @mcz

Noticed this for a little while now, but only recently had the time to dig in and find the cause.

@fwsmit
Copy link
Member

fwsmit commented Apr 14, 2022

Thank you for your contribution. It'd be nice if @mcz could check if this doesn't regress their fixes. Otherwise, I'll take a look next week to review the code better.

@mcz
Copy link
Contributor

mcz commented Apr 17, 2022

Doesn't look like it would break my patch.
I think that technically it would be correct to check if the focused window is any root window (each screen has its own) by checking against the RootWindow() for every screen that exist on the opened display (from 0 to ScreenCount() - 1). Not sure if that would be worth it since true multi-screen setups are so rare (with xinerama and xrandr being dominant). In my opinion doing so would only make sense as part of a big change separating multi-monitor (xinerama/xrandr) from multi-screen logic, which once again would a lot of work benefiting basically no one.

@fwsmit
Copy link
Member

fwsmit commented Apr 26, 2022

Thanks for reviewing mcz and thanks for the patch cdown. I'll merge this now.

@fwsmit fwsmit merged commit 58206fe into dunst-project:master Apr 26, 2022
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.

None yet

3 participants