Re: master 4b98a79a50: Improve X event timestamp tracking

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> branch: master
> commit 4b98a79a508ebdc719abfcf51ee6de32e46d0e1c
> Author: Daniel Colascione <dancol@dancol.org>
> Commit: Daniel Colascione <dancol@dancol.org>

Sorry, but I will revert this change.  As the X port maintainer, it
looks wrong to me from several different points of view.

Please finish the discussion in the bug thread before installing this
code in the future.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On Sun, 2022-08-07 at 11:46 +0800, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
> > branch: master
> > commit 4b98a79a508ebdc719abfcf51ee6de32e46d0e1c
> > Author: Daniel Colascione <dancol@dancol.org>
> > Commit: Daniel Colascione <dancol@dancol.org>
> 
> Sorry, but I will revert this change.  As the X port maintainer, it
> looks wrong to me from several different points of view.

You are not the sole X port maintainer.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> You are not the sole X port maintainer.

etc/MAINTAINERS says otherwise.

I don't buy the argument for having an entire terminal hook, meaningless
outside X (name one other window system under which it makes sense),
that is supposed to make x-focus-frame magically work in situations
where it currently does not.

Let's continue the discussion in bug#57012.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On Sun, 2022-08-07 at 11:51 +0800, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
> > You are not the sole X port maintainer.
> 
> etc/MAINTAINERS says otherwise.

I've contributed code all over this project and never been limited by
MAINTAINERS. This file does not grant you the authority to
arbitrarily reject contributions that fix long-standing bugs. You're
welcome to make improvements to committed code just like anyone else.
I'd suggest minimizing friction in the future, not blocking useful
work form someone who spent all day debugging this issue.

> I don't buy the argument for having an entire terminal hook, meaningless
> outside X (name one other window system under which it makes sense),
> that is supposed to make x-focus-frame magically work in situations
> where it currently does not.

An "entire terminal hook" is a trivial function pointer. 
Don't you think this is a mountain out of a molehill? What,
precisely, is the resource being consumed by minimizing terminal hook
structure fields? 

Sure, we could have open-coded typecases or inscrutably invocations
of some "force" parameter --- or we could make a generic terminal
operation that clearly and explicitly expresses user intent. It's not
as if the X event timestamp mechanism exists without reason either.
Focus-stealing prevent isn't some "draconian" measure to work around
or a bug in window managers, but instead a way to properly order
events observed in a distributed, asynchronous system.

*In general*, code is cleaner when 1) explicit programmer intent is
expressed (as opposed to "add a force qualifier, because force"), and
2) that programmer intent is communicated without special cases.
You're proposing scrapping a generic mechanism and replacing it with
a special case, and I don't right now see the net benefit.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> I've contributed code all over this project and never been limited by
> MAINTAINERS. This file does not grant you the authority to
> arbitrarily reject contributions that fix long-standing bugs. You're
> welcome to make improvements to committed code just like anyone else.
> I'd suggest minimizing friction in the future, not blocking useful
> work form someone who spent all day debugging this issue.

I have not arbitrarily rejected anything.  I've told you why this isn't
a good idea, yet you proceeded to install the change anyway.  Please, no
rush!

> An "entire terminal hook" is a trivial function pointer.
> Don't you think this is a mountain out of a molehill? What,
> precisely, is the resource being consumed by minimizing terminal hook
> structure fields?

Consider the following situation: a programmer is writing some code and
notices that `x-focus-frame' is not working.  But the doc string says it
should work, without calling magic functions to note "out-of-band
interaction".

I'm not concerned about how many bytes a terminal hook takes.  I'm
concerned about exposing a clean abstraction over the window system to
users and programmers working on display-independent parts of Emacs.

> Sure, we could have open-coded typecases or inscrutably invocations
> of some "force" parameter --- or we could make a generic terminal
> operation that clearly and explicitly expresses user intent. It's not
> as if the X event timestamp mechanism exists without reason either.

The X server clock exists to provide orderly synchronization of input
focus and selection ownership.  Ensuring Emacs works with that
synchronization isn't the job of Lisp code in server.el or C code in
termhooks.h, it's the job of C code in x*.c.

> Focus-stealing prevent isn't some "draconian" measure to work around
> or a bug in window managers, but instead a way to properly order
> events observed in a distributed, asynchronous system.

Focus stealing prevention is a draconian measure to ensure that programs
in the background do not suddenly move themselves into the foreground.

Which works, until it doesn't, like with the Emacs server.

> You're proposing scrapping a generic mechanism and replacing it with
> a special case, and I don't right now see the net benefit.

Because that generic mechanism exposes low-level window system
implementation details to users.  The Lisp programmer shouldn't need to
know he must call two functions, instead of one, to ensure that
x-focus-frame results in a frame being activated, which defeats the
whole point of Emacs abstracting over the window system.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On Sun, 2022-08-07 at 12:23 +0800, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
> > I've contributed code all over this project and never been limited by
> > MAINTAINERS. This file does not grant you the authority to
> > arbitrarily reject contributions that fix long-standing bugs. You're
> > welcome to make improvements to committed code just like anyone else.
> > I'd suggest minimizing friction in the future, not blocking useful
> > work form someone who spent all day debugging this issue.
> 
> I have not arbitrarily rejected anything.  I've told you why this isn't
> a good idea, yet you proceeded to install the change anyway.  Please, no
> rush!
> 
> > An "entire terminal hook" is a trivial function pointer.
> > Don't you think this is a mountain out of a molehill? What,
> > precisely, is the resource being consumed by minimizing terminal hook
> > structure fields?
> 
> Consider the following situation: a programmer is writing some code and
> notices that `x-focus-frame' is not working.  But the doc string says it
> should work, without calling magic functions to note "out-of-band
> interaction".
> I'm not concerned about how many bytes a terminal hook takes.  I'm
> concerned about exposing a clean abstraction over the window system
to
> users and programmers working on display-independent parts of
Emacs.

If Emacs, in the background, calls x-focus-frame to activate a window
and there's no explicit user intent to activate that window, the WM
is within its rights to reject the proposed change to the window
stacking. In this case, then the focus stealing mechanism is working
at intended. Adding a "no, I really mean it" flag to x-focus-frame
would encourage people to break this mechanism by passing that flag
whenever x-focus-frame didn't work no matter the reason it didn't
work --- even when this function *shouldn't* work because Emacs
really is trying to do something that would trigger focus stealing
prevent heuristics. 

server.el is a special case: it's okay to break the usual event
ordering here because the user *did* interact with Emacs, albeit
through a side channel, not the X server. It's not that developers
need to call two functions to make some API work, but rather,
developers should call an additional function to communicate
information to the core that allows an otherwise forbidden-by-design
state transition to occur after all.

In other words, the clean abstraction *is* telling Emacs "Oh, by the
way, the user interacted with this frame", not telling x-focus-frame
"please, actually do your job for some reason". There's nothing X-
specific about giving the Emacs core a hint that the user recently
interacted with a frame in some manner not reflected in the normal
flow of events. If most window systems want to ignore this hint,
that's fine, but that doesn't make the hint a leaky abstraction.

> 
> > Sure, we could have open-coded typecases or inscrutably invocations
> > of some "force" parameter --- or we could make a generic terminal
> > operation that clearly and explicitly expresses user intent. It's not
> > as if the X event timestamp mechanism exists without reason either.
> 
> The X server clock exists to provide orderly synchronization of input
> focus and selection ownership.  Ensuring Emacs works with that
> synchronization isn't the job of Lisp code in server.el or C code in
> termhooks.h, it's the job of C code in x*.c.

Don't you think it is the job of server.el to tell that window system
core code a fact that it wouldn't have otherwise known --- that the
user interacted with the frame in some manner the window system
wouldn't have otherwise know about? I think an invocation of
gnuserver that raiess a window ought to count as interaction. There
are probably other use-cases as well.

> 
> > Focus-stealing prevent isn't some "draconian" measure to work around
> > or a bug in window managers, but instead a way to properly order
> > events observed in a distributed, asynchronous system.
> 
> Focus stealing prevention is a draconian measure to ensure that programs
> in the background do not suddenly move themselves into the foreground.
> 
> Which works, until it doesn't, like with the Emacs server.

Yes. Ideally, we'd detect in emacsclient that emacsclient and emacs
were running on the same X server and transmit the command buffer as
a ClientEvent --- and we'd update our user timestamp when we got that
event, giving us a globally correct event ordering. In the meantime,
the "user interacted out of band" hint seems like the most
conservative approach for working around focus stealing mitigations.

> 
> > You're proposing scrapping a generic mechanism and replacing it with
> > a special case, and I don't right now see the net benefit.
> 
> Because that generic mechanism exposes low-level window system
> implementation details to users.  The Lisp programmer shouldn't need to
> know he must call two functions, instead of one, to ensure that
> x-focus-frame results in a frame being activated, which defeats the
> whole point of Emacs abstracting over the window system.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> If Emacs, in the background, calls x-focus-frame to activate a window
> and there's no explicit user intent to activate that window, the WM
> is within its rights to reject the proposed change to the window
> stacking. In this case, then the focus stealing mechanism is working
> at intended. Adding a "no, I really mean it" flag to x-focus-frame
> would encourage people to break this mechanism by passing that flag
> whenever x-focus-frame didn't work no matter the reason it didn't
> work --- even when this function *shouldn't* work because Emacs
> really is trying to do something that would trigger focus stealing
> prevent heuristics. 

