[PATCH v3 1/2] user32: Fix menu item rectangle calculation in GetMenuItemRect().

Zhiyi Zhang zzhang at codeweavers.com
Mon Jul 16 10:04:38 CDT 2018


This fixes the menu of BibleWorks 10 not showing up and some todo
in tests.

Sysmenu is in non-client area. And because MapWindowPoints()
should not be used with non-client area coordinates. Doing so
results in wrong coordinates for sysmenu item rectangles.

Signed-off-by: Zhiyi Zhang <zzhang at codeweavers.com>
---
v3: Fix the accessing menu pointer after it got released. Sorry I missed that.

 dlls/user32/menu.c       | 13 ++++++--
 dlls/user32/tests/menu.c | 67 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c
index ab1ae9bc1b..1fd3cc4a12 100644
--- a/dlls/user32/menu.c
+++ b/dlls/user32/menu.c
@@ -5306,6 +5306,7 @@ BOOL WINAPI GetMenuItemRect(HWND hwnd, HMENU hMenu, UINT uItem, RECT *rect)
 {
     POPUPMENU *menu;
     UINT pos;
+    RECT window_rect;
 
     TRACE("(%p,%p,%d,%p)\n", hwnd, hMenu, uItem, rect);
 
@@ -5324,9 +5325,17 @@ BOOL WINAPI GetMenuItemRect(HWND hwnd, HMENU hMenu, UINT uItem, RECT *rect)
 
     *rect = menu->items[pos].rect;
     OffsetRect(rect, menu->items_rect.left, menu->items_rect.top);
-    release_menu_ptr(menu);
 
-    MapWindowPoints(hwnd, 0, (POINT *)rect, 2);
+    /* Popup menu item draws in the client area */
+    if (menu->wFlags & MF_POPUP) MapWindowPoints(hwnd, 0, (POINT *)rect, 2);
+    /* Sysmenu draws in the non-client area, can't use MapWindowPoints in non-client area */
+    else
+    {
+        GetWindowRect(hwnd, &window_rect);
+        OffsetRect(rect, window_rect.left, window_rect.top);
+    }
+
+    release_menu_ptr(menu);
     return TRUE;
 }
 
diff --git a/dlls/user32/tests/menu.c b/dlls/user32/tests/menu.c
index 7f338ce9b1..54a5ea232a 100644
--- a/dlls/user32/tests/menu.c
+++ b/dlls/user32/tests/menu.c
@@ -341,6 +341,62 @@ static void test_getmenubarinfo(void)
     DestroyWindow(hwnd);
 }
 
