FreeRDP / FreeRDP

FreeRDP is a free remote desktop protocol library and clients
http://www.freerdp.com/
Apache License 2.0
10.34k stars 14.5k forks source link

Popup menus and window actions do not work correctly in RemoteApp #10155

Closed toreonify closed 1 week ago

toreonify commented 2 weeks ago

Describe the bug

To Reproduce Steps to reproduce the behavior (context menus):

  1. Connect to Windows Server with 1C 8.3 through RemoteApp
  2. Open 1C client
  3. Try to open context menu in database list with right mouse click

Steps to reproduce the behavior (window state):

  1. Connect to Windows Server through RemoteApp
  2. Open some application, for example Task Manager
  3. Maximize window by clicking "maximize" in titlebar
  4. Restore windows by clicking "restore" in titlebar
  5. Switch to any other window

Steps to reproduce the behavior (allowed actions):

  1. Connect to Windows Server with 1C 8.3 through RemoteApp
  2. Open 1C client
  3. Ensure that only close action is available on titlebar and in titlebar context menu.
  4. Maximize window through context menu in taskbar.

Expected behavior Context menus: menus are shown on screen and do not disappear.

Window state: window stays "restored", not maximizing on its own.

Allowed actions: actions are identical to actions in Windows titlebar context menu.

Screenshots

Videos were recored earlier, but the same happens on later versions.

https://github.com/FreeRDP/FreeRDP/assets/10438627/cddbacbd-e3b0-415b-b1c1-e16aa54d9a34

https://github.com/FreeRDP/FreeRDP/assets/10438627/1b81e91e-f573-4c41-9830-df97e516be7b

https://github.com/FreeRDP/FreeRDP/assets/10438627/65a05bb5-6769-4fe7-841c-3b21b260c890

Application details

Environment:

Additional context On Windows Server 2008 R2 Task Manager window has WS_POPUP flag but on Windows Server 2012 R2 doesn't.

Provided patch for 2.10.0 fixes described problems (though, not tested on many apps).

diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c
index bd3eb0d56..44092fac3 100644
--- a/client/X11/xf_client.c
+++ b/client/X11/xf_client.c
@@ -1998,6 +1998,17 @@ static BOOL xfreerdp_client_new(freerdp* instance, rdpContext* context)
    xfc->WM_STATE = XInternAtom(xfc->display, "WM_STATE", False);
    xfc->x11event = CreateFileDescriptorEvent(NULL, FALSE, FALSE, xfc->xfds, WINPR_FD_READ);

+   xfc->_NET_WM_ALLOWED_ACTIONS = XInternAtom(xfc->display, "_NET_WM_ALLOWED_ACTIONS", False);
+
+   xfc->_NET_WM_ACTION_CLOSE = XInternAtom(xfc->display, "_NET_WM_ACTION_CLOSE", False);
+   xfc->_NET_WM_ACTION_MINIMIZE = XInternAtom(xfc->display, "_NET_WM_ACTION_MINIMIZE", False);
+   xfc->_NET_WM_ACTION_MOVE = XInternAtom(xfc->display, "_NET_WM_ACTION_MOVE", False);
+   xfc->_NET_WM_ACTION_RESIZE = XInternAtom(xfc->display, "_NET_WM_ACTION_RESIZE", False);
+   xfc->_NET_WM_ACTION_MAXIMIZE_HORZ = XInternAtom(xfc->display, "_NET_WM_ACTION_MAXIMIZE_HORZ", False);
+   xfc->_NET_WM_ACTION_MAXIMIZE_VERT = XInternAtom(xfc->display, "_NET_WM_ACTION_MAXIMIZE_VERT", False);
+   xfc->_NET_WM_ACTION_FULLSCREEN = XInternAtom(xfc->display, "_NET_WM_ACTION_FULLSCREEN", False);
+   xfc->_NET_WM_ACTION_CHANGE_DESKTOP = XInternAtom(xfc->display, "_NET_WM_ACTION_CHANGE_DESKTOP", False);
+
    if (!xfc->x11event)
    {
        WLog_ERR(TAG, "Could not create xfds event");
diff --git a/client/X11/xf_rail.c b/client/X11/xf_rail.c
index 090f599a0..b981ba4a7 100644
--- a/client/X11/xf_rail.c
+++ b/client/X11/xf_rail.c
@@ -97,8 +97,6 @@ void xf_rail_send_activate(xfContext* xfc, Window xwindow, BOOL enabled)

    if (enabled)
        xf_SetWindowStyle(xfc, appWindow, appWindow->dwStyle, appWindow->dwExStyle);
-   else
-       xf_SetWindowStyle(xfc, appWindow, 0, 0);

    activate.windowId = appWindow->windowId;
    activate.enabled = enabled;
@@ -518,6 +516,12 @@ static BOOL xf_rail_window_common(rdpContext* context, const WINDOW_ORDER_INFO*
                                        visibilityRectsOffsetY, appWindow->visibilityRects,
                                        appWindow->numVisibilityRects);
        }
