Fix window focus stack

[balazs-endresz]

This reverts f96255beb (except for some unnecessary whitespace changes) to fix #647

I've been using this for a few days and had no issues at all. It works fine with multiple workspaces too.

[raveit65]

I am in worry, that we run again in strange window switching foreground/background behavior with your PR.
On the other side it wasn't really clear which commit from metacity did really fix the strange window switching foreground/background behavior issue.
I will start to reproduce #647 and test your revert.

[raveit65]

Well, i can reproduce #647

Expected behaviour

when closing a window A, focus should return (pop off stack) to window that had the focus just before window A got the focus.
in other words, there should be a stack of windows successively having the focus.
this is what normally happens in other operating systems or window managers (compiz works as expected).
Actual behaviour

when window A is closed behavior is unpredictable. Often focus returns to desktop. Behavior varies with programs and
associated windows having the focus.
Steps to reproduce the behaviour

open window B, open window A, close window A, see whether window B still regains focus.

But your PR doesn't fix the issue on my box.
Tested with 2 mate-terminal windows.
Maybe you have a better reproducer...

[raveit65]

Hmm,
now i can't reproduce the issue any more with latest fedora 36 build.
I' am giving up. Please provide a clear reproducer and share your gsettings keys of marco.

[balazs-endresz]

I've tried a new vm with Ubuntu MATE 22.04. I just noticed that I don't get this issue with any pre-installed programs but it does happen when opening for example gitk from the command line. I've also noticed that alt-tab gets stuck when closing gitk. But the main issue I was trying to solve was that after closing gitk the terminal window still appeared as inactive even though I could type in it. I've tried to reproduce the various issues mentioned in the ticket here and metacity but it all seems to work with this PR. I'll give fedora a try soon too.

1. mate 22.04 stock - previous window active only after click.webm
2. mate 22.04 stock - stuck alt tab after closing window.webm
3. mate 22.04 patched.webm

[balazs-endresz]

One more thing to watch out for is that mate-netbook/maximus breaks the window focus/active states in a similar way (but when opening or unmaximising windows), so make sure that's not running. I'd like to look into that too at some point. I think mate-netbook used to be installed by default, so I wonder if that could have been actually the root cause of the focus issues back then.

[balazs-endresz]

On a fresh fedora 36 vm I get pretty much the same issue, with one minor difference though is that I can't type in the un-focused window, unlike on ubuntu. But this PR fixes it the same way on fedora as well.

4. fedora 36 stock.webm
5. fedora 36 patched.webm

[balazs-endresz]

I'll see if I can make this work without reverting that commit. I was just happy that I managed to make it work correctly after upgrading to 22.04 (from 18.04), but I guess there should be a better way to fix this.

[balazs-endresz]

I've found another way to fix this at https://github.com/mate-desktop/marco/blob/v1.26.0/src/core/display.c#L2223 by matching not just NotifyDetailNone but NotifyPointerRoot as well.

When I open from the terminal and then close an app e.g. mate-tweak then event->xfocus.detail is equal to NotifyDetailNone but if I do the same with gitk it's NotifyPointerRoot.

Would changing that make more sense? So that NotifyPointerRoot would also trigger meta_workspace_focus_default_window (instead of reverting f96255beb).

[balazs-endresz]

I've updated the PR with the NotifyPointerRoot change instead. I've noticed that NotifyPointerRoot is always mentioned together with NotifyDetailNone in the X docs: https://tronche.com/gui/x/xlib/events/input-focus/normal-and-grabbed.html, so this does seems like the right thing to do.

(Btw, very rarely, maybe one out of 50 tries the focus is still not regained. After adding some logging it appears that event_callback with the FocusOut event is called only twice when that happens. Normally it's called many more times for some reason. This occasional focus issue was also mentioned in another ticket but I can't find that now.)

[raveit65]

@mate-desktop/core-team
Can someone else from the team review this PR, please.
I am complete busy in life.

[lukefromdc]

I normally use compiz so #647 is not something I would notice in quick marco tests if it is intermittant. I was unable to duplicate the issue with two Pluma windows open,moving focus from one to the other. On closing the pluma window I had just moved focus to, focus returned to the previous pluma window.

For this test I used marco with compositng ON, and never at any time have I turned on focus follows mouse

[lukefromdc]

While I was NOT able to duplicate the issue in a quick test, I didn't get any ugly surprises with this either. Again,note that in my normal workflow I use compiz so I would not see an intermittant issue.

[lukefromdc]

Also I cannot test whether this fixes #301 as I have neither snappy nor flatpack installed

[raveit65]

Ok, with you gitk example the foreground problem is reproducible for me now. But it isn't the focus which is broken. The problem is that the window has focus but it isn't in the foreground.
After closing gitk the terminal window isn't in foreground but you can type in the terminal window. That means the focus is working. With your PR the terminal window has focus and it is in the foreground.
If you agree with that you should rename the commit. focus --> foreground

[balazs-endresz]

Yep, that makes sense. I've just reworded the commit message.

[muktupavels]

Can you reproduce that problem with metacity, mutter, compiz? None of them handles NotifyPointerRoot...

[balazs-endresz]

metacity works correctly without this change. I can't reproduce an event with NotifyDetailNone, so meta_workspace_focus_default_window is never called. I get the following three events with any app I try:

NotifyPointerRoot	6
NotifyPointer		5
NotifyNonlinearVirtual	4

When I apply this patch to metacity it still works correctly at least.

[raveit65]

I noticed another fix by this commit.
In vlc-player full screen mode the control-bar is fading in/out correctly, when entering the window with the mouse.

[lukefromdc]

Is this ready to go or is more work/review needed?

[muktupavels]

That code was added to handle specific case. I doubt that your problem is caused by X protocol, so debug message now likely will be lies in some cases. Adding NotifyPointerRoot is not fix, it just workaround / hides real problem...

[lukefromdc]

OK, I will wait on this for others to weigh in or more work to be done

[raveit65]

LGTM,
Commit runs fine since several days and fixes several issues.
Ready to go for me.

[mmhere]

A potentially difficult follow-on question. Do we have a way to track when this change appears in various distros' repositories? Building from source is not an option for [most] non-developers. Building like that also introduces local "which version is installed" issues, like when the local distro update overrides with whatever old version is in their repository.

For me, recent Ubuntu flavours would be interesting, but Linuxmint (version 21, latest, presently) is of primary interest.

I know that tracking downstream adoption is probly outside the scope here, so please be kind with any responses.

Thank You.

Or... is it possible to provide a .deb package out of this github project (and could an end user trust it ;-)? I'd be willing to test in situ (Linuxmint 21) then report back.

[lukefromdc]

A .deb package could be broken at any time by a change in an underlying library which would require recompilation. We would have to maintain that, and I can tell you from personal experience with my own custom built .debs that that happens far more often than underlying code changes that force source code changes. On Debian Unstable this can happen at any time, and there are also testing and stable to deal with, plus Ubuntu, plus Mint, etc