+static void test_GetMenuItemRect(void)
+{
+    HWND hwnd;
+    HMENU hmenu;
+    HMENU popup_hmenu;
+    RECT window_rect;
+    RECT item_rect;
+    POINT client_top_left;
+    INT caption_height;
+    BOOL ret;
+
+    hwnd = CreateWindowW((LPCWSTR)MAKEINTATOM(atomMenuCheckClass), NULL, WS_OVERLAPPEDWINDOW, 0, 0, 100, 100, NULL,
+                         NULL, NULL, NULL);
+    ok(hwnd != NULL, "CreateWindow failed with error %d\n", GetLastError());
+    hmenu = CreateMenu();
+    ok(hmenu != NULL, "CreateMenu failed with error %d\n", GetLastError());
+    popup_hmenu = CreatePopupMenu();
+    ok(popup_hmenu != NULL, "CreatePopupMenu failed with error %d\n", GetLastError());
+    ret = AppendMenuA(popup_hmenu, MF_STRING, 0, "Popup");
+    ok(ret, "AppendMenu failed with error %d\n", GetLastError());
+    ret = AppendMenuA(hmenu, MF_STRING | MF_POPUP, (UINT_PTR)popup_hmenu, "Menu");
+    ok(ret, "AppendMenu failed with error %d\n", GetLastError());
+    ret = SetMenu(hwnd, hmenu);
+    ok(ret, "SetMenu failed with error %d\n", GetLastError());
+
+    /* Get the menu item rectangle of the displayed sysmenu item */
+    ret = GetMenuItemRect(hwnd, hmenu, 0, &item_rect);
+    ok(ret, "GetMenuItemRect failed with error %d\n", GetLastError());
+    GetWindowRect(hwnd, &window_rect);
+    /* Get the screen coordinate of the left top corner of the client rectangle */
+    client_top_left.x = 0;
+    client_top_left.y = 0;
+    MapWindowPoints(hwnd, 0, &client_top_left, 1);
+    caption_height = GetSystemMetrics(SM_CYFRAME) + GetSystemMetrics(SM_CYCAPTION);
+
+    ok(item_rect.left == client_top_left.x, "Expect item_rect.left %d == %d\n", item_rect.left, client_top_left.x);
+    ok(item_rect.right <= window_rect.right, "Expect item_rect.right %d <= %d\n", item_rect.right, window_rect.right);
+    /* A gap of 1 pixel is added deliberately in commit 75f9e64, so using equal operator would fail on Wine.
+     * Check that top and bottom are correct with 1 pixel margin tolerance */
+    ok(item_rect.top - (window_rect.top + caption_height) <= 1, "Expect item_rect.top %d - %d <= 1\n", item_rect.top,
+       window_rect.top + caption_height);
+    ok(item_rect.bottom - (client_top_left.y - 1) <= 1, "Expect item_rect.bottom %d - %d <= 1\n", item_rect.bottom,
+       client_top_left.y - 1);
+
+    /* Get the item rectangle of the not yet displayed popup menu item. */
+    ret = GetMenuItemRect(hwnd, popup_hmenu, 0, &item_rect);
+    ok(ret, "GetMenuItemRect failed with error %d\n", GetLastError());
+    ok(item_rect.left == client_top_left.x, "Expect item_rect.left %d == %d\n", item_rect.left, client_top_left.x);
+    ok(item_rect.right == client_top_left.x, "Expect item_rect.right %d == %d\n", item_rect.right, client_top_left.x);
+    ok(item_rect.top == client_top_left.y, "Expect item_rect.top %d == %d\n", item_rect.top, client_top_left.y);
+    ok(item_rect.bottom == client_top_left.y, "Expect item_rect.bottom %d == %d\n", item_rect.bottom,
+       client_top_left.y);
+
+    DestroyWindow(hwnd);
+}
+
 static void test_system_menu(void)
 {
     WCHAR testW[] = {'t','e','s','t',0};
@@ -2188,15 +2244,15 @@ static struct menu_mouse_tests_s {
     { INPUT_KEYBOARD, {{0}}, {VK_F10, 0}, TRUE, FALSE },
     { INPUT_KEYBOARD, {{0}}, {VK_F10, 0}, FALSE, FALSE },
 
-    { INPUT_MOUSE, {{1, 2}, {0}}, {0}, TRUE, TRUE }, /* test 20 */
+    { INPUT_MOUSE, {{1, 2}, {0}}, {0}, TRUE, FALSE }, /* test 20 */
     { INPUT_MOUSE, {{1, 1}, {0}}, {0}, FALSE, FALSE },
-    { INPUT_MOUSE, {{1, 0}, {0}}, {0}, TRUE, TRUE },
+    { INPUT_MOUSE, {{1, 0}, {0}}, {0}, TRUE, FALSE },
     { INPUT_MOUSE, {{1, 1}, {0}}, {0}, FALSE, FALSE },
-    { INPUT_MOUSE, {{1, 0}, {2, 2}, {0}}, {0}, TRUE, TRUE },
+    { INPUT_MOUSE, {{1, 0}, {2, 2}, {0}}, {0}, TRUE, FALSE },
     { INPUT_MOUSE, {{2, 1}, {0}}, {0}, FALSE, FALSE },
-    { INPUT_MOUSE, {{1, 0}, {2, 0}, {0}}, {0}, TRUE, TRUE },
+    { INPUT_MOUSE, {{1, 0}, {2, 0}, {0}}, {0}, TRUE, FALSE },
     { INPUT_MOUSE, {{3, 0}, {0}}, {0}, FALSE, FALSE },
-    { INPUT_MOUSE, {{1, 0}, {2, 0}, {0}}, {0}, TRUE, TRUE },
+    { INPUT_MOUSE, {{1, 0}, {2, 0}, {0}}, {0}, TRUE, FALSE },
     { INPUT_MOUSE, {{3, 1}, {0}}, {0}, TRUE, TRUE },
     { INPUT_MOUSE, {{1, 1}, {0}}, {0}, FALSE, FALSE },
     { -1 }
@@ -4199,6 +4255,7 @@ START_TEST(menu)
     test_subpopup_locked_by_menu();
     test_menu_ownerdraw();
     test_getmenubarinfo();
+    test_GetMenuItemRect();
     test_menu_bmp_and_string();
     test_menu_getmenuinfo();
     test_menu_setmenuinfo();
-- 
2.18.0





More information about the wine-devel mailing list