[PATCH xserver] dix: always send focus event on grab change

[Samuel Thibault]

Focus events are useless when 'from' and 'to' are the same. But when
this is the result of a (Un)GrabKeyboard request, we should always send
them, including when the window manager had previously used XSetInputFocus
to specify the focus on a window which happens to be now taking a grab.

This is notably needed for window manager using XI to always get keyboard
events even during grabs, so they can determine exactly when grabbing is
active.

Signed-off-by: Samuel Thibault <***@ens-lyon.org>
---
dix/enterleave.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dix/enterleave.c b/dix/enterleave.c
index 1b341f2de..a2f625bc9 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -1562,7 +1562,7 @@ DoFocusEvents(DeviceIntPtr pDev, WindowPtr from, WindowPtr to, int mode)
if (!IsKeyboardDevice(pDev))
return;

- if (from == to)
+ if (from == to && mode != NotifyGrab && mode != NotifyUngrab)
return;

CoreFocusEvents(pDev, from, to, mode);

[Peter Hutterer]

Post by Samuel Thibault
Focus events are useless when 'from' and 'to' are the same. But when
this is the result of a (Un)GrabKeyboard request, we should always send
them, including when the window manager had previously used XSetInputFocus
to specify the focus on a window which happens to be now taking a grab.
This is notably needed for window manager using XI to always get keyboard
events even during grabs, so they can determine exactly when grabbing is
active.

good catch, thanks.
Reviewed-by: Peter Hutterer <***@who-t.net>

Ajax - feel free to take this one or wait for 1.20.1. It should be safe but
there could be subtle bugs. Proably not any worse than having this broken
for the last 10 years.

Cheers,
Peter

Post by Samuel Thibault
---
dix/enterleave.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dix/enterleave.c b/dix/enterleave.c
index 1b341f2de..a2f625bc9 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -1562,7 +1562,7 @@ DoFocusEvents(DeviceIntPtr pDev, WindowPtr from, WindowPtr to, int mode)
if (!IsKeyboardDevice(pDev))
return;
- if (from == to)
+ if (from == to && mode != NotifyGrab && mode != NotifyUngrab)
return;
CoreFocusEvents(pDev, from, to, mode);
--
2.16.3
_______________________________________________
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info:

[Adam Jackson]

Post by Peter Hutterer

Post by Samuel Thibault
Focus events are useless when 'from' and 'to' are the same. But when
this is the result of a (Un)GrabKeyboard request, we should always send
them, including when the window manager had previously used XSetInputFocus
to specify the focus on a window which happens to be now taking a grab.
This is notably needed for window manager using XI to always get keyboard
events even during grabs, so they can determine exactly when grabbing is
active.

good catch, thanks.
Ajax - feel free to take this one or wait for 1.20.1. It should be safe but
there could be subtle bugs. Proably not any worse than having this broken
for the last 10 years.

Merged, thanks:

remote: I: patch #215922 updated using rev c67f2eac56518163981af59f5accb7c79bc00f6a.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
14be894b3f..c67f2eac56 master -> master

- ajax
_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo

[Samuel Thibault]

c67f2eac5651 ("dix: always send focus event on grab change") made dix
always sent events when it's a NotifyGrab or NotifyUngrab, even if
from == to, because 'from' can just come from a previous XSetInputFocus
call.

However, when an application calls XGrabKeyboard several times on
the same window, we are now sending spurious FocusOut+FocusIn with
NotifyGrab, even if the grab does not actually change. This makes screen
readers for blind people spuriously emit activity events which disturb
screen reading workflow when e.g. switching between menus.

This commit avoids calling DoFocusEvents in that precise case, i.e. when
oldWin is a previous grab and the new grab is the same window.

Signed-off-by: Samuel Thibault <***@ens-lyon.org>

