This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch 3.6.x in repository nx-libs. commit 7aa969cd4ee5fe6ecf74f82442e4a0a7491191c1 Author: Ulrich Sibiller <uli42@gmx.de> Date: Wed Jun 16 20:16:20 2021 +0200 Fix Xfixes event handling Calling the callback on receptions of an EXTERNAL xfixes event was a weird idea from the start. However, since adding the protection trap around it it made no sense at all because it effectivly made the callback a noop for that case. So let's drop all the trap nonsense and implement it properly. The callback will only be used for actions by internal clients. --- nx-X11/programs/Xserver/hw/nxagent/Clipboard.c | 166 +++++++++++++------------ nx-X11/programs/Xserver/hw/nxagent/Clipboard.h | 3 + nx-X11/programs/Xserver/hw/nxagent/Events.c | 101 +++++++-------- nx-X11/programs/Xserver/hw/nxagent/Trap.c | 8 -- nx-X11/programs/Xserver/hw/nxagent/Trap.h | 7 -- 5 files changed, 137 insertions(+), 148 deletions(-) diff --git a/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c b/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c index 20a380817..4a0bf9cb5 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c @@ -873,14 +873,13 @@ void invalidateTargetCaches(void) /* * This is called from Events.c dispatch loop on reception of a * SelectionClear event. We receive this event if someone on the real - * X server claims the selection ownership. + * X server claims the selection ownership we have/had. + * Three versions of this routine with different parameter types. */ -void nxagentHandleSelectionClearFromXServer(XEvent *X) +void nxagentHandleSelectionClearFromXServerByIndex(int index) { #ifdef DEBUG - fprintf(stderr, "---------\n%s: SelectionClear event for selection [%lu][%s] window [0x%lx] time [%lu].\n", - __func__, X->xselectionclear.selection, NameForRemoteAtom(X->xselectionclear.selection), - X->xselectionclear.window, X->xselectionclear.time); + fprintf(stderr, "%s: SelectionClear event for selection index [%u].\n", __func__, index); #endif if (!agentClipboardInitialized) @@ -899,43 +898,56 @@ void nxagentHandleSelectionClearFromXServer(XEvent *X) return; } - int index = nxagentFindRemoteSelectionIndex(X->xselectionclear.selection); - if (index != -1) + if (IS_LOCAL_OWNER(index)) { - if (IS_LOCAL_OWNER(index)) - { - /* Send a SelectionClear event to (our) previous owner. */ - xEvent x = {0}; - x.u.u.type = SelectionClear; - x.u.selectionClear.time = GetTimeInMillis(); - x.u.selectionClear.window = lastSelectionOwner[index].window; - x.u.selectionClear.atom = CurrentSelections[index].selection; + /* Send a SelectionClear event to (our) previous owner. */ + xEvent x = {0}; + x.u.u.type = SelectionClear; + x.u.selectionClear.time = GetTimeInMillis(); + x.u.selectionClear.window = lastSelectionOwner[index].window; + x.u.selectionClear.atom = CurrentSelections[index].selection; - sendEventToClient(lastSelectionOwner[index].client, &x); + sendEventToClient(lastSelectionOwner[index].client, &x); - /* - * Set the root window with the NullClient as selection owner. Our - * clients asking for the owner via XGetSelectionOwner() will get - * this for an answer. - */ - CurrentSelections[index].window = screenInfo.screens[0]->root->drawable.id; - CurrentSelections[index].client = NullClient; + /* + * Set the root window with the NullClient as selection owner. Our + * clients asking for the owner via XGetSelectionOwner() will get + * this for an answer. + */ + CurrentSelections[index].window = screenInfo.screens[0]->root->drawable.id; + CurrentSelections[index].client = NullClient; - clearSelectionOwnerData(index); + clearSelectionOwnerData(index); - setClientSelectionStage(index, SelectionStageNone); + setClientSelectionStage(index, SelectionStageNone); - invalidateTargetCache(index); - } - #ifdef DEBUG - else - { - fprintf(stderr, "%s: selection already cleared - doing nothing.\n", __func__); - } - #endif + invalidateTargetCache(index); + } +} + +void nxagentHandleSelectionClearFromXServerByAtom(XlibAtom sel) +{ + #ifdef DEBUG + fprintf(stderr, "---------\n%s: SelectionClear event for remote selection atom [%lu][%s].\n", __func__, sel, NameForRemoteAtom(sel)); + #endif + + int index = nxagentFindRemoteSelectionIndex(sel); + if (index != -1) + { + nxagentHandleSelectionClearFromXServerByIndex(index); } } +void nxagentHandleSelectionClearFromXServer(XEvent *X) +{ + #ifdef DEBUG + fprintf(stderr, "---------\n%s: SelectionClear event for selection [%lu][%s] window [0x%lx] time [%lu].\n", + __func__, X->xselectionclear.selection, NameForRemoteAtom(X->xselectionclear.selection), + X->xselectionclear.window, X->xselectionclear.time); + #endif + nxagentHandleSelectionClearFromXServerByAtom(X->xselectionclear.selection); +} + /* * Send a SelectionNotify event as reply to the RequestSelection * event X. If success is True take the property from the event, else @@ -2202,25 +2214,11 @@ static void resetSelectionOwnerOnXServer(void) #ifdef NXAGENT_CLIPBOARD /* - * The callback is called from dix. This is the normal operation - * mode. The callback is also called when nxagent gets XFixes events - * from the real X server. In that case the Trap is set and the - * callback will do nothing. + * The callback is called from dix. */ - void nxagentSetSelectionCallback(CallbackListPtr *callbacks, void *data, void *args) { - /* - * Only act if the trap is unset. The trap indicates that we are - * triggered by an XFixes clipboard event originating from the real - * X server. In that case we do not want to propagate back changes - * to the real X server, because it already knows about them and we - * would end up in an infinite loop of events. If there was a better - * way to identify that situation during callback processing we - * could get rid of the Trap... - */ - SelectionInfoRec *info = (SelectionInfoRec *)args; #ifdef DEBUG @@ -2249,58 +2247,66 @@ void nxagentSetSelectionCallback(CallbackListPtr *callbacks, void *data, if (index == -1) { #ifdef DEBUG - fprintf(stderr, "%s: selection [%s] will not be handled by the clipboard code\n", __func__, NameForLocalAtom(pCurSel->selection)); + fprintf(stderr, "%s: selection [%s] can/will not be handled by the clipboard code\n", __func__, NameForLocalAtom(pCurSel->selection)); #endif return; } /* - * Always invalidate the target cache for the relevant selection, - * even if the trap is set. This ensures not having invalid data in - * the cache. + * Always invalidate the target cache for the relevant selection. + * This ensures not having invalid data in the cache. */ invalidateTargetCache(index); - if (nxagentExternalClipboardEventTrap) - { - #ifdef DEBUG - fprintf(stderr, "%s: Trap is set, doing nothing\n", __func__); - #endif - return; - } - - #ifdef DEBUG + fprintf(stderr, "%s: pCurSel->window [0x%x]\n", __func__, pCurSel->window); + fprintf(stderr, "%s: pCurSel->pWin [0x%x]\n", __func__, WINDOWID(pCurSel->pWin)); + fprintf(stderr, "%s: pCurSel->selection [%s]\n", __func__, NameForLocalAtom(pCurSel->selection)); fprintf(stderr, "%s: pCurSel->lastTimeChanged [%u]\n", __func__, pCurSel->lastTimeChanged.milliseconds); + fprintf(stderr, "%s: pCurSel->client [%s]\n", __func__, nxagentClientInfoString(pCurSel->client)); #endif - if (info->kind == SelectionSetOwner) + if (nxagentOption(Clipboard) != ClipboardNone) /* FIXME: shouldn't we also check for != ClipboardClient? */ { - #ifdef DEBUG - fprintf(stderr, "%s: pCurSel->pWin [0x%x]\n", __func__, WINDOWID(pCurSel->pWin)); - fprintf(stderr, "%s: pCurSel->selection [%s]\n", __func__, NameForLocalAtom(pCurSel->selection)); - #endif + Selection *pSel = NULL; + Selection nullSel = { + .client = NullClient, + .window = None, + .pWin = NULL, + .selection = pCurSel->selection, + .lastTimeChanged = pCurSel->lastTimeChanged + }; + + if (info->kind == SelectionSetOwner) + { + pSel = pCurSel; + } + else if (info->kind == SelectionWindowDestroy) + { + if (pCurSel->window == lastSelectionOwner[index].window) + { + pSel = &nullSel; + } + } + else if (info->kind == SelectionClientClose) + { + if (pCurSel->client == lastSelectionOwner[index].client) + { + pSel = &nullSel; + } + } + else + { + } - if (pCurSel->pWin != NULL && - nxagentOption(Clipboard) != ClipboardNone && /* FIXME: shouldn't we also check for != ClipboardClient? */ - (pCurSel->selection == localSelelectionAtoms[nxagentPrimarySelection] || - pCurSel->selection == localSelelectionAtoms[nxagentClipboardSelection])) + if (pSel) { #ifdef DEBUG fprintf(stderr, "%s: calling setSelectionOwnerOnXServer\n", __func__); #endif - setSelectionOwnerOnXServer(pCurSel); + setSelectionOwnerOnXServer(pSel); } } - else if (info->kind == SelectionWindowDestroy) - { - } - else if (info->kind == SelectionClientClose) - { - } - else - { - } } #endif diff --git a/nx-X11/programs/Xserver/hw/nxagent/Clipboard.h b/nx-X11/programs/Xserver/hw/nxagent/Clipboard.h index aca8d94af..4518595d1 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Clipboard.h +++ b/nx-X11/programs/Xserver/hw/nxagent/Clipboard.h @@ -57,11 +57,14 @@ extern void nxagentClearClipboard(ClientPtr pClient, WindowPtr pWindow); extern int nxagentConvertSelection(ClientPtr client, WindowPtr pWin, Atom selection, Window requestor, Atom property, Atom target, Time time); +extern void nxagentHandleSelectionClearFromXServerByIndex(int index); #ifdef XEvent +extern void nxagentHandleSelectionClearFromXServerByAtom(XlibAtom sel); extern void nxagentHandleSelectionClearFromXServer(XEvent *X); extern void nxagentHandleSelectionRequestFromXServer(XEvent *X); extern void nxagentHandleSelectionNotifyFromXServer(XEvent *X); #else +extern void nxagentHandleSelectionClearFromXServerByAtom(); extern void nxagentHandleSelectionClearFromXServer(); extern void nxagentHandleSelectionRequestFromXServer(); extern void nxagentHandleSelectionNotifyFromXServer(); diff --git a/nx-X11/programs/Xserver/hw/nxagent/Events.c b/nx-X11/programs/Xserver/hw/nxagent/Events.c index 906915ad0..a678e3841 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Events.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Events.c @@ -2841,69 +2841,64 @@ int nxagentHandleXFixesSelectionNotify(XEvent *X) return 0; } + #ifdef DEBUG + fprintf(stderr, "---------\n"); + #endif + #ifdef TEST fprintf(stderr, "%s: Handling event.\n", __func__); #endif - if (SelectionCallback) - { - Atom local = nxagentRemoteToLocalAtom(xfixesEvent -> xfixesselection.selection); - - int index = nxagentFindCurrentSelectionIndex(local); - if (index != -1) - { - if (CurrentSelections[index].client != 0) - { - #ifdef TEST - fprintf(stderr, "%s: Doing nothing.\n", __func__); - #endif - - return 1; - } - - #ifdef TEST - fprintf(stderr, "%s: Calling callbacks for %d [%s] selection.\n", __func__, - CurrentSelections[index].selection, NameForAtom(CurrentSelections[index].selection)); - #endif + #ifdef DEBUG + fprintf(stderr, "%s: Event timestamp [%ld]\n", __func__, xfixesEvent->xfixesselection.timestamp); + fprintf(stderr, "%s: Event selection timestamp [%ld]\n", __func__, xfixesEvent->xfixesselection.selection_timestamp); + fprintf(stderr, "%s: Event selection window [0x%lx]\n", __func__, xfixesEvent->xfixesselection.window); + fprintf(stderr, "%s: Event selection owner [0x%lx]\n", __func__, xfixesEvent->xfixesselection.owner); + fprintf(stderr, "%s: Event selection [%s]\n", __func__, NameForAtom(nxagentRemoteToLocalAtom(xfixesEvent->xfixesselection.selection))); - #ifdef DEBUG - fprintf(stderr, "%s: CurrentSelections[%d].lastTimeChanged [%u]\n", __func__, index, CurrentSelections[index].lastTimeChanged.milliseconds); - fprintf(stderr, "%s: Event timestamp [%ld]\n", __func__, xfixesEvent->xfixesselection.timestamp); - fprintf(stderr, "%s: Event selection timestamp [%ld]\n", __func__, xfixesEvent->xfixesselection.selection_timestamp); - fprintf(stderr, "%s: Event selection window [0x%lx]\n", __func__, xfixesEvent->xfixesselection.window); - fprintf(stderr, "%s: Event selection owner [0x%lx]\n", __func__, xfixesEvent->xfixesselection.owner); - fprintf(stderr, "%s: Event selection [%s]\n", __func__, NameForAtom(local)); + fprintf(stderr, "%s: Subtype ", __func__); - fprintf(stderr, "%s: Subtype ", __func__); + switch (xfixesEvent -> xfixesselection.subtype) + { + case SelectionSetOwner: fprintf(stderr, "SelectionSetOwner.\n"); break; + case SelectionWindowDestroy: fprintf(stderr, "SelectionWindowDestroy.\n"); break; + case SelectionClientClose: fprintf(stderr, "SelectionClientClose.\n"); break; + default: fprintf(stderr, ".\n"); break; + } + #endif - switch (xfixesEvent -> xfixesselection.subtype) - { - case SelectionSetOwner: fprintf(stderr, "SelectionSetOwner.\n"); break; - case SelectionWindowDestroy: fprintf(stderr, "SelectionWindowDestroy.\n"); break; - case SelectionClientClose: fprintf(stderr, "SelectionClientClose.\n"); break; - default: fprintf(stderr, ".\n"); break; - } - #endif + if (xfixesEvent->xfixesselection.owner && xfixesEvent->xfixesselection.owner == nxagentWindow(screenInfo.screens[0]->root)) + { + /* + * This is an event that must have been triggered by nxagent itself + * - by calling XSetSelectionOwner(). As this is no news for us we + * can ignore the event. + */ - SelectionInfoRec info = { - .selection = &CurrentSelections[index], - .kind = xfixesEvent->xfixesselection.subtype - }; + #ifdef DEBUG + fprintf(stderr, "%s: (new) owner is nxagent (window is [0x%lx]) - ignoring it.\n", __func__, xfixesEvent->xfixesselection.window); + #endif + return 0; + } - /* - * The trap indicates that we are triggered by a clipboard event - * originating from the real X server. In that case we do not - * want to propagate back changes to the real X server, because - * it already knows about them and we would end up in an - * infinite loop of events. If there was a better way to - * identify that situation during Callback processing we could - * get rid of the Trap... - */ - nxagentExternalClipboardEventTrap = True; - CallCallbacks(&SelectionCallback, &info); - nxagentExternalClipboardEventTrap = False; - } + if (xfixesEvent -> xfixesselection.subtype == SelectionSetOwner|| + xfixesEvent -> xfixesselection.subtype == SelectionWindowDestroy || + xfixesEvent -> xfixesselection.subtype == SelectionClientClose) + { + /* + * Reception of an owner change on the real X server is - for nxagent - the same as + * receiving a SelectionClear event. We just need to tell a (possible) internal + * owner that is no longer owning the selection. + */ + nxagentHandleSelectionClearFromXServerByAtom(xfixesEvent -> xfixesselection.selection); + } + else + { + #ifdef DEBUG + fprintf(stderr, "%s: WARNING unexpected xfixesselection subtype [%d]\n", __func__, xfixesEvent -> xfixesselection.subtype); + #endif } + return 1; } diff --git a/nx-X11/programs/Xserver/hw/nxagent/Trap.c b/nx-X11/programs/Xserver/hw/nxagent/Trap.c index 6eade2073..c0a1fdde0 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Trap.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Trap.c @@ -103,11 +103,3 @@ Bool nxagentXkbCapsTrap = False; */ Bool nxagentXkbNumTrap = False; - -/* - * Set to indicate we are processing a clipboard event triggered by - * the real X server. This is used to avoid endless loops if callbacks - * would trigger another event by the real X server - */ - -Bool nxagentExternalClipboardEventTrap = False; diff --git a/nx-X11/programs/Xserver/hw/nxagent/Trap.h b/nx-X11/programs/Xserver/hw/nxagent/Trap.h index 1c10f00ea..02ad48f99 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Trap.h +++ b/nx-X11/programs/Xserver/hw/nxagent/Trap.h @@ -101,11 +101,4 @@ extern Bool nxagentXkbCapsTrap; */ extern Bool nxagentXkbNumTrap; -/* - * Set to indicate we are processing a clipboard event triggered by - * the real X server. This is used to avoid endless loops if callbacks - * would trigger another event by the real X server - */ -extern Bool nxagentExternalClipboardEventTrap; - #endif /* __Trap_H__ */ -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/nx-libs.git