Why would that be so?  The window manager doesn't get to decide what the
author of some Lisp code wanted.

Our users and developers are not sloppy programmers who write code that
steals focus from other windows for no reason whatsoever.  The intent is
demonstrated by code calling x-focus-frame being written.

> server.el is a special case: it's okay to break the usual event
> ordering here because the user *did* interact with Emacs, albeit
> through a side channel, not the X server. It's not that developers
> need to call two functions to make some API work, but rather,
> developers should call an additional function to communicate
> information to the core that allows an otherwise forbidden-by-design
> state transition to occur after all.

Why would the activation otherwise be "forbidden-by-design" (_our_
design, not the design of the window manager authors?)  Focus stealing
prevention doesn't exist on any other platform we support, so
x-focus-frame always works there.  As such, users expect the same from
X.  This is compounded by the fact that x-focus-frame says nothing about
requiring "user interaction" before the frame is activated.

> In other words, the clean abstraction *is* telling Emacs "Oh, by the
> way, the user interacted with this frame", not telling x-focus-frame
> "please, actually do your job for some reason". There's nothing X-
> specific about giving the Emacs core a hint that the user recently
> interacted with a frame in some manner not reflected in the normal
> flow of events. If most window systems want to ignore this hint,
> that's fine, but that doesn't make the hint a leaky abstraction.

It is a leaky abstraction because such a hint only exists on X, and was
never ever part of Emacs design.  Other window systems don't "ignore
this hint", because such a hint simply does not exist.

> Don't you think it is the job of server.el to tell that window system
> core code a fact that it wouldn't have otherwise known --- that the
> user interacted with the frame in some manner the window system
> wouldn't have otherwise know about?

No.  Lisp code not a platform implementation detail (be it in server.el
or whatever) does not have to know about this at all.

> I think an invocation of gnuserver that raiess a window ought to count
> as interaction. There are probably other use-cases as well.

No, it's not.  There is an actual startup notification protocol, but
it's not well suited for implementation in emacsclient.

> Yes. Ideally, we'd detect in emacsclient that emacsclient and emacs
> were running on the same X server and transmit the command buffer as
> a ClientEvent --- and we'd update our user timestamp when we got that
> event, giving us a globally correct event ordering. In the meantime,
> the "user interacted out of band" hint seems like the most
> conservative approach for working around focus stealing mitigations.

Client events don't include timestamps, so while you can ensure "correct
event ordering" with those, you cannot get the server time from them.

The actual protocol here is the startup notification protocol, which
emacsclient doesn't support for an obvious reason.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On Sun, 2022-08-07 at 13:26 +0800, Po Lu wrote:
> The window manager doesn't get to decide what the
> author of some Lisp code wanted.

Yes, it does. That's the whole point of being the window manager. Are
you suggesting that application developers, not users, ought to get
the final word on what windows go where?

> Our users and developers are not sloppy programmers who write code that
> steals focus from other windows for no reason whatsoever.  

Yes, they are sloppy, just like all other users and developers, you
and me included, are sloppy. What's the old quip? Ah, right: "If men
were angels, no laws would be necessary". If developers were perfect,
we wouldn't need ASLR, or memory protection, or file permissions, or
fuzzing, or memory-safe languages --- yet here we are.

Developers of Emacs are no more angelic than developers using any
other toolkit, and focus stealing prevention mitigates their mistakes
as much as it does any others. If a user doesn't want focus stealing
preventation, he can disable it or use a window manager that doesn't
provide it. It's not the place of Emacs to override the user's
preference.

> The intent is
> demonstrated by code calling x-focus-frame being written.

If a process filter tries to asynchronously raise a window when the
user is the middle of browsing cat pictures, and that user has
configured his WM to block attempts by applications in the background
to raise windows, the WM is right to block that raise attempt. The WM
is where policy belongs.

> > server.el is a special case: it's okay to break the usual event
> > ordering here because the user *did* interact with Emacs, albeit
> > through a side channel, not the X server. It's not that developers
> > need to call two functions to make some API work, but rather,
> > developers should call an additional function to communicate
> > information to the core that allows an otherwise forbidden-by-design
> > state transition to occur after all.
> 
> Why would the activation otherwise be "forbidden-by-design" (_our_
> design, not the design of the window manager authors?)  Focus stealing
> prevention doesn't exist on any other platform we support, so
> x-focus-frame always works there.  
> 

It does exist elsewhere. From MSDN's page on SetForegroundWindow
(https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setforegroundwindow)
:

  The system restricts which processes can set the foreground window.
  A process can set the foreground window only if one of the
following
  conditions is true:

  * The process is the foreground process.

  * The process was started by the foreground process.

  * The process received the last input event.

  * There is no foreground process.

  * The process is being debugged.

  * The foreground process is not a Modern Application or the
    Start Screen.

  * The foreground is not locked (see LockSetForegroundWindow).

  * The foreground lock time-out has expired (see *
    SPI_GETFOREGROUNDLOCKTIMEOUT in SystemParametersInfo).

  * No menus are active.


Also from that page:

  A process that can set the foreground window can enable another
  process to set the foreground window by calling the
  AllowSetForegroundWindow function. The process specified by
  dwProcessId loses the ability to set the foreground window the next
  time the user generates input, unless the input is directed at that
  process, or the next time a process calls AllowSetForegroundWindow,
  unless that process is specified.

Guess what API emacsclient calls.

> As such, users expect the same from
> X.  This is compounded by the fact that x-focus-frame says nothing about
> requiring "user interaction" before the frame is activated.
> 
> > In other words, the clean abstraction *is* telling Emacs "Oh, by the
> > way, the user interacted with this frame", not telling x-focus-frame
> > "please, actually do your job for some reason". There's nothing X-
> > specific about giving the Emacs core a hint that the user recently
> > interacted with a frame in some manner not reflected in the normal
> > flow of events. If most window systems want to ignore this hint,
> > that's fine, but that doesn't make the hint a leaky abstraction.
> 
> It is a leaky abstraction because such a hint only exists on X, and was
> never ever part of Emacs design.  Other window systems don't "ignore
> this hint", because such a hint simply does not exist.

It's a generic hint that only one or two backends care about right
now. That's not the same as a leaky abstraction.

> 
> > Don't you think it is the job of server.el to tell that window system
> > core code a fact that it wouldn't have otherwise known --- that the
> > user interacted with the frame in some manner the window system
> > wouldn't have otherwise know about?
> 
> No.  Lisp code not a platform implementation detail (be it in server.el
> or whatever) does not have to know about this at all.

Platform-implementation code shouldn't have to know about platform
specifics. That's why frame operations should be generic and
polymorphic, not ad-hoc and gated behind type tests.

I'll say it again: server.el hinting to Emacs that the user has
interacted with a frame is not an implementation detail of a window
system.

> 
> > I think an invocation of gnuserver that raiess a window ought to count
> > as interaction. There are probably other use-cases as well.
> 
> No, it's not.  There is an actual startup notification protocol, but
> it's not well suited for implementation in emacsclient.

Startup notification isn't suitable here because we're not starting
anything.

> 
> > Yes. Ideally, we'd detect in emacsclient that emacsclient and emacs
> > were running on the same X server and transmit the command buffer as
> > a ClientEvent --- and we'd update our user timestamp when we got that
> > event, giving us a globally correct event ordering. In the meantime,
> > the "user interacted out of band" hint seems like the most
> > conservative approach for working around focus stealing mitigations.
> 
> Client events don't include timestamps, so while you can ensure "correct
> event ordering" with those, you cannot get the server time from them.

Emacsclient could include *its* server time in the message.

> The actual protocol here is the startup notification protocol, which
> emacsclient doesn't support for an obvious reason.

Well, a related protocol would be nice. Feel free to propose one.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> Yes, it does. That's the whole point of being the window manager. Are
> you suggesting that application developers, not users, ought to get
> the final word on what windows go where?

_Emacs_ users and developers get the final word, not window manager
developers.

> Yes, they are sloppy, just like all other users and developers, you
> and me included, are sloppy. What's the old quip? Ah, right: "If men
> were angels, no laws would be necessary". If developers were perfect,
> we wouldn't need ASLR, or memory protection, or file permissions, or
> fuzzing, or memory-safe languages --- yet here we are.

So you are saying that ASLR or memory protection is the same as focus
stealing "prevention"? Seriously?

> Developers of Emacs are no more angelic than developers using any
> other toolkit, and focus stealing prevention mitigates their mistakes
> as much as it does any others. If a user doesn't want focus stealing
> preventation, he can disable it or use a window manager that doesn't
> provide it. It's not the place of Emacs to override the user's
> preference.

Why can't the user also customize `x-allow-focus-stealing' (see the
patch I sent) to nil?  Or better, report a bug to the Emacs developers
while at it?

Focus stealing prevention is not a user choice, and can't even be turned
off in the popular window managers.

> If a process filter tries to asynchronously raise a window when the
> user is the middle of browsing cat pictures, and that user has
> configured his WM to block attempts by applications in the background
> to raise windows, the WM is right to block that raise attempt. The WM
> is where policy belongs.