Index: xorg-server-1.19.2/dix/events.c
===================================================================
--- xorg-server-1.19.2.orig/dix/events.c
+++ xorg-server-1.19.2/dix/events.c
@@ -1530,7 +1530,9 @@ ActivatePointerGrab(DeviceIntPtr mouse,
mouse->spriteInfo->sprite->hotPhys.y = 0;
ConfineCursorToWindow(mouse, grab->confineTo, FALSE, TRUE);
}
- DoEnterLeaveEvents(mouse, mouse->id, oldWin, grab->window, NotifyGrab);
+ if (! (grabinfo->grab && oldWin == grabinfo->grab->window
+ && oldWin == grab->window))
+ DoEnterLeaveEvents(mouse, mouse->id, oldWin, grab->window, NotifyGrab);
mouse->valuator->motionHintWindow = NullWindow;
if (syncEvents.playingEvents)
grabinfo->grabTime = syncEvents.time;
@@ -1640,7 +1642,9 @@ ActivateKeyboardGrab(DeviceIntPtr keybd,
oldWin = keybd->focus->win;
if (keybd->valuator)
keybd->valuator->motionHintWindow = NullWindow;
- if (oldWin)
+ if (oldWin &&
+ ! (grabinfo->grab && oldWin == grabinfo->grab->window
+ && oldWin == grab->window))
DoFocusEvents(keybd, oldWin, grab->window, NotifyGrab);
if (syncEvents.playingEvents)
grabinfo->grabTime = syncEvents.time;
_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg

[Samuel Thibault]

Hello,

Ping?

Samuel

Post by Samuel Thibault
c67f2eac5651 ("dix: always send focus event on grab change") made dix
always sent events when it's a NotifyGrab or NotifyUngrab, even if
from == to, because 'from' can just come from a previous XSetInputFocus
call.
However, when an application calls XGrabKeyboard several times on
the same window, we are now sending spurious FocusOut+FocusIn with
NotifyGrab, even if the grab does not actually change. This makes screen
readers for blind people spuriously emit activity events which disturb
screen reading workflow when e.g. switching between menus.
This commit avoids calling DoFocusEvents in that precise case, i.e. when
oldWin is a previous grab and the new grab is the same window.
Index: xorg-server-1.19.2/dix/events.c
===================================================================
--- xorg-server-1.19.2.orig/dix/events.c
+++ xorg-server-1.19.2/dix/events.c
@@ -1530,7 +1530,9 @@ ActivatePointerGrab(DeviceIntPtr mouse,
mouse->spriteInfo->sprite->hotPhys.y = 0;
ConfineCursorToWindow(mouse, grab->confineTo, FALSE, TRUE);
}
- DoEnterLeaveEvents(mouse, mouse->id, oldWin, grab->window, NotifyGrab);
+ if (! (grabinfo->grab && oldWin == grabinfo->grab->window
+ && oldWin == grab->window))
+ DoEnterLeaveEvents(mouse, mouse->id, oldWin, grab->window, NotifyGrab);
mouse->valuator->motionHintWindow = NullWindow;
if (syncEvents.playingEvents)
grabinfo->grabTime = syncEvents.time;
@@ -1640,7 +1642,9 @@ ActivateKeyboardGrab(DeviceIntPtr keybd,
oldWin = keybd->focus->win;
if (keybd->valuator)
keybd->valuator->motionHintWindow = NullWindow;
- if (oldWin)
+ if (oldWin &&
+ ! (grabinfo->grab && oldWin == grabinfo->grab->window
+ && oldWin == grab->window))
DoFocusEvents(keybd, oldWin, grab->window, NotifyGrab);
if (syncEvents.playingEvents)
grabinfo->grabTime = syncEvents.time;

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-

[Adam Jackson]

Post by Samuel Thibault
Hello,
Ping?

Tsk, thought I'd pushed this already. Merged, thanks:

https://gitlab.freedesktop.org/xorg/xserver/merge_requests/77

- ajax

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-deve