[PATCH v7] user32: Don't save maximized position for toplevel windows covering the entire work area.

[Gabriel Ivăncescu]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51672
Signed-off-by: Gabriel Ivăncescu 
---
 dlls/user32/tests/win.c | 54 +++++++++++++++++++------
 dlls/user32/winpos.c    | 90 +++++++++++++++++++++++++++++++++--------
 2 files changed, 115 insertions(+), 29 deletions(-)

diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
index 228395e..e23ab81 100644
--- a/dlls/user32/tests/win.c
+++ b/dlls/user32/tests/win.c
@@ -11624,8 +11624,9 @@ static void test_IsWindowEnabled(void)

static void test_window_placement(void)
 {
-    RECT orig = {100, 200, 300, 400}, orig2 = {200, 300, 400, 500}, rect;
+    RECT orig = {100, 200, 300, 400}, orig2 = {200, 300, 400, 500}, rect, work_rect;
     WINDOWPLACEMENT wp = {sizeof(wp)};
+    MONITORINFO mon_info;
     HWND hwnd;
     BOOL ret;

@@ -11633,6 +11634,10 @@ static void test_window_placement(void)
         orig.left, orig.top, orig.right - orig.left, orig.bottom - orig.top, 0, 0, 0, 0);
     ok(!!hwnd, "failed to create window, error %u\n", GetLastError());

+    mon_info.cbSize = sizeof(mon_info);
+    GetMonitorInfoW(MonitorFromWindow(hwnd, MONITOR_DEFAULTTOPRIMARY), &mon_info);
+    work_rect = mon_info.rcWork;
+
     ret = GetWindowPlacement(hwnd, &wp);
     ok(ret, "failed to get window placement, error %u\n", GetLastError());
     ok(wp.showCmd == SW_SHOWNORMAL, "got show cmd %u\n", wp.showCmd);
@@ -11675,7 +11680,6 @@ static void test_window_placement(void)
     ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11693,6 +11697,42 @@ todo_wine
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
         wine_dbgstr_rect(&wp.rcNormalPosition));

+    SetWindowPos(hwnd, 0, work_rect.left, work_rect.top, work_rect.right - work_rect.left,
+                 work_rect.bottom - work_rect.top, SWP_NOZORDER | SWP_NOACTIVATE);
+    ret = GetWindowPlacement(hwnd, &wp);
+    ok(ret, "failed to get window placement, error %u\n", GetLastError());
+    ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
+    ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
+        "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
+    ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
+        "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
+    ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
+        wine_dbgstr_rect(&wp.rcNormalPosition));
+
+    SetWindowPos(hwnd, 0, work_rect.left, work_rect.top, work_rect.right - work_rect.left - 1,
+                 work_rect.bottom - work_rect.top, SWP_NOZORDER | SWP_NOACTIVATE);
+    ret = GetWindowPlacement(hwnd, &wp);
+    ok(ret, "failed to get window placement, error %u\n", GetLastError());
+    ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
+    ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
+        "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
+    ok(wp.ptMaxPosition.x == work_rect.left && wp.ptMaxPosition.y == work_rect.top,
+        "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
+    ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
+        wine_dbgstr_rect(&wp.rcNormalPosition));
+
+    SetWindowPos(hwnd, 0, work_rect.left, work_rect.top, work_rect.right - work_rect.left,
+                 work_rect.bottom - work_rect.top - 1, SWP_NOZORDER | SWP_NOACTIVATE);
+    ret = GetWindowPlacement(hwnd, &wp);
+    ok(ret, "failed to get window placement, error %u\n", GetLastError());
+    ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
+    ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
+        "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
+    ok(wp.ptMaxPosition.x == work_rect.left && wp.ptMaxPosition.y == work_rect.top,
+        "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
+    ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
+        wine_dbgstr_rect(&wp.rcNormalPosition));
+
     ShowWindow(hwnd, SW_MINIMIZE);