I don't see 

> It does exist elsewhere. From MSDN's page on SetForegroundWindow
> (https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setforegroundwindow)
> :
>
>   The system restricts which processes can set the foreground window.
>   A process can set the foreground window only if one of the
> following
>   conditions is true:
>
>   * The process is the foreground process.
>
>   * The process was started by the foreground process.
>
>   * The process received the last input event.
>
>   * There is no foreground process.
>
>   * The process is being debugged.
>
>   * The foreground process is not a Modern Application or the
>     Start Screen.
>
>   * The foreground is not locked (see LockSetForegroundWindow).
>
>   * The foreground lock time-out has expired (see *
>     SPI_GETFOREGROUNDLOCKTIMEOUT in SystemParametersInfo).
>
>   * No menus are active.

I don't see an analogue here, because there's no way for us to manually
specify that Emacs received the last input event.

> Also from that page:
>
>   A process that can set the foreground window can enable another
>   process to set the foreground window by calling the
>   AllowSetForegroundWindow function. The process specified by
>   dwProcessId loses the ability to set the foreground window the next
>   time the user generates input, unless the input is directed at that
>   process, or the next time a process calls AllowSetForegroundWindow,
>   unless that process is specified.

That's different, because another program is explictly prohibiting a
program from setting the input focus.  Whether or not input was received
is also accounted for by the window system, not the program itself.

Also, is Emacs a "Modern Application" on MS-Windows? I thought it still
runs on Windows 9X.

> Guess what API emacsclient calls.

Yet x-focus-frame works.

> It's a generic hint that only one or two backends care about right
> now. That's not the same as a leaky abstraction.

It's not a hint at all.

> Platform-implementation code shouldn't have to know about platform
> specifics. That's why frame operations should be generic and
> polymorphic, not ad-hoc and gated behind type tests.

So it doesn't have to apply such a hint.  It only has to run
x-focus-frame, and as a result the frame is activated.

> I'll say it again: server.el hinting to Emacs that the user has
> interacted with a frame is not an implementation detail of a window
> system.

> Startup notification isn't suitable here because we're not starting
> anything.

It's the only protocol that transfers the X server timestamp at which a
user launched a program (by dropping a file onto an Emacsclient desktop
icon) to the program being launched.

> Emacsclient could include *its* server time in the message.

How will emacsclient be able to send the server time reliably, ensuring
"correct ordering"?

Consider the following sequence of events:

emacsclient                        X server                     emacs
ChangeProperty ------------------->
                                   <other event> -------------->
               <------------------ PropertyNotify
                                   <other event> -------------->
ClientMessage  ------------------>
                                   <other event> -------------->
                                   ClientMessage -------------->

By the time the client message arrives, the timestamp will already be
out of date.

> Well, a related protocol would be nice. Feel free to propose one.

It would be useless since nobody else will support it at this point.
All active development is happening around Wayland (no matter how much
your or I despise that fact.)

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On 8/7/22 02:07, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
>
>> Yes, it does. That's the whole point of being the window manager. Are
>> you suggesting that application developers, not users, ought to get
>> the final word on what windows go where?
> _Emacs_ users and developers get the final word, not window manager
> developers.

Should Emacs set override-redirect all its frames, draw client-side 
decorations, and grab the keyboard all the time too --- just in case the 
window manager does something wrong? The purpose of a window manager is 
to manage windows: it's the part of the system tasked with 
what-goes-where policy. I'm genuinely confused about why you think Emacs 
in particular should go to lengths to contravene that policy.

This discussion is a great example of why OS developer shouldn't give 
application authors an "escape hatch" that lets them opt of policy 
enforcement under the theory that they must have some good reason for 
doing so. Turns out, everyone thinks he has a good reason.

>> Yes, they are sloppy, just like all other users and developers, you
>> and me included, are sloppy. What's the old quip? Ah, right: "If men
>> were angels, no laws would be necessary". If developers were perfect,
>> we wouldn't need ASLR, or memory protection, or file permissions, or
>> fuzzing, or memory-safe languages --- yet here we are.
> So you are saying that ASLR or memory protection is the same as focus
> stealing "prevention"? Seriously?

You're the one suggesting that Emacs developers are somehow talented 
enough not to abuse APIs.

>> Developers of Emacs are no more angelic than developers using any
>> other toolkit, and focus stealing prevention mitigates their mistakes
>> as much as it does any others. If a user doesn't want focus stealing
>> preventation, he can disable it or use a window manager that doesn't
>> provide it. It's not the place of Emacs to override the user's
>> preference.
> Why can't the user also customize `x-allow-focus-stealing' (see the
> patch I sent) to nil?  Or better, report a bug to the Emacs developers
> while at it?

Because 1) users won't know it's there, and 2) setting 
x-allow-focus-stealing to nil will break the legitimate emacsclient 
use-case, and 3) what if every program did this?

> Focus stealing prevention is not a user choice, and can't even be turned
> off in the popular window managers.

It's a preference in some window managers. That it's reached 
always-enabled status in others suggests that it's a good thing and a 
signaled user preference that Emacs should not attempt to defeat.

>> If a process filter tries to asynchronously raise a window when the
>> user is the middle of browsing cat pictures, and that user has
>> configured his WM to block attempts by applications in the background
>> to raise windows, the WM is right to block that raise attempt. The WM
>> is where policy belongs.
> I don't see
>
>> It does exist elsewhere. From MSDN's page on SetForegroundWindow
>> (https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setforegroundwindow)
>> :
>>
>>    The system restricts which processes can set the foreground window.
>>    A process can set the foreground window only if one of the
>> following
>>    conditions is true:
>>
>>    * The process is the foreground process.
>>
>>    * The process was started by the foreground process.
>>
>>    * The process received the last input event.
>>
>>    * There is no foreground process.
>>
>>    * The process is being debugged.
>>
>>    * The foreground process is not a Modern Application or the
>>      Start Screen.
>>
>>    * The foreground is not locked (see LockSetForegroundWindow).
>>
>>    * The foreground lock time-out has expired (see *
>>      SPI_GETFOREGROUNDLOCKTIMEOUT in SystemParametersInfo).
>>
>>    * No menus are active.
> I don't see an analogue here, because there's no way for us to manually
> specify that Emacs received the last input event.
>
>> Also from that page:
>>
>>    A process that can set the foreground window can enable another
>>    process to set the foreground window by calling the
>>    AllowSetForegroundWindow function. The process specified by
>>    dwProcessId loses the ability to set the foreground window the next
>>    time the user generates input, unless the input is directed at that
>>    process, or the next time a process calls AllowSetForegroundWindow,
>>    unless that process is specified.
> That's different, because another program is explictly prohibiting a
> program from setting the input focus.

The target of

AllowSetForegroundWindow *gains* the ability to set focus where it otherwise couldn't. It's a nice handoff mechanism.

> Whether or not input was received
> is also accounted for by the window system, not the program itself.

Yep. That's one way Win32 ends up being better than X.

> Also, is Emacs a "Modern Application" on MS-Windows? I thought it still
> runs on Windows 9X.


Emacs is every bit a modern Windows application. There's been a lot of 
effort put into graceful degradation when running under 
less-than-cutting-edge environments like Windows 95.

>> Guess what API emacsclient calls.
> Yet x-focus-frame works.
>
>> It's a generic hint that only one or two backends care about right
>> now. That's not the same as a leaky abstraction.
> It's not a hint at all.

Yes it is.

>
>> Platform-implementation code shouldn't have to know about platform
>> specifics. That's why frame operations should be generic and
>> polymorphic, not ad-hoc and gated behind type tests.
> So it doesn't have to apply such a hint.  It only has to run
> x-focus-frame, and as a result the frame is activated.

Sometimes contrary to what the user wants.

>> I'll say it again: server.el hinting to Emacs that the user has
>> interacted with a frame is not an implementation detail of a window
>> system.
>> Startup notification isn't suitable here because we're not starting
>> anything.
> It's the only protocol that transfers the X server timestamp at which a
> user launched a program (by dropping a file onto an Emacsclient desktop
> icon) to the program being launched.
>
>> Emacsclient could include *its* server time in the message.
> How will emacsclient be able to send the server time reliably, ensuring
> "correct ordering"?
>
> Consider the following sequence of events:
>
> emacsclient                        X server                     emacs
> ChangeProperty ------------------->
>                                     <other event> -------------->
>                 <------------------ PropertyNotify
>                                     <other event> -------------->
> ClientMessage  ------------------>
>                                     <other event> -------------->
>                                     ClientMessage -------------->
>
> By the time the client message arrives, the timestamp will already be
> out of date.

Yeah, you're right. A synthetic input event would work, wouldn't it? It 
doesn't matter, though, because emacsclient doesn't work that way.

>> Well, a related protocol would be nice. Feel free to propose one.
> It would be useless since nobody else will support it at this point.
> All active development is happening around Wayland (no matter how much
> your or I despise that fact.)

I'm not a Wayland fan either, but we're way past the "disagree and 
commit" phase on that one. I just hope that protocols emerge that let us 
claw back some of the lost flexibility of X11.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> Should Emacs set override-redirect all its frames, draw client-side
> decorations, and grab the keyboard all the time too --- just in case
> the window manager does something wrong? The purpose of a window
> manager is to manage windows: it's the part of the system tasked with
> what-goes-where policy. I'm genuinely confused about why you think
> Emacs in particular should go to lengths to contravene that policy.

