wmutils / opt

optional addons to wmutils
Other
114 stars 24 forks source link

fix focus events #14

Closed ivzave closed 8 years ago

ivzave commented 8 years ago

Some applications transfer focus by themselves. It makes sense to react to focus changes (events 9 and 10) as they appear instead of always expecting focus to be where we have it set to be. This requires filtering some garbage events which is done in this patch. Works fine at least for me.

z3bra commented 8 years ago

I don't understand what the issue is. Could you describe a concrete example please? Also, thanks for following my "code style" :P

ivzave commented 8 years ago

Concrete example: when I click an URL in some application it could be opened in new tab in existing browser window. Such window will get focus then but it won't be highlighted (chwb) since no new window is mapped. Also sometimes windows aren't correctly highlighted when being mapped because mapping notify comes before the window is able to react to appropriate requests for some reason, e.g. opening new Chrome window. My approach is to use chwb in response to actual focus changes as reported by X.

z3bra commented 8 years ago

I can see the use to call chwb on focus change instead of mapping requests, but why do you need the filter out some focus events?

z3bra commented 8 years ago

I still can't understand what this "fix" brings...

ivzave commented 8 years ago

We are actually interested only in focus switches between top level windows. I've used this page https://tronche.com/gui/x/xlib/events/input-focus/normal-and-grabbed.html to implement filtering. As you can see the NotifyNonlinear is the only kind of events we are interested in. But for some reason I didn't sort out this does fail sometimes. Empyrically I've figured out adding also NotifyAncestor makes this to work pretty well.

z3bra commented 8 years ago

I fail to understand how this page explains that we're only interrested in NotifyNonLinear events. You, state that we should only raise focus events on top level window (I disagree with you here, as this might not be the case for everyone), yet, NotifyNonLinear can be set on events related to a child window. Also, as your tests demonstrate, using only this detail fails to raise all the events you personally want. You then started to add new details randomly until it semingly works, but don't understand why it does. We're not gonna merge something that seems to works by magic, and that nobody understands.

Finally, I still cannot understsnt what the ISSUE is there. You're saying what we want, but not WHY we want that. Please develop your point. Otherwise, I'll just close this PR because it will be a waste of both our times.

ivzave commented 8 years ago

X generates a bunch of events for each focus transfer. If you read carefully page I've referenced you can notice for example that X will fire focus events on each window in window tree between one that lost focus and one that got it, and also potentially on each window between window having mouse pointer and windows involved in focus transfer. Just throwing all these focus events to outside losing any detail except window id is completely useless. E.g. reacting to such events by un/highlighting windows leads to just flickering and wrong resulting highlighted window. These events are generated in expectation of user filtering them according to his interest. ISSUE here is lack of appropriate filter and lack of possibility to implement it outside since we drop all essential details. Two options I see to fix this:

  1. Also report detail and mode to user so we can filter events outside (breaks interface - we report only event id and window id for all other events).
  2. Pass masks for focus events in additional arguments to wew.
  3. Implement some sensible filter in wew. I've chosen third option because it was done this way for enter events. The case I see most meaningful (and also I personally interested in) is focus events on top level windows. You've mentioned this might not be the case for everyone. I can reply any specific filter implemented here might not be the case for everyone, but current behaviour IS case for noone. Regarding implementation (if we decide to report focus events on top level windows) I realize I don't have full understanding here. I'll try to find some time to figure things out and maybe improve filter if you agree it has sense to implement it in wew itself.
z3bra commented 8 years ago

Now that's a well formatted answer, providing meaningful explanations about facts and design choice! It would have been perfect to write all of this in the first place, thus avoiding the pointless discussion above.

Anyway, your point is valid regarding the details not being exposed. We won't expose it as it would break the API, so filtering them might be a good option. I'll take a look at what all these details mean, a find a good combination of them.

ivzave commented 8 years ago

Updated PR. Explanation:

Regarding mode field: according to https://tronche.com/gui/x/xlib/events/input-focus/grab.html NotifyGrab and NotifyUngrab events should be filtered out since they aren't related to actual focus transfers.

Regarding detail field: according to https://tronche.com/gui/x/xlib/events/input-focus/normal-and-grabbed.html NotifyVirtual, NotifyNonlinearVirtual and NotifyPointer should be filtered out because they are fired on windows not directly involved in focus transfer.

dcat commented 8 years ago

itt