ret = GetWindowPlacement(hwnd, &wp);
@@ -11701,7 +11741,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11714,7 +11753,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11727,7 +11765,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWNORMAL, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11745,7 +11782,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWNORMAL, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == 100 && wp.ptMinPosition.y == 100,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig2), "got normal pos %s\n",
@@ -11761,7 +11797,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig2), "got normal pos %s\n",
@@ -11783,7 +11818,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11804,7 +11838,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == 100 && wp.ptMinPosition.y == 100,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11825,7 +11858,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11839,7 +11871,6 @@ todo_wine
     ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
@@ -11854,7 +11885,6 @@ todo_wine
     ok(wp.showCmd == SW_NORMAL, "got show cmd %u\n", wp.showCmd);
     ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
         "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
-todo_wine
     ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
         "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
     ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c
index 6e96a4b..e95f333 100644
--- a/dlls/user32/winpos.c
+++ b/dlls/user32/winpos.c
@@ -675,6 +675,33 @@ BOOL WINAPI MoveWindow( HWND hwnd, INT x, INT y, INT cx, INT cy,
 }

+/*******************************************************************
+ *           get_work_rect
+ *
+ * Get the work area that a maximized window can cover, depending on style.
+ */
+static BOOL get_work_rect( HWND hwnd, RECT *rect )
+{
+    HMONITOR monitor = MonitorFromWindow( hwnd, MONITOR_DEFAULTTOPRIMARY );
+    MONITORINFO mon_info;
+    DWORD style;
+
+    if (!monitor) return FALSE;
+
+    mon_info.cbSize = sizeof(mon_info);
+    GetMonitorInfoW( monitor, &mon_info );
+
+    style = GetWindowLongW( hwnd, GWL_STYLE );
+    *rect = mon_info.rcMonitor;
+    if (style & WS_MAXIMIZEBOX)
+    {
+        if ((style & WS_CAPTION) == WS_CAPTION || !(style & (WS_CHILD | WS_POPUP)))
+            *rect = mon_info.rcWork;
+    }
+    return TRUE;
+}
+
+
 /*******************************************************************
  *           WINPOS_GetMinMaxInfo
  *
@@ -683,8 +710,8 @@ BOOL WINAPI MoveWindow( HWND hwnd, INT x, INT y, INT cx, INT cy,
 MINMAXINFO WINPOS_GetMinMaxInfo( HWND hwnd )
 {
     DPI_AWARENESS_CONTEXT context;
+    RECT rc_work, rc_primary;
     MINMAXINFO MinMax;
-    HMONITOR monitor;
     INT xinc, yinc;
     LONG style = GetWindowLongW( hwnd, GWL_STYLE );
     LONG adjustedStyle;
@@ -738,22 +765,8 @@ MINMAXINFO WINPOS_GetMinMaxInfo( HWND hwnd )

/* if the app didn't change the values, adapt them for the current monitor */

-    if ((monitor = MonitorFromWindow( hwnd, MONITOR_DEFAULTTOPRIMARY )))
+    if (get_work_rect( hwnd, &rc_work ))
     {
-        RECT rc_work, rc_primary;
-        MONITORINFO mon_info;
-
-        mon_info.cbSize = sizeof(mon_info);
-        GetMonitorInfoW( monitor, &mon_info );
-
-        rc_work = mon_info.rcMonitor;
-
-        if (style & WS_MAXIMIZEBOX)
-        {
-            if ((style & WS_CAPTION) == WS_CAPTION || !(style & (WS_CHILD | WS_POPUP)))
-                rc_work = mon_info.rcWork;
-        }
-
         rc_primary = get_primary_monitor_rect();
         if (MinMax.ptMaxSize.x == (rc_primary.right - rc_primary.left) + 2 * xinc &&
             MinMax.ptMaxSize.y == (rc_primary.bottom - rc_primary.top) + 2 * yinc)
@@ -916,6 +929,42 @@ static POINT get_minimized_pos( HWND hwnd, POINT pt )
     return pt;
 }