Because our users asked for it, and such a policy is causing Emacs bugs.
Like various other places where we fight with the window manager.

> This discussion is a great example of why OS developer shouldn't give
> application authors an "escape hatch" that lets them opt of policy
> enforcement under the theory that they must have some good reason for
> doing so. Turns out, everyone thinks he has a good reason.

Fortunately for us, X was designed with a completely different attitude
in mind.

> You're the one suggesting that Emacs developers are somehow talented
> enough not to abuse APIs.

Well, your suggestion is also abusing an API, just in a different way.
_NET_WM_USER_TIME only applies to ButtonPress and KeyPress events (and
their input extension equivalents.)  Programs aren't supposed to get the
timestamp from the X server and set the user time, they're only supposed
to do so in response to a limited set of input events.

> Because 1) users won't know it's there, and 2) setting
> x-allow-focus-stealing to nil will break the legitimate emacsclient
> use-case, and 3) what if every program did this?

It will be documented in PROBLEMS, and emacsclient can be modified to
bind it to some other value in that case.  And Emacs isn't every
program.

> It's a preference in some window managers. That it's reached
> always-enabled status in others suggests that it's a good thing and a
> signaled user preference that Emacs should not attempt to defeat.

So what about everywhere else we bludgeon the window manager into
obeying our commands? Starting from the various frame resizing hacks
(Martin knows more) to actually determining window manager positioning
behavior and fighting with type "A" window managers over the position of
a frame? All of those solutions have worked fine over decades, so I
don't understand why this has to be any different.

> Yep. That's one way Win32 ends up being better than X.

But then the bug could never be fixed under MS Windows.

> Emacs is every bit a modern Windows application. There's been a lot of
> effort put into graceful degradation when running under
> less-than-cutting-edge environments like Windows 95.

I'm talking about the "Modern Application" referred to in the doc,
specifically this part:

>>>    * The foreground process is not a Modern Application or the
>>>      Start Screen.

> Yes it is.

Why, if there's no way to set it?

> Sometimes contrary to what the user wants.

Then the user can customize the variable to nil.

> Yeah, you're right. A synthetic input event would work, wouldn't it?

Synthesizing input events doesn't work either, because the X server will
cargo cult the timestamp you put in the event that's sent.  The only
halfway-reliable way is to change a property on the frame's window and
wait for the PropertyChange event.

(Grabbing the server doesn't work either because the input events that
pile up will get sent first, after the grab is released.)

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 4364 bytes --]



On August 7, 2022 02:41:25 Po Lu <luangruo@yahoo.com> wrote:

> Daniel Colascione <dancol@dancol.org> writes:
>
>> Should Emacs set override-redirect all its frames, draw client-side
>> decorations, and grab the keyboard all the time too --- just in case
>> the window manager does something wrong? The purpose of a window
>> manager is to manage windows: it's the part of the system tasked with
>> what-goes-where policy. I'm genuinely confused about why you think
>> Emacs in particular should go to lengths to contravene that policy.
>
> Because our users asked for it, and such a policy is causing Emacs bugs.
> Like various other places where we fight with the window manager.

Sure, but don't we have, in this instance, an option that doesn't involve 
fighting the window manager?


>
>
>> This discussion is a great example of why OS developer shouldn't give
>> application authors an "escape hatch" that lets them opt of policy
>> enforcement under the theory that they must have some good reason for
>> doing so. Turns out, everyone thinks he has a good reason.
>
> Fortunately for us, X was designed with a completely different attitude
> in mind.

It's definitely from a more naive time. You can tell by the lack of 
inter-client security.

>
>
>> You're the one suggesting that Emacs developers are somehow talented
>> enough not to abuse APIs.
>
> Well, your suggestion is also abusing an API, just in a different way.
> _NET_WM_USER_TIME only applies to ButtonPress and KeyPress events (and
> their input extension equivalents.)  Programs aren't supposed to get the
> timestamp from the X server and set the user time, they're only supposed
> to do so in response to a limited set of input events.

I'd argue that my suggestion is consistent with the spirit of the API and 
changes behavior only in a narrow case. Your proposal involves changing the 
behavior of every x-focus-frame call made when Emacs doesn't have focus, 
yes? (Also, aren't you worried that the focus check will lead to behavioral 
inconsistencies?)


>
>
>> Because 1) users won't know it's there, and 2) setting
>> x-allow-focus-stealing to nil will break the legitimate emacsclient
>> use-case, and 3) what if every program did this?
>
> It will be documented in PROBLEMS, and emacsclient can be modified to
> bind it to some other value in that case.  And Emacs isn't every
> program.
>
>> It's a preference in some window managers. That it's reached
>> always-enabled status in others suggests that it's a good thing and a
>> signaled user preference that Emacs should not attempt to defeat.
>
> So what about everywhere else we bludgeon the window manager into
> obeying our commands? Starting from the various frame resizing hacks
> (Martin knows more) to actually determining window manager positioning
> behavior and fighting with type "A" window managers over the position of
> a frame? All of those solutions have worked fine over decades, so I
> don't understand why this has to be any different.

Because there's a less invasive alternative.


>
>
>> Yep. That's one way Win32 ends up being better than X.
>
> But then the bug could never be fixed under MS Windows.

The bug doesn't exist on MS Windows because the Win32 API has a function 
specifically designed to help apps do what emacsclient wants to do.

>
>
>> Emacs is every bit a modern Windows application. There's been a lot of
>> effort put into graceful degradation when running under
>> less-than-cutting-edge environments like Windows 95.
>
> I'm talking about the "Modern Application" referred to in the doc,
> specifically this part:
>
>>>> * The foreground process is not a Modern Application or the
>>>> Start Screen.
>
>> Yes it is.
>
> Why, if there's no way to set it?
>
>> Sometimes contrary to what the user wants.
>
> Then the user can customize the variable to nil.

Customizing the variable to nil breaks emacsclient.

>
>
>> Yeah, you're right. A synthetic input event would work, wouldn't it?
>
> Synthesizing input events doesn't work either, because the X server will
> cargo cult the timestamp you put in the event that's sent.  The only
> halfway-reliable way is to change a property on the frame's window and
> wait for the PropertyChange event.
>
> (Grabbing the server doesn't work either because the input events that
> pile up will get sent first, after the grab is released.)


[-- Attachment #2: Type: text/html, Size: 10324 bytes --]

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> Sure, but don't we have, in this instance, an option that doesn't
> involve fighting the window manager?

No, we don't.  It's still fighting with the window manager, just in a
less well abstracted and more arbitrary way.

> It's definitely from a more naive time. You can tell by the lack of
> inter-client security.

The truth is very few people still have open X servers that aren't
protected by, at least, MIT-MAGIC-COOKIE-1.

> I'd argue that my suggestion is consistent with the spirit of the API
> and changes behavior only in a narrow case.

The spirit of the API has already been stretched so far that it is only
usefully interpreted when a window is first mapped.

> Your proposal involves changing the behavior of every x-focus-frame
> call made when Emacs doesn't have focus, yes?

Yes, it changes the call to work, as opposed to not work.

> (Also, aren't you worried that the focus check will lead to behavioral
> inconsistencies?)

What inconsistencies?

> Because there's a less invasive alternative.

It's more invasive, because it adds an extra Lisp function that
programmers have to remember to call.

> Customizing the variable to nil breaks emacsclient.

They can configure their window manager to allow emacsclient to raise
frames, or make emacsclient bind it to some other value that works for
them.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Eli Zaretskii]

