On Thu, Mar 04, 2021 at 11:07:29PM +0100 I heard the voice of
Rhialto, and lo! it spake thus:
>
> I've been wondering about the use of SynthesiseFocusIn().
> SetFocus() does mostly the same as what HandleFocusOut() followed by
> HandleFocusIn() does, plus an actual call to XSetInputFocus(). And
> those are the exact functions that get called to handle the
> synthetic focus messages.  It would be cleaner perhaps to have
> either one method or the other but not both.

Well, we need the handling of HandleFocusX() because we may not always
be the initiators of focus changes, but we need to react to them.
Now, there is a fair measure of semi-duplicated code there, and
certainly the path through the event handlers is...   I mean, does
that even qualify as a path?   :>

It's probably not entirely unreasonable to imagine that all the "react
to focus change" things like decorations etc should be purely in
HandleFocusX, and SetFocus either (a) should just achieve them via the
synthesized events, or (2) should call a shared function for them.

But, the details of just what X actually sends are surprisingly
surprising; I spent a while tracking focus and enter stuff a while
back when working on somethingorother, and all my dreams of "this is
The Way X Will Always Signal This, so we can stop half-implementing
parts of it scattered all over different events" were dashed pretty
hard, and I decided it was simplest to assume it all worked perfectly
and pretend I never saw any of that code   :|


> I re-ordered it like below, and the results are a lot better.
> 
>                         if(!Scr->TitleFocus) {
>                             if(Tmp_win->wmhints->input) {
>                                 SetFocus(Tmp_win, ewp->time);
>                             }
>                             if (Tmp_win->protocols & DoesWmTakeFocus) {
>                                 SendTakeFocusMessage(Tmp_win, ewp->time);
>                             }
>                         }

This seems facially reasonable, in light of

> I'm also uncertain about what is meant with "both locally & globally
> active clients".

This is the ICCCM focus stuff:
https://www.x.org/releases/X11R7.7/doc/xorg-docs/icccm/icccm.html#Input_Focus

DoesWmTakeFocus is our indicator that it's one of the
{Locally,Globally} Active cases.  So the point of the comments is that
the Locally Active case should get a SetFocus() call, but Globally
shouldn't.  And both Locally and Globally should get the
SendTakeFocusMessage().

But with the matrix as is, we only need the two orthagonal checks;
Passive and Locally Active would both have wmhints->input set, and
those are the only cases where we'd want to SetFocus().  And only the
*Active's will have DoesWmTakeFocus, and only they should get the
SendTakeFocusMessage().  So the existing comments are correct-ish,
though probably slightly misleading in that they suggest a diff
between Locally Active and Passive, in a place where there isn't
one...


-- 
Matthew Fuller     (MF4839)   |  [email protected]
Systems/Network Administrator |  http://www.over-yonder.net/~fullermd/
           On the Internet, nobody can hear you scream.

Reply via email to