+static RECT get_maximized_work_rect( HWND hwnd )
+{
+    RECT work_rect = { 0 };
+
+    if ((GetWindowLongW( hwnd, GWL_STYLE ) & (WS_MINIMIZE | WS_MAXIMIZE)) == WS_MAXIMIZE)
+    {
+        if (!get_work_rect( hwnd, &work_rect ))
+            SetRect( &work_rect, 0, 0, GetSystemMetrics( SM_CXSCREEN ), GetSystemMetrics( SM_CYSCREEN ) );
+    }
+    return work_rect;
+}
+
+static void update_maximized_pos( WND *wnd, RECT *work_rect )
+{
+    /* For top level windows covering the work area, we might have
+       to "forget" the maximized position. Windows presumably does
+       this to avoid situations where the border style changes,
+       which would lead the window to be outside the screen, or the
+       window gets reloaded on a different screen, and the "saved"
+       position no longer applies to it (despite being maximized).
+
+       Some applications (e.g. Imperiums: Greek Wars) depend on this.
+    */
+    if (wnd->parent && wnd->parent != GetDesktopWindow())
+        return;
+
+    if (wnd->dwStyle & WS_MAXIMIZE)
+    {
+        if (wnd->window_rect.left  <= work_rect->left  && wnd->window_rect.top    <= work_rect->top &&
+            wnd->window_rect.right >= work_rect->right && wnd->window_rect.bottom >= work_rect->bottom)
+            wnd->max_pos.x = wnd->max_pos.y = -1;
+    }
+    else
+        wnd->max_pos.x = wnd->max_pos.y = -1;
+}
+

/***********************************************************************
  *           WINPOS_MinMaximize
@@ -1280,6 +1329,7 @@ UINT WINAPI GetInternalWindowPos( HWND hwnd, LPRECT rectWnd,
  */
 BOOL WINAPI GetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *wndpl )
 {
+    RECT work_rect = get_maximized_work_rect( hwnd );
     WND *pWnd = WIN_GetPtr( hwnd );

if (!pWnd) return FALSE;
@@ -1337,6 +1387,7 @@ BOOL WINAPI GetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *wndpl )
     {
         pWnd->normal_rect = pWnd->window_rect;
     }
+    update_maximized_pos( pWnd, &work_rect );

wndpl->length  = sizeof(*wndpl);
     if( pWnd->dwStyle & WS_MINIMIZE )