> From: Daniel Colascione <dancol@dancol.org>
> Cc: emacs-devel@gnu.org
> Date: Sun, 07 Aug 2022 00:39:32 -0400
> 
> > Consider the following situation: a programmer is writing some code and
> > notices that `x-focus-frame' is not working.  But the doc string says it
> > should work, without calling magic functions to note "out-of-band
> > interaction".
> > I'm not concerned about how many bytes a terminal hook takes.  I'm
> > concerned about exposing a clean abstraction over the window system
> to
> > users and programmers working on display-independent parts of
> Emacs.
> 
> If Emacs, in the background, calls x-focus-frame to activate a window
> and there's no explicit user intent to activate that window, the WM
> is within its rights to reject the proposed change to the window
> stacking. In this case, then the focus stealing mechanism is working
> at intended. Adding a "no, I really mean it" flag to x-focus-frame
> would encourage people to break this mechanism by passing that flag
> whenever x-focus-frame didn't work no matter the reason it didn't
> work --- even when this function *shouldn't* work because Emacs
> really is trying to do something that would trigger focus stealing
> prevent heuristics. 
> 
> server.el is a special case: it's okay to break the usual event
> ordering here because the user *did* interact with Emacs, albeit
> through a side channel, not the X server. It's not that developers
> need to call two functions to make some API work, but rather,
> developers should call an additional function to communicate
> information to the core that allows an otherwise forbidden-by-design
> state transition to occur after all.
> 
> In other words, the clean abstraction *is* telling Emacs "Oh, by the
> way, the user interacted with this frame", not telling x-focus-frame
> "please, actually do your job for some reason". There's nothing X-
> specific about giving the Emacs core a hint that the user recently
> interacted with a frame in some manner not reflected in the normal
> flow of events. If most window systems want to ignore this hint,
> that's fine, but that doesn't make the hint a leaky abstraction.

I'm as far from understanding the fine technical details of X
interaction as it gets, but from the POV of an interested bystander,
it sounds like the practical difference between you two is whether in
the following snippet:

  --- a/lisp/server.el
  +++ b/lisp/server.el
  @@ -1721,7 +1721,9 @@ server-switch-buffer
		;; a minibuffer/dedicated-window (if there's no
  other).
		(error (pop-to-buffer next-buffer)))))))
       (when server-raise-frame
  -      (select-frame-set-input-focus (window-frame)))))
  +      (let ((frame (window-frame)))
  +        (frame-note-oob-interaction frame)
  +        (select-frame-set-input-focus frame)))))

we should call a special function that Daniel suggests to add, or
introduce a new optional argument to select-frame-set-input-focus to
force raising the frame, is that right?

Daniel, you say that adding such a "force" argument will encourage
Lisp programs to abuse it, but wouldn't they be able to abuse the new
function in the same way?

You also say that the issue is more general than just that of raising
a frame when it gets focus, but do we actually know about other
situations where that could be an issue?  If we do, then indeed a more
general approach is more beneficial, IMO.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On Sun, 2022-08-07 at 08:41 +0300, Eli Zaretskii wrote:
> > From: Daniel Colascione <dancol@dancol.org>
> > Cc: emacs-devel@gnu.org
> > Date: Sun, 07 Aug 2022 00:39:32 -0400
> > 
> > > Consider the following situation: a programmer is writing some
> > > code and
> > > notices that `x-focus-frame' is not working.  But the doc
> > > string says it
> > > should work, without calling magic functions to note "out-of-
> > > band
> > > interaction".
> > > I'm not concerned about how many bytes a terminal hook takes. 
> > > I'm
> > > concerned about exposing a clean abstraction over the window
> > > system
> > to
> > > users and programmers working on display-independent parts of
> > Emacs.
> > 
> > If Emacs, in the background, calls x-focus-frame to activate a
> > window
> > and there's no explicit user intent to activate that window, the
> > WM
> > is within its rights to reject the proposed change to the window
> > stacking. In this case, then the focus stealing mechanism is
> > working
> > at intended. Adding a "no, I really mean it" flag to x-focus-
> > frame
> > would encourage people to break this mechanism by passing that
> > flag
> > whenever x-focus-frame didn't work no matter the reason it didn't
> > work --- even when this function *shouldn't* work because Emacs
> > really is trying to do something that would trigger focus
> > stealing
> > prevent heuristics. 
> > 
> > server.el is a special case: it's okay to break the usual event
> > ordering here because the user *did* interact with Emacs, albeit
> > through a side channel, not the X server. It's not that
> > developers
> > need to call two functions to make some API work, but rather,
> > developers should call an additional function to communicate
> > information to the core that allows an otherwise forbidden-by-
> > design
> > state transition to occur after all.
> > 
> > In other words, the clean abstraction *is* telling Emacs "Oh, by
> > the
> > way, the user interacted with this frame", not telling x-focus-
> > frame
> > "please, actually do your job for some reason". There's nothing
> > X-
> > specific about giving the Emacs core a hint that the user
> > recently
> > interacted with a frame in some manner not reflected in the
> > normal
> > flow of events. If most window systems want to ignore this hint,
> > that's fine, but that doesn't make the hint a leaky abstraction.
> 
> I'm as far from understanding the fine technical details of X
> interaction as it gets, but from the POV of an interested
> bystander,
> it sounds like the practical difference between you two is whether
> in
> the following snippet:
> 
>   --- a/lisp/server.el
>   +++ b/lisp/server.el
>   @@ -1721,7 +1721,9 @@ server-switch-buffer
> 		;; a minibuffer/dedicated-window (if there's no
>   other).
> 		(error (pop-to-buffer next-buffer)))))))
>        (when server-raise-frame
>   -      (select-frame-set-input-focus (window-frame)))))
>   +      (let ((frame (window-frame)))
>   +        (frame-note-oob-interaction frame)
>   +        (select-frame-set-input-focus frame)))))
> 
> we should call a special function that Daniel suggests to add, or
> introduce a new optional argument to select-frame-set-input-focus
> to
> force raising the frame, is that right?
> 
> Daniel, you say that adding such a "force" argument will encourage
> Lisp programs to abuse it, but wouldn't they be able to abuse the
> new
> function in the same way?

I think it's important not only to be aware of what developers *can*
do with an API, but of what kinds of solutions the spelling and shape
of that API will guide them towards. Here, I think developers who
want to raise windows in the background contrary to user preference
are more likely to add a "force" flag to their x-focus-window calls 
than to explicitly lie to Emacs about the user having interacted with
a frame when he hasn't --- in the latter case, I think they're more
likely to be aware that they're subverting user intent.

> 
> You also say that the issue is more general than just that of
> raising
> a frame when it gets focus, but do we actually know about other
> situations where that could be an issue?  If we do, then indeed a
> more
> general approach is more beneficial, IMO.

Here's one approach I thought about: while we're executing code in
the context of the server, dynamically bind a variable that select-
frame inspects --- say, select-frame-mark-interacted. When this
variable is true, select-frame (if not NORECORD) would mark each
frame selected in the course of execution as having been "interacted
with" by the user. This way, anything we do on behalf of the user
while executing an action from emacsclient gets counted as user
activity without our having to call (frame-note-oob-interaction
frame) in that (when server-raise-frame ...).

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm as far from understanding the fine technical details of X
> interaction as it gets, but from the POV of an interested bystander,
> it sounds like the practical difference between you two is whether in
> the following snippet:
>
>   --- a/lisp/server.el
>   +++ b/lisp/server.el
>   @@ -1721,7 +1721,9 @@ server-switch-buffer
> 		;; a minibuffer/dedicated-window (if there's no
>   other).
> 		(error (pop-to-buffer next-buffer)))))))
>        (when server-raise-frame
>   -      (select-frame-set-input-focus (window-frame)))))
>   +      (let ((frame (window-frame)))
>   +        (frame-note-oob-interaction frame)
>   +        (select-frame-set-input-focus frame)))))
>
> we should call a special function that Daniel suggests to add, or
> introduce a new optional argument to select-frame-set-input-focus to
> force raising the frame, is that right?

Right, but after digging some more I would rather just make
x-focus-frame activate the frame by default, as long as noactivate is
non-nil.  As documented.

> You also say that the issue is more general than just that of raising
> a frame when it gets focus, but do we actually know about other
> situations where that could be an issue?  If we do, then indeed a more
> general approach is more beneficial, IMO.

I can't think of any more general issue here.

Anyway, this patch makes x-focus-frame work as documented.  Daniel,
please see if it works for you (on and outside GTK) with the original
recipe in the bug report:

diff --git a/src/xfns.c b/src/xfns.c
index 672097c0d8..267e2a824c 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -2578,6 +2578,7 @@ append_wm_protocols (struct x_display_info *dpyinfo,
 #if !defined HAVE_GTK3 && defined HAVE_XSYNC
   bool found_wm_sync_request = false;
 #endif
+  bool found_wm_take_focus = false;
   unsigned long bytes_after;
 
   block_input ();
@@ -2601,6 +2602,9 @@ append_wm_protocols (struct x_display_info *dpyinfo,
 		   == dpyinfo->Xatom_net_wm_sync_request)
 	    found_wm_sync_request = true;
 #endif
+	  else if (existing_protocols[nitems]
+		   == dpyinfo->Xatom_wm_take_focus)
+	    found_wm_take_focus = true;
 	}
     }
 
@@ -2609,6 +2613,8 @@ append_wm_protocols (struct x_display_info *dpyinfo,
 
   if (!found_wm_ping)
     protos[num_protos++] = dpyinfo->Xatom_net_wm_ping;
+  if (!found_wm_take_focus)
+    protos[num_protos++] = dpyinfo->Xatom_wm_take_focus;
 #if !defined HAVE_GTK3 && defined HAVE_XSYNC
   if (!found_wm_sync_request && dpyinfo->xsync_supported_p)
     protos[num_protos++] = dpyinfo->Xatom_net_wm_sync_request;
diff --git a/src/xterm.c b/src/xterm.c
index 97985c8d9e..b5487f7e9d 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -17295,6 +17295,10 @@ handle_one_xevent (struct x_display_info *dpyinfo,
                   }
                 /* Not certain about handling scroll bars here */
 #endif
+		/* Set the last focus time to the time provided inside
+		   the _WM_TAKE_FOCUS message.  */
+
+		dpyinfo->last_focus_time = event->xclient.data.l[1];
 		goto done;
               }
 
@@ -25883,6 +25887,44 @@ xembed_request_focus (struct frame *f)
 			 XEMBED_REQUEST_FOCUS, 0, 0, 0);
 }
 
