Bug 421068 - Kwin sends out undated WM_TAKE_FOCUS messages that goad firefox into stealing focus
Summary: Kwin sends out undated WM_TAKE_FOCUS messages that goad firefox into stealing...
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: core (show other bugs)
Version: 5.14.5
Platform: Debian stable Linux
: NOR grave
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-05 17:02 UTC by Alain Knaff
Modified: 2022-05-19 11:29 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Pass valid time to sendClientMessage (635 bytes, patch)
2020-05-22 11:12 UTC, Alain Knaff
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alain Knaff 2020-05-05 17:02:05 UTC
SUMMARY

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.

STEPS TO REPRODUCE

1. Open Firefox
2. Open Web Console Window (by using Inspect element)
3. Open another window (for example, a konsole), and place it such that it covers the top of the firefox web console
4. Open another window (another konsole) and place it such that it covers the bottom of the firexfox web console, leaving a narrow gap between both where web console is visible behind them.
5.  Move mouse back and forth between the top and the bottom konsole, crossing the web console each time.
6.  Whenever you are in a konsole with the mouse, observe the cursor shape.
7.  Pretty quickly you see that your konsole does not have focus (hollow cursor) even the mouse is in the window.
8.  Press cursor keys: you see that it's the Firefox web console who has focus!

OBSERVED RESULT

Firefox web console has focus

EXPECTED RESULT

Konsole should have focus

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Debian 10, 5.14.5.1-1
(available in About System)
KDE Plasma Version: 5.14.5.1-1
KDE Frameworks Version: 
Qt Version: 4.8.7+dfsg-18

ADDITIONAL INFORMATION
Bug initially reported to gtk under https://gitlab.gnome.org/GNOME/gtk/-/issues/2699

xtruss line showing the bad message WM_TAKE_FOCUS message:

--- SendEvent-generated ClientMessage(window=w#04000003, type=a#306, format=32, data=00000133:00000000:00000000:00000000:00000000)
Comment 1 Alain Knaff 2020-05-21 15:21:37 UTC
Any comments on this?
Comment 2 Alain Knaff 2020-05-21 16:29:28 UTC
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.
Comment 3 Alain Knaff 2020-05-21 16:55:08 UTC
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)
Comment 4 David Edmundson 2020-05-21 17:53:56 UTC
>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.
Comment 5 Alain Knaff 2020-05-21 20:06:12 UTC
(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? :-)
Comment 6 David Edmundson 2020-05-22 10:44:27 UTC
I assume the patch is basically just passing xTime() as the last argument of sendClientMessage in Client::takeFocus()

?
Comment 7 Alain Knaff 2020-05-22 11:10:51 UTC
(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.
Comment 8 Alain Knaff 2020-05-22 11:12:15 UTC
Created attachment 128685 [details]
Pass valid time to sendClientMessage

Just attaching the patch here, easier than getting kdesrc-build going...
Comment 9 David Edmundson 2020-05-22 13:04:23 UTC
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
Comment 10 David Edmundson 2020-06-10 10:11:33 UTC
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
Comment 11 Dmitry Batrak 2022-05-19 11:29:45 UTC
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