+
+       if (appWindow->rail_state == WINDOW_SHOW_MAXIMIZED)
+       {
+           xf_SendClientEvent(xfc, appWindow->handle, xfc->_NET_WM_STATE, 4, _NET_WM_STATE_ADD,
+              xfc->_NET_WM_STATE_MAXIMIZED_VERT, xfc->_NET_WM_STATE_MAXIMIZED_HORZ, 0, 0);
+       }
    }

    /* We should only be using the visibility rects for shaping the window */
diff --git a/client/X11/xf_window.c b/client/X11/xf_window.c
index 9b5b1c4a6..25910b954 100644
--- a/client/X11/xf_window.c
+++ b/client/X11/xf_window.c
@@ -654,6 +654,8 @@ void xf_SetWindowStyle(xfContext* xfc, xfAppWindow* appWindow, UINT32 style, UIN
    Atom window_type;
    BOOL redirect = FALSE;

+   window_type = xfc->_NET_WM_WINDOW_TYPE_NORMAL;
+
    if ((ex_style & WS_EX_NOACTIVATE) || (ex_style & WS_EX_TOOLWINDOW))
    {
        redirect = TRUE;
@@ -669,16 +671,24 @@ void xf_SetWindowStyle(xfContext* xfc, xfAppWindow* appWindow, UINT32 style, UIN
    {
        window_type = xfc->_NET_WM_WINDOW_TYPE_NORMAL;
    }
-   else if (style & WS_POPUP)
+
+   if (style & WS_POPUP)
    {
-       /* this includes dialogs, popups, etc, that need to be full-fledged windows */
-       appWindow->is_transient = TRUE;
        window_type = xfc->_NET_WM_WINDOW_TYPE_DIALOG;
-       xf_SetWindowUnlisted(xfc, appWindow->handle);
+       /* this includes dialogs, popups, etc, that need to be full-fledged windows */
+
+       if (!((ex_style & WS_EX_DLGMODALFRAME) || (ex_style & WS_EX_LAYERED) || (style & WS_SYSMENU)))
+       {
+           appWindow->is_transient = TRUE;
+           redirect = TRUE;
+
+           xf_SetWindowUnlisted(xfc, appWindow->handle);
+       }
    }
-   else
+
+   if (!(style == 0 && ex_style == 0))
    {
-       window_type = xfc->_NET_WM_WINDOW_TYPE_NORMAL;
+       xf_SetWindowActions(xfc, appWindow);
    }

    {
@@ -700,6 +710,39 @@ void xf_SetWindowStyle(xfContext* xfc, xfAppWindow* appWindow, UINT32 style, UIN
                    PropModeReplace, (BYTE*)&window_type, 1);
 }

+void xf_SetWindowActions(xfContext* xfc, xfAppWindow* appWindow)
+{
+   Atom allowed_actions[] = {
+       xfc->_NET_WM_ACTION_CLOSE,
+       xfc->_NET_WM_ACTION_MINIMIZE,
+       xfc->_NET_WM_ACTION_MOVE,
+       xfc->_NET_WM_ACTION_RESIZE,
+       xfc->_NET_WM_ACTION_MAXIMIZE_HORZ,
+       xfc->_NET_WM_ACTION_MAXIMIZE_VERT,
+       xfc->_NET_WM_ACTION_FULLSCREEN,
+       xfc->_NET_WM_ACTION_CHANGE_DESKTOP
+   };
+
+   if (!(appWindow->dwStyle & WS_SYSMENU))
+       allowed_actions[0] = 0;
+
+   if (!(appWindow->dwStyle & WS_MINIMIZEBOX))
+       allowed_actions[1] = 0;
+
+   if (!(appWindow->dwStyle & WS_SIZEBOX))
+       allowed_actions[3] = 0;
+
+   if (!(appWindow->dwStyle & WS_MAXIMIZEBOX))
+   {
+       allowed_actions[4] = 0;
+       allowed_actions[5] = 0;
+       allowed_actions[6] = 0;
+   }
+
+   XChangeProperty(xfc->display, appWindow->handle, xfc->_NET_WM_ALLOWED_ACTIONS, XA_ATOM, 32,
+       PropModeReplace, (unsigned char *) &allowed_actions, 8);
+}
+
 void xf_SetWindowText(xfContext* xfc, xfAppWindow* appWindow, const char* name)
 {
    xf_SetWindowTitleText(xfc, appWindow->handle, name);
diff --git a/client/X11/xf_window.h b/client/X11/xf_window.h
index 0f85af1d2..b9bb5deeb 100644
--- a/client/X11/xf_window.h
+++ b/client/X11/xf_window.h
@@ -179,6 +179,7 @@ void xf_SetWindowRects(xfContext* xfc, xfAppWindow* appWindow, RECTANGLE_16* rec
 void xf_SetWindowVisibilityRects(xfContext* xfc, xfAppWindow* appWindow, UINT32 rectsOffsetX,
                                  UINT32 rectsOffsetY, RECTANGLE_16* rects, int nrects);
 void xf_SetWindowStyle(xfContext* xfc, xfAppWindow* appWindow, UINT32 style, UINT32 ex_style);
+void xf_SetWindowActions(xfContext* xfc, xfAppWindow* appWindow);
 void xf_UpdateWindowArea(xfContext* xfc, xfAppWindow* appWindow, int x, int y, int width,
                          int height);
 void xf_DestroyWindow(xfContext* xfc, xfAppWindow* appWindow);
diff --git a/client/X11/xfreerdp.h b/client/X11/xfreerdp.h
index 636e60ad1..514d7597e 100644
--- a/client/X11/xfreerdp.h
+++ b/client/X11/xfreerdp.h
@@ -245,6 +245,18 @@ struct xf_context
    Atom WM_PROTOCOLS;
    Atom WM_DELETE_WINDOW;

+   /* Allow actions */
+   Atom _NET_WM_ALLOWED_ACTIONS;
+
+   Atom _NET_WM_ACTION_CLOSE;
+   Atom _NET_WM_ACTION_MINIMIZE;
+   Atom _NET_WM_ACTION_MOVE;
+   Atom _NET_WM_ACTION_RESIZE;
+   Atom _NET_WM_ACTION_MAXIMIZE_HORZ;
+   Atom _NET_WM_ACTION_MAXIMIZE_VERT;
+   Atom _NET_WM_ACTION_FULLSCREEN;
+   Atom _NET_WM_ACTION_CHANGE_DESKTOP;
+
    /* Channels */
 #if defined(CHANNEL_TSMF_CLIENT)
    TsmfClientContext* tsmf;

One regression that I've found - dialog windows ("Run As", "Open file", etc) show up on taskbar, but work normally. On Windows they are not shown in taskbar. Maybe there is another way to check window type to not confuse them as normal windows.

akallabeth commented 2 weeks ago

@toreonify thank you for the report.

toreonify commented 2 weeks ago

could you create a pr for current master?

https://github.com/FreeRDP/FreeRDP/pull/10160