+static Bool
+server_timestamp_predicate (Display *display,
+			    XEvent *xevent,
+			    XPointer arg)
+{
+  XID *args = (XID *) arg;
+
+  if (xevent->type == PropertyNotify
+      && xevent->xproperty.window == args[0]
+      && xevent->xproperty.atom == args[1])
+    return True;
+
+  return False;
+}
+
+/* Get the server time.  The X server is guaranteed to deliver the
+   PropertyNotify event, so there is no reason to use x_if_event.  */
+
+static Time
+x_get_server_time (struct frame *f)
+{
+  Atom property_atom;
+  XEvent property_dummy;
+  struct x_display_info *dpyinfo;
+
+  dpyinfo = FRAME_DISPLAY_INFO (f);
+  property_atom = dpyinfo->Xatom_EMACS_SERVER_TIME_PROP;
+
+  XChangeProperty (dpyinfo->display, FRAME_OUTER_WINDOW (f),
+		   property_atom, XA_ATOM, 32,
+		   PropModeReplace, (unsigned char *) &property_atom, 1);
+
+  XIfEvent (dpyinfo->display, &property_dummy, server_timestamp_predicate,
+	    (XPointer) &(XID[]) {FRAME_OUTER_WINDOW (f), property_atom});
+
+  return property_dummy.xproperty.time;
+}
+
 /* Activate frame with Extended Window Manager Hints */
 
 static void
@@ -25890,11 +25932,11 @@ x_ewmh_activate_frame (struct frame *f)
 {
   XEvent msg;
   struct x_display_info *dpyinfo;
+  Time time;
 
   dpyinfo = FRAME_DISPLAY_INFO (f);
 
-  if (FRAME_VISIBLE_P (f)
-      && x_wm_supports (f, dpyinfo->Xatom_net_active_window))
+  if (FRAME_VISIBLE_P (f))
     {
       /* See the documentation at
 	 https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html
@@ -25904,13 +25946,52 @@ x_ewmh_activate_frame (struct frame *f)
       msg.xclient.message_type = dpyinfo->Xatom_net_active_window;
       msg.xclient.format = 32;
       msg.xclient.data.l[0] = 1;
-      msg.xclient.data.l[1] = dpyinfo->last_user_time;
+      msg.xclient.data.l[1] = (dpyinfo->last_user_time > dpyinfo->last_focus_time
+			       ? dpyinfo->last_user_time
+			       : dpyinfo->last_focus_time);
       msg.xclient.data.l[2] = (!dpyinfo->x_focus_frame
 			       ? None
 			       : FRAME_OUTER_WINDOW (dpyinfo->x_focus_frame));
       msg.xclient.data.l[3] = 0;
       msg.xclient.data.l[4] = 0;
 
+      /* No frame is currently focused on that display, so apply any
+	 bypass for focus stealing prevention that the user has
+	 specified.  */
+      if (!dpyinfo->x_focus_frame)
+	{
+	  if (EQ (Vx_allow_focus_stealing, Qimitate_pager))
+	    msg.xclient.data.l[0] = 2;
+	  else if (EQ (Vx_allow_focus_stealing, Qnewer_time))
+	    {
+	      block_input ();
+	      time = x_get_server_time (f);
+#ifdef USE_GTK
+	      x_set_gtk_user_time (f, time);
+#endif
+	      /* Temporarily override dpyinfo->x_focus_frame so the
+		 user time property is set on the right window.  */
+	      dpyinfo->x_focus_frame = f;
+	      x_display_set_last_user_time (dpyinfo, time, true);
+	      dpyinfo->x_focus_frame = NULL;
+	      unblock_input ();
+
+	      msg.xclient.data.l[1] = time;
+	    }
+	  else if (EQ (Vx_allow_focus_stealing, Qraise_and_focus))
+	    {
+	      time = x_get_server_time (f);
+
+	      x_ignore_errors_for_next_request (dpyinfo);
+	      XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
+			      RevertToParent, time);
+	      XRaiseWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f));
+	      x_stop_ignoring_errors (dpyinfo);
+
+	      return;
+	    }
+	}
+
       XSendEvent (dpyinfo->display, dpyinfo->root_window,
 		  False, (SubstructureRedirectMask
 			  | SubstructureNotifyMask), &msg);
@@ -25954,14 +26035,19 @@ x_focus_frame (struct frame *f, bool noactivate)
     xembed_request_focus (f);
   else
     {
-      /* Ignore any BadMatch error this request might result in.  */
-      x_ignore_errors_for_next_request (dpyinfo);
-      XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
-		      RevertToParent, CurrentTime);
-      x_stop_ignoring_errors (dpyinfo);
-
-      if (!noactivate)
+      if (!noactivate && NILP (Vx_no_window_manager)
+	  && x_wm_supports (f, dpyinfo->Xatom_net_active_window))
+	/* Calling XSetInputFocus manually can be skipped, since
+	   activating a frame means to make it the input focus.  */
 	x_ewmh_activate_frame (f);
+      else
+	{
+	  /* Ignore any BadMatch error this request might result in.  */
+	  x_ignore_errors_for_next_request (dpyinfo);
+	  XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
+			  RevertToParent, CurrentTime);
+	  x_stop_ignoring_errors (dpyinfo);
+	}
     }
 }
 
@@ -29068,6 +29154,9 @@ syms_of_xterm (void)
   Fput (Qsuper, Qmodifier_value, make_fixnum (super_modifier));
   DEFSYM (QXdndSelection, "XdndSelection");
   DEFSYM (Qx_selection_alias_alist, "x-selection-alias-alist");
+  DEFSYM (Qimitate_pager, "imitate-pager");
+  DEFSYM (Qnewer_time, "newer-time");
+  DEFSYM (Qraise_and_focus, "raise-and-focus");
 
   DEFVAR_LISP ("x-ctrl-keysym", Vx_ctrl_keysym,
     doc: /* Which keys Emacs uses for the ctrl modifier.
@@ -29294,4 +29383,23 @@ syms_of_xterm (void)
 In addition, when this variable is a list, only preserve the
 selections whose names are contained within.  */);
   Vx_auto_preserve_selections = list2 (QCLIPBOARD, QPRIMARY);
+
+  DEFVAR_LISP ("x-allow-focus-stealing", Vx_allow_focus_stealing,
+    doc: /* How to bypass window manager focus stealing prevention.
+
+Some window managers prevent `x-focus-frame' from activating the given
+frame when Emacs is in the background.  This variable specifies the
+strategy used to activate frames when that is the case, and has
+several valid values (any other value means to not bypass window
+manager focus stealing prevention):
+
+  - The symbol `imitate-pager', which means to pretend that Emacs is a
+    pager.
+
+  - The symbol `newer-time', which means to fetch the current time
+    from the X server and use it to activate the frame.
+
+  - The symbol `raise-and-focus', which means to raise the window and
+    focus it manually.  */);
+  Vx_allow_focus_stealing = Qnewer_time;
 }
diff --git a/src/xterm.h b/src/xterm.h
index 7be0f2ede6..b7f8edb98f 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -538,8 +538,9 @@ #define MAX_CLIP_RECTS 2
   struct scroll_bar *last_mouse_scroll_bar;
 
   /* Time of last user interaction as returned in X events on this
-     display.  */
-  Time last_user_time;
+     display, and the time of the last window-manager assisted
+     focus.  */
+  Time last_user_time, last_focus_time;
 
   /* Position where the mouse was last time we reported a motion.
      This is a position on last_mouse_motion_frame.  */

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On Sun, 2022-08-07 at 13:53 +0800, Po Lu wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm as far from understanding the fine technical details of X
> > interaction as it gets, but from the POV of an interested bystander,
> > it sounds like the practical difference between you two is whether in
> > the following snippet:
> > 
> >   --- a/lisp/server.el
> >   +++ b/lisp/server.el
> >   @@ -1721,7 +1721,9 @@ server-switch-buffer
> > 		;; a minibuffer/dedicated-window (if there's no
> >   other).
> > 		(error (pop-to-buffer next-buffer)))))))
> >        (when server-raise-frame
> >   -      (select-frame-set-input-focus (window-frame)))))
> >   +      (let ((frame (window-frame)))
> >   +        (frame-note-oob-interaction frame)
> >   +        (select-frame-set-input-focus frame)))))
> > 
> > we should call a special function that Daniel suggests to add, or
> > introduce a new optional argument to select-frame-set-input-focus to
> > force raising the frame, is that right?
> 
> Right, but after digging some more I would rather just make
> x-focus-frame activate the frame by default, as long as noactivate is
> non-nil.  As documented.

And I'd rather not explicitly bypass window manager policy. Let's not
get into an arms race.

By the way: if we're keen on making things work as documented,
shouldn't we add a bunch of workarounds for x-raise-frame, which
basically does nothing right now?

> 
> > You also say that the issue is more general than just that of raising
> > a frame when it gets focus, but do we actually know about other
> > situations where that could be an issue?  If we do, then indeed a more
> > general approach is more beneficial, IMO.
> 
> I can't think of any more general issue here.
> 
> Anyway, this patch makes x-focus-frame work as documented.  Daniel,
> please see if it works for you (on and outside GTK) with the original
> recipe in the bug report:

Don't we still have the problem of XSetInputFocus succeeding while
the EWMH activation fails, leaving the whole window stack in a state
confusing for both the human and WM components of the system?

