Bug 421068

Any comments on this?

Ok, now I located the commit which (probably) broke this:

https://github.com/KDE/kwin/commit/e73e331f35b9da4bbb99c80c5c1eedd2ae99422b

So, until 2016, kwin *used* to send a valid timestamp, but this got deliberately broken to appease some Java Applications which were "extremely picky" and would refuse focus. But after that change, we now have GTK applications which dutifully take kwin's word for it, and actually do claim focus "at any time", i.e. at a moment where doing so is no longer appropriate, and where they disrupt the application that the user is actually working with.

Well, nowadays Java on the Desktop is pretty much dead, whereas GTK is alive and kicking (even if tied to KDE's competitor Gnome). And GTK is used by some very widely used applications such as Firefox and Thunderbird.

So we should really go back to respecting section 4.1.7 of the ICCCM "client to Window Manager Communication" communication rules, and include a meaningful timestamp. Indeed the ICCCM document clearly says:

  Windows with the atom WM_TAKE_FOCUS in their WM_PROTOCOLS property may receive 
  a ClientMessage event from the window manager (as described in section 4.2.8) 
  with WM_TAKE_FOCUS in its data[0] field and a valid timestamp (i.e. not 
  CurrentTime ) in its data[1] field.

Another thing. If there really are still a sizable number of users around who use Java on the desktop, and who depend on the current "take focus at any time" behavior, maybe this could be made configurable on a per-application basis by using the "Special Applications Settings" window, i.e. the one where you also configure Focus Stealing Prevention per Application.

Interestingly enough, this configuration window only contains a setting "Accept Focus", but no "Uses NET::TakeFocusProtocol". The latter would be very useful for the Firefox/Thunderbird case, because these 2 applications work alright even if they don't get these wm_take_focus messages. Indeed, the Client::takeFocus method already *gives* the application focus (via m_client.focus()) before allowing it to grab focus for itself. Thus the second step here is redundant in most cases (... and, as far as I understood, only useful for applications that do indeed want to hand focus to one specific subwindow of theirs)

>Any comments on this?

Firstly, thanks for the research.  It's very appreciated.

I'm normally quite reluctant on anything replacing one bug for another, this is the first report on this and the GTK code hasn't changed in a long time. The other report this was fixing has several. 

On the other hand being spec compliant is important.

Just wanted to say, we have read this, we'll get back to you.

(In reply to David Edmundson from comment #4)
> >Any comments on this?
> 
> Firstly, thanks for the research.  It's very appreciated.

Thanks for your sign of life :-)

It's reassuring to see that this doesn't fall on deaf ears.:-)

> 
> I'm normally quite reluctant on anything replacing one bug for another, this
> is the first report on this and the GTK code hasn't changed in a long time.
> The other report this was fixing has several. 

Well, the thing is this behavior has been around for quite a while, but it was hard to pinpoint it on a specific component. KDE, Firefox and GTK were throwing the ball to each other...

Focus management bugs have been reported to the KDE bugtracker since at least 2004 (not the same bug, but same category), and most often only with lukewarm success, unfortunately. Thus, "recent bug report" does not necessarily mean "recent problem". It could have been lurking quite a while, and only have become more frequent / more annoying due to timing and performance changes elsewhere. It's a race condition bug after all.

But, if push comes to shove, and indeed more than just Java applications will be broken by this, maybe we could make the behavior configurable by application by using the "Special Window Settings" and "Special Application Settings" dialogs?

> 
> On the other hand being spec compliant is important.

Indeed...

> 
> Just wanted to say, we have read this, we'll get back to you.

Thanks for the reassuring words.

I do have a patch by now (literally a 3 liner...), is it useful to attach it here, or must I get through the whole dance of setting up a complete kdesrc-build environment and submit a merge request? :-)

I assume the patch is basically just passing xTime() as the last argument of sendClientMessage in Client::takeFocus()

?

(In reply to David Edmundson from comment #6)
> I assume the patch is basically just passing xTime() as the last argument of
> sendClientMessage in Client::takeFocus()
> 
> ?

Yes indeed. And calling updateXTime(); just before.

Created attachment 128685 [details]
Pass valid time to sendClientMessage

Just attaching the patch here, easier than getting kdesrc-build going...

Built, tested (I modded the patch slightly as xTime() is the default arg)

Uploaded to our review tool: https://invent.kde.org/plasma/kwin/-/merge_requests/14

Git commit e9c68f36bdff63a49589a5eb76c02a1e98451331 by David Edmundson, on behalf of Alain Knaff.
Committed on 10/06/2020 at 10:11.
Pushed by davidedmundson into branch 'master'.

[x11] Send a valid timestamp in TakeFocus messages

Kwin sends out undated WM_TAKE_FOCUS client messages. Gtk based
applications such as Firefox react to these by handing focus to one of
their subwindows using XSetInputFocus(), and pass on the null time field
that they received in the client message to XSetInputFocus().

If for whatever reason the application (firefox) is slow to process the
event, it might issue that XSetInputFocus() message at a time when it
has already lost focus to the next application. This results in Firefox
stealing back the focus from the next application. Normally, such an
occurrence would not happen, as the server could tell by the time field
that the message is stale.

Until 2016 (e73e331f35b9da4bbb99c80c5c1eedd2ae99422b) kwin *used* to
send a valid timestamp, but this got deliberately broken to appease some
Java Applications which were "extremely picky" and would refuse focus.

This was based on the assumption that no other toolkit used the
timestamp from take focus events which is now proven to be false.

ICCCM document states:

Windows with the atom WM_TAKE_FOCUS in their WM_PROTOCOLS property may
receive  a ClientMessage event from the window manager (as described in
section 4.2.8) with WM_TAKE_FOCUS in its data[0] field and a valid
timestamp (i.e. not CurrentTime ) in its data[1] field."

M  +4    -3    x11client.cpp
M  +1    -2    x11client.h

https://invent.kde.org/plasma/kwin/commit/e9c68f36bdff63a49589a5eb76c02a1e98451331

Looks like this change has caused a regression for 'globally active' focus clients, including Java AWT/Swing applications, and JetBrains IDEs (IntelliJ IDEA, PyCharm, CLion, in particular). It doesn't look like a client-side issue, I've provided more details at https://bugs.kde.org/show_bug.cgi?id=347153#c42