On Sun 07 Mar 2021 at 18:14:05 -0600, Matthew D. Fuller wrote:
> 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.

Some day...

> 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   :|

Yeah I just read the ICCM text on focus that you linked to and that
already is not so enlightening...

> > 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

That leads me to moving the code and comments around like below.
I added "passive and" to the first comment, and I took the
SynthesiseFocusIn() in the conditions. It could be called and also
SetFocus(), which would be duplicative. Also this mirrors the code above
better although it doesn't quite match the structure.

                        if (Scr->TitleFocus) {
                            if(Tmp_win->wmhints->input
                                            && Event.xcrossing.focus) {
                                SynthesiseFocusIn(Tmp_win->w);
                            }
                        } else {
                            if(Tmp_win->wmhints->input) {
                                /* passive and locally active clients need
                                   help from WM to get the input focus */
                                SetFocus(Tmp_win, ewp->time);
                            }
                            /* must deal with WM_TAKE_FOCUS clients now, if
                               we're not in TitleFocus mode */
                            if(Tmp_win->protocols & DoesWmTakeFocus) {
                                /* for both locally & globally active clnts */
                                SendTakeFocusMessage(Tmp_win, ewp->time);
                            }
                        }

This structure seems more comprehensible. However when I tried to apply
the same structure to the code above (for the frame), the icon manager
focus spoiled it. I could however condense the somewhat verbose
condition

                    bool icon_manager_focus =
                            Scr->IconManagerFocus &&
                            Tmp_win->iconmanagerlist &&
                            ewp->window == Tmp_win->iconmanagerlist->w;

which was used basically twice.

https://bazaar.launchpad.net/~rhialto/ctwm/fix-sloppy-notitlefocus/revision/677

> 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...

It does seem kind of weird that there is a case where both
wmhints->input and DoesWmTakeFocus are set, so that both
XSetInputFocus() and SendTakeFocusMessage() are called. But I can't read
from the ICCCM docs that it isn't meant that way...

> Matthew Fuller     (MF4839)   |  [email protected]
-Olaf.
-- 
___ Q: "What's an anagram of Banach-Tarski?"  -- Olaf "Rhialto" Seibert
\X/ A: "Banach-Tarski Banach-Tarski."         -- rhialto at falu dot nl

Attachment: signature.asc
Description: PGP signature

Reply via email to