> 
> diff --git a/src/xfns.c b/src/xfns.c
> index 672097c0d8..267e2a824c 100644
> --- a/src/xfns.c
> +++ b/src/xfns.c
> @@ -2578,6 +2578,7 @@ append_wm_protocols (struct x_display_info *dpyinfo,
>  #if !defined HAVE_GTK3 && defined HAVE_XSYNC
>    bool found_wm_sync_request = false;
>  #endif
> +  bool found_wm_take_focus = false;
>    unsigned long bytes_after;
>  
>    block_input ();
> @@ -2601,6 +2602,9 @@ append_wm_protocols (struct x_display_info *dpyinfo,
>  		   == dpyinfo->Xatom_net_wm_sync_request)
>  	    found_wm_sync_request = true;
>  #endif
> +	  else if (existing_protocols[nitems]
> +		   == dpyinfo->Xatom_wm_take_focus)
> +	    found_wm_take_focus = true;
>  	}
>      }
>  
> @@ -2609,6 +2613,8 @@ append_wm_protocols (struct x_display_info *dpyinfo,
>  
>    if (!found_wm_ping)
>      protos[num_protos++] = dpyinfo->Xatom_net_wm_ping;
> +  if (!found_wm_take_focus)
> +    protos[num_protos++] = dpyinfo->Xatom_wm_take_focus;
>  #if !defined HAVE_GTK3 && defined HAVE_XSYNC
>    if (!found_wm_sync_request && dpyinfo->xsync_supported_p)
>      protos[num_protos++] = dpyinfo->Xatom_net_wm_sync_request;
> diff --git a/src/xterm.c b/src/xterm.c
> index 97985c8d9e..b5487f7e9d 100644
> --- a/src/xterm.c
> +++ b/src/xterm.c
> @@ -17295,6 +17295,10 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>                    }
>                  /* Not certain about handling scroll bars here */
>  #endif
> +		/* Set the last focus time to the time provided inside
> +		   the _WM_TAKE_FOCUS message.  */
> +
> +		dpyinfo->last_focus_time = event->xclient.data.l[1];
>  		goto done;
>                }
>  
> @@ -25883,6 +25887,44 @@ xembed_request_focus (struct frame *f)
>  			 XEMBED_REQUEST_FOCUS, 0, 0, 0);
>  }
>  
> +static Bool
> +server_timestamp_predicate (Display *display,
> +			    XEvent *xevent,
> +			    XPointer arg)
> +{
> +  XID *args = (XID *) arg;
> +
> +  if (xevent->type == PropertyNotify
> +      && xevent->xproperty.window == args[0]
> +      && xevent->xproperty.atom == args[1])
> +    return True;
> +
> +  return False;
> +}
> +
> +/* Get the server time.  The X server is guaranteed to deliver the
> +   PropertyNotify event, so there is no reason to use x_if_event.  */

Even if the window disappears while we're waiting?