@@ -1408,6 +1459,7 @@ static void make_point_onscreen( POINT *pt )
 static BOOL WINPOS_SetPlacement( HWND hwnd, const WINDOWPLACEMENT *wndpl, UINT flags )
 {
     DWORD style;
+    RECT work_rect = get_maximized_work_rect( hwnd );
     WND *pWnd = WIN_GetPtr( hwnd );
     WINDOWPLACEMENT wp = *wndpl;

@@ -1425,7 +1477,11 @@ static BOOL WINPOS_SetPlacement( HWND hwnd, const WINDOWPLACEMENT *wndpl, UINT f
     if (!pWnd || pWnd == WND_OTHER_PROCESS || pWnd == WND_DESKTOP) return FALSE;

if (flags & PLACE_MIN) pWnd->min_pos = point_thread_to_win_dpi( hwnd, wp.ptMinPosition );
-    if (flags & PLACE_MAX) pWnd->max_pos = point_thread_to_win_dpi( hwnd, wp.ptMaxPosition );
+    if (flags & PLACE_MAX)
+    {
+        pWnd->max_pos = point_thread_to_win_dpi( hwnd, wp.ptMaxPosition );
+        update_maximized_pos( pWnd, &work_rect );
+    }
     if (flags & PLACE_RECT) pWnd->normal_rect = rect_thread_to_win_dpi( hwnd, wp.rcNormalPosition );

style = pWnd->dwStyle;

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=96527

Your paranoid android.

=== w1064_tsign (32 bit report) ===

user32:
win.c:4086: Test failed: hwnd 0011005A/0011005A message 0200
win.c:4089: Test failed: hwnd 0011005A/0011005A message 0201

[Huw Davies]

On Fri, Aug 27, 2021 at 08:11:15PM +0300, Gabriel Ivăncescu wrote:
> diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c
> index 6e96a4b..e95f333 100644
> --- a/dlls/user32/winpos.c
> +++ b/dlls/user32/winpos.c
> @@ -675,6 +675,33 @@ BOOL WINAPI MoveWindow( HWND hwnd, INT x, INT y, INT cx, INT cy,
>  }
>
> +/*******************************************************************
> -
> -
>       get_work_rect
> -
> -
> -
> - Get the work area that a maximized window can cover, depending on style.
> - */
>
> +static BOOL get_work_rect( HWND hwnd, RECT *rect )
> +{
> - HMONITOR monitor = MonitorFromWindow( hwnd, MONITOR_DEFAULTTOPRIMARY );
> - MONITORINFO mon_info;
> - DWORD style;
> -
> - if (!monitor) return FALSE;
> -
> - mon_info.cbSize = sizeof(mon_info);
> - GetMonitorInfoW( monitor, &mon_info );
> -
> - style = GetWindowLongW( hwnd, GWL_STYLE );
> - *rect = mon_info.rcMonitor;
> - if (style & WS_MAXIMIZEBOX)
> - {
> -
>    if ((style & WS_CAPTION) == WS_CAPTION || !(style & (WS_CHILD | WS_POPUP)))
> -
>        *rect = mon_info.rcWork;
> - }
> - return TRUE;
>
> +}

I'd suggest splitting this patch into two.  Adding the get_work_rect()
helper in the first then adding the changes to [GS]etWindowPlacement()
in the second.
> +static RECT get_maximized_work_rect( HWND hwnd )
> +{
> - RECT work_rect = { 0 };
> -
> - if ((GetWindowLongW( hwnd, GWL_STYLE ) & (WS_MINIMIZE | WS_MAXIMIZE)) == WS_MAXIMIZE)
> - {
> -
>    if (!get_work_rect( hwnd, &work_rect ))
> -
>        SetRect( &work_rect, 0, 0, GetSystemMetrics( SM_CXSCREEN ), GetSystemMetrics( SM_CYSCREEN ) );

Does this failure "SetRect" actually do anything?  get_work_rect() is
supposed to default to the primary monitor.  If it really is needed
shouldn't it use get_primary_monitor_rect()?
> - }
> - return work_rect;
>
> +}
> -
>
> +static void update_maximized_pos( WND *wnd, RECT *work_rect )
> +{
> - /* For top level windows covering the work area, we might have
> -
>   to "forget" the maximized position. Windows presumably does
> -
>   this to avoid situations where the border style changes,
> -
>   which would lead the window to be outside the screen, or the
> -
>   window gets reloaded on a different screen, and the "saved"
> -
>   position no longer applies to it (despite being maximized).
> -
> -
>   Some applications (e.g. Imperiums: Greek Wars) depend on this.
> - */

This sort of comment belongs above the function (like you did for
get_work_rect()).
> - if (wnd->parent && wnd->parent != GetDesktopWindow())
> -
>    return;
> -
> - if (wnd->dwStyle & WS_MAXIMIZE)
> - {
> -
>    if (wnd->window_rect.left  <= work_rect->left  && wnd->window_rect.top    <= work_rect->top &&
> -
>        wnd->window_rect.right >= work_rect->right && wnd->window_rect.bottom >= work_rect->bottom)
> -
>        wnd->max_pos.x = wnd->max_pos.y = -1;
> - }
> - else
> -
>    wnd->max_pos.x = wnd->max_pos.y = -1;
>
> +}
> -

The above two helpers could be moved further down the file to just
above when they're needed.  (They're specific enough that nothing else
is going to use them).

Huw.