> +
> +static Time
> +x_get_server_time (struct frame *f)
> +{
> +  Atom property_atom;
> +  XEvent property_dummy;
> +  struct x_display_info *dpyinfo;
> +
> +  dpyinfo = FRAME_DISPLAY_INFO (f);
> +  property_atom = dpyinfo->Xatom_EMACS_SERVER_TIME_PROP;
> +
> +  XChangeProperty (dpyinfo->display, FRAME_OUTER_WINDOW (f),
> +		   property_atom, XA_ATOM, 32,
> +		   PropModeReplace, (unsigned char *) &property_atom, 1);
> +
> +  XIfEvent (dpyinfo->display, &property_dummy, server_timestamp_predicate,
> +	    (XPointer) &(XID[]) {FRAME_OUTER_WINDOW (f), property_atom});
> +
> +  return property_dummy.xproperty.time;
> +}
> +
>  /* Activate frame with Extended Window Manager Hints */
>  
>  static void
> @@ -25890,11 +25932,11 @@ x_ewmh_activate_frame (struct frame *f)
>  {
>    XEvent msg;
>    struct x_display_info *dpyinfo;
> +  Time time;
>  
>    dpyinfo = FRAME_DISPLAY_INFO (f);
>  
> -  if (FRAME_VISIBLE_P (f)
> -      && x_wm_supports (f, dpyinfo->Xatom_net_active_window))
> +  if (FRAME_VISIBLE_P (f))
>      {
>        /* See the documentation at
>  	 https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html
> @@ -25904,13 +25946,52 @@ x_ewmh_activate_frame (struct frame *f)
>        msg.xclient.message_type = dpyinfo->Xatom_net_active_window;
>        msg.xclient.format = 32;
>        msg.xclient.data.l[0] = 1;
> -      msg.xclient.data.l[1] = dpyinfo->last_user_time;
> +      msg.xclient.data.l[1] = (dpyinfo->last_user_time > dpyinfo->last_focus_time
> +			       ? dpyinfo->last_user_time
> +			       : dpyinfo->last_focus_time);
>        msg.xclient.data.l[2] = (!dpyinfo->x_focus_frame
>  			       ? None
>  			       : FRAME_OUTER_WINDOW (dpyinfo->x_focus_frame));
>        msg.xclient.data.l[3] = 0;
>        msg.xclient.data.l[4] = 0;
>  
> +      /* No frame is currently focused on that display, so apply any
> +	 bypass for focus stealing prevention that the user has
> +	 specified.  */
> +      if (!dpyinfo->x_focus_frame)
> +	{
> +	  if (EQ (Vx_allow_focus_stealing, Qimitate_pager))
> +	    msg.xclient.data.l[0] = 2;
> +	  else if (EQ (Vx_allow_focus_stealing, Qnewer_time))
> +	    {
> +	      block_input ();
> +	      time = x_get_server_time (f);
> +#ifdef USE_GTK
> +	      x_set_gtk_user_time (f, time);
> +#endif
> +	      /* Temporarily override dpyinfo->x_focus_frame so the
> +		 user time property is set on the right window.  */
> +	      dpyinfo->x_focus_frame = f;
> +	      x_display_set_last_user_time (dpyinfo, time, true);
> +	      dpyinfo->x_focus_frame = NULL;
> +	      unblock_input ();
> +
> +	      msg.xclient.data.l[1] = time;
> +	    }
> +	  else if (EQ (Vx_allow_focus_stealing, Qraise_and_focus))
> +	    {
> +	      time = x_get_server_time (f);
> +
> +	      x_ignore_errors_for_next_request (dpyinfo);
> +	      XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
> +			      RevertToParent, time);
> +	      XRaiseWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f));
> +	      x_stop_ignoring_errors (dpyinfo);
> +
> +	      return;
> +	    }
> +	}
> +
>        XSendEvent (dpyinfo->display, dpyinfo->root_window,
>  		  False, (SubstructureRedirectMask
>  			  | SubstructureNotifyMask), &msg);
> @@ -25954,14 +26035,19 @@ x_focus_frame (struct frame *f, bool noactivate)
>      xembed_request_focus (f);
>    else
>      {
> -      /* Ignore any BadMatch error this request might result in.  */
> -      x_ignore_errors_for_next_request (dpyinfo);
> -      XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
> -		      RevertToParent, CurrentTime);
> -      x_stop_ignoring_errors (dpyinfo);
> -
> -      if (!noactivate)
> +      if (!noactivate && NILP (Vx_no_window_manager)
> +	  && x_wm_supports (f, dpyinfo->Xatom_net_active_window))
> +	/* Calling XSetInputFocus manually can be skipped, since
> +	   activating a frame means to make it the input focus.  */
>  	x_ewmh_activate_frame (f);
> +      else
> +	{
> +	  /* Ignore any BadMatch error this request might result in.  */
> +	  x_ignore_errors_for_next_request (dpyinfo);
> +	  XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
> +			  RevertToParent, CurrentTime);
> +	  x_stop_ignoring_errors (dpyinfo);
> +	}
>      }
>  }
>  
> @@ -29068,6 +29154,9 @@ syms_of_xterm (void)
>    Fput (Qsuper, Qmodifier_value, make_fixnum (super_modifier));
>    DEFSYM (QXdndSelection, "XdndSelection");
>    DEFSYM (Qx_selection_alias_alist, "x-selection-alias-alist");
> +  DEFSYM (Qimitate_pager, "imitate-pager");
> +  DEFSYM (Qnewer_time, "newer-time");
> +  DEFSYM (Qraise_and_focus, "raise-and-focus");
>  
>    DEFVAR_LISP ("x-ctrl-keysym", Vx_ctrl_keysym,
>      doc: /* Which keys Emacs uses for the ctrl modifier.
> @@ -29294,4 +29383,23 @@ syms_of_xterm (void)
>  In addition, when this variable is a list, only preserve the
>  selections whose names are contained within.  */);
>    Vx_auto_preserve_selections = list2 (QCLIPBOARD, QPRIMARY);
> +
> +  DEFVAR_LISP ("x-allow-focus-stealing", Vx_allow_focus_stealing,
> +    doc: /* How to bypass window manager focus stealing prevention.


It's hard to imagine any user sitting down and tweaking this
variable. This seems like one of those preferences added not to
account for differences in taste, but to punt a hard technical choice
to users not prepared to make it.

> +
> +Some window managers prevent `x-focus-frame' from activating the given
> +frame when Emacs is in the background.  This variable specifies the
> +strategy used to activate frames when that is the case, and has
> +several valid values (any other value means to not bypass window
> +manager focus stealing prevention):


Again, I disagree philosophically with the thrust of this patch.
Focus stealing prevention isn't some nuisance to work around. If
Emacs properly manages user-interaction timestamps, there's no need
to work around it. We don't need to pretend we're a panel or
something if we just give the window manager the information it needs
to make the right decision.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> And I'd rather not explicitly bypass window manager policy. Let's not
> get into an arms race.

We already do that all the time.  Just a few examples:
x-mouse-click-focus-ignore-position, x-wait-for-event-timeout,
x-frame-normalize-before-maximize, x-set-frame-visibility-more-laxly,
x-input-grab-touch-events.  Martin can probably name more.

> By the way: if we're keen on making things work as documented,
> shouldn't we add a bunch of workarounds for x-raise-frame, which
> basically does nothing right now?

raise-frame is supposed to raise the frame without changing the input
focus, but if it doesn't work it should be fixed.

> Don't we still have the problem of XSetInputFocus succeeding while
> the EWMH activation fails, leaving the whole window stack in a state
> confusing for both the human and WM components of the system?

XSetInputFocus is no longer called when EWMH activation is being used.
Calling it previously was a bug, it shouldn't be used with EWMH
activation.

> Even if the window disappears while we're waiting?

How can the frame be deleted while we're waiting?  I've never ever seen
such an unruly X program.

> It's hard to imagine any user sitting down and tweaking this
> variable. This seems like one of those preferences added not to
> account for differences in taste, but to punt a hard technical choice
> to users not prepared to make it.

I also wrote something in etc/PROBLEMS.  Users are expected to read that
when they experience problems, and to try the different options in order
until it starts working.

> Again, I disagree philosophically with the thrust of this patch.
> Focus stealing prevention isn't some nuisance to work around. If
> Emacs properly manages user-interaction timestamps, there's no need
> to work around it. We don't need to pretend we're a panel or
> something if we just give the window manager the information it needs
> to make the right decision.

If you (or a user) don't like it, the variable can be set to nil.
But it would be best for our functions to behave according to their
documentation first.

Please tell me if it works or not.

Re: master 4b98a79a50: Improve X event timestamp tracking

[Daniel Colascione]

On 8/7/22 02:15, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
>
>> And I'd rather not explicitly bypass window manager policy. Let's not
>> get into an arms race.
> We already do that all the time.  Just a few examples:
> x-mouse-click-focus-ignore-position, x-wait-for-event-timeout,
> x-frame-normalize-before-maximize, x-set-frame-visibility-more-laxly,
> x-input-grab-touch-events.  Martin can probably name more.

None of those things is about working against policy implemented by the 
system on behalf of the user. Your patch is explicitly about making 
Emacs defy that policy. Is that how well-behaved X clients should behave?

>> By the way: if we're keen on making things work as documented,
>> shouldn't we add a bunch of workarounds for x-raise-frame, which
>> basically does nothing right now?
> raise-frame is supposed to raise the frame without changing the input
> focus, but if it doesn't work it should be fixed.

Window managers typically don't let you just raise frames arbitrarily. 
What should the documentation of this function say?

>> Don't we still have the problem of XSetInputFocus succeeding while
>> the EWMH activation fails, leaving the whole window stack in a state
>> confusing for both the human and WM components of the system?
> XSetInputFocus is no longer called when EWMH activation is being used.
> Calling it previously was a bug, it shouldn't be used with EWMH
> activation.

Ok, thanks.

>> Even if the window disappears while we're waiting?
> How can the frame be deleted while we're waiting?  I've never ever seen
> such an unruly X program.
XDestroyWindow? Windows can die at any time, yes? Although I suppose if 
someone raced XDestroyWindow against XChangeProperty we'd either get the 
PropertyNotify or the xlib error callback would fire, depending on who 
won the race. I *think* we can't get into a situation in which we'd wait 
forever for the PropertyNotify. I'll have to double check.
>> It's hard to imagine any user sitting down and tweaking this
>> variable. This seems like one of those preferences added not to
>> account for differences in taste, but to punt a hard technical choice
>> to users not prepared to make it.
> I also wrote something in etc/PROBLEMS.  Users are expected to read that
> when they experience problems, and to try the different options in order
> until it starts working.

It's not realistic to expect users to read any file in etc. When users 
have a problem, they try again, then ask their friends, then search the 
internet, and then give up and install VSCode. Like it or not, that's 
the flow, and checked-in documentation won't help.

>> Again, I disagree philosophically with the thrust of this patch.
>> Focus stealing prevention isn't some nuisance to work around. If
>> Emacs properly manages user-interaction timestamps, there's no need
>> to work around it. We don't need to pretend we're a panel or
>> something if we just give the window manager the information it needs
>> to make the right decision.
> If you (or a user) don't like it, the variable can be set to nil.
> But it would be best for our functions to behave according to their
> documentation first.

Then let's change the documentation: add a note to x-focus-frame 
reminding callers that it's ultimately up to the window manager whether 
a window is activated --- and it is. A WM is free to ignore our 
activation requests even if we pretend to be a panel or a fire truck or 
a cool bird or whatever. Why implement three separate ways of working 
against that grain when we can do what the WM wants?

Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> None of those things is about working against policy implemented by
> the system on behalf of the user. Your patch is explicitly about
> making Emacs defy that policy. Is that how well-behaved X clients
> should behave?

Firstly, that's not true.  Secondly, as a general rule, there are no
well-behaved X clients in existence.  Let's see what GTK does:

      /* Translate a timestamp of GDK_CURRENT_TIME appropriately */
      if (timestamp == GDK_CURRENT_TIME)
        {
#ifdef GDK_WINDOWING_X11
	  if (GDK_IS_X11_WINDOW(gdk_window))
	    {
	      GdkDisplay *display;

	      display = gtk_widget_get_display (widget);
	      timestamp = gdk_x11_display_get_user_time (display);
	    }
	  else
#endif
	    timestamp = gtk_get_current_event_time ();

[...]

      gdk_window_focus (gdk_window, timestamp);

And Qt:

    updateNetWmUserTime(connection()->time());

[...]

        event.response_type = XCB_CLIENT_MESSAGE;
        event.format = 32;
        event.sequence = 0;
        event.window = m_window;
        event.type = atom(QXcbAtom::_NET_ACTIVE_WINDOW);
        event.data.data32[0] = 1;
        event.data.data32[1] = connection()->time();
        event.data.data32[2] = focusWindow ? focusWindow->winId() : XCB_NONE;
        event.data.data32[3] = 0;
        event.data.data32[4] = 0;

(where the connection time is updated by frame drawn events, so is in
effect always very close to the server time regardless of how much
interaction has happened.)

Focus stealing prevention only works with naive clients and very old
clients, along with newly mapped windows.

> Window managers typically don't let you just raise frames
> arbitrarily. What should the documentation of this function say?

XRaiseWindow works here on GNOME Shell, kwin and mwm/dtwm, which is what
I can easily test with.  Please tell me where it doesn't work, and I
will try to fix it.

> XDestroyWindow? Windows can die at any time, yes?

They can't, not through user interaction.  Nobody defends against
extremely unruly clients that kill toplevel windows at random, even
though the fix is as simple as grabbing the server.

> Although I suppose if someone raced XDestroyWindow against
> XChangeProperty we'd either get the PropertyNotify or the xlib error
> callback would fire, depending on who won the race. I *think* we can't
> get into a situation in which we'd wait forever for the
> PropertyNotify. I'll have to double check.

A hang is possible but not something that can practically happen.  You'd
have to be a genius to time the XDestroyWindow right between the
XChangeProperty and PropertyNotify.

> It's not realistic to expect users to read any file in etc. When users
> have a problem, they try again, then ask their friends, then search
> the internet, and then give up and install VSCode. Like it or not,
> that's the flow, and checked-in documentation won't help.

Here's how VS Code (Chromium) implements activating a frame:

  x11::Time timestamp = X11EventSource::GetInstance()->GetTimestamp();

  // override_redirect windows ignore _NET_ACTIVE_WINDOW.
  // https://crbug.com/940924
  if (wm_supports_active_window && !override_redirect_) {
    std::array<uint32_t, 5> data = {
        // We're an app.
        1,
        static_cast<uint32_t>(timestamp),
        // TODO(thomasanderson): if another chrome window is active, specify
        // that here.  The EWMH spec claims this may make the WM more likely to
        // service our _NET_ACTIVE_WINDOW request.
        0,
        0,
        0,
    };
    SendClientMessage(xwindow_, x_root_window_,
                      x11::GetAtom("_NET_ACTIVE_WINDOW"), data);

(also see https://codereview.chromium.org/2287583003/ for context)

once again, the timestamp is automatically obtained from the X server
(thus bypassing any focus stealing prevention) after a few optimizations
aimed at reducing round-trips are made.

> Then let's change the documentation: add a note to x-focus-frame
> reminding callers that it's ultimately up to the window manager
> whether a window is activated --- and it is. A WM is free to ignore
> our activation requests even if we pretend to be a panel or a fire
> truck or a cool bird or whatever. Why implement three separate ways of
> working against that grain when we can do what the WM wants?

That isn't what the documentation has said for more than 20 years, so
this would in effect be an incompatible change.