Hi all, my previous menu patch ("Make sure we clear the owning window's hMenu in DestroyMenu().") caused problems with certain apps. That's because DestroyMenu set the hMenu member of the hWnd that the hMenu had been assigned to to 0. Since a SetMenu changed the hMenu of a hWnd to a new hMenu without removing the link from the old hMenu to its thus old hWnd, the *new* hMenu handle registered in the hWnd got erased as soon as we did a DestroyMenu on the *old* hMenu not affiliated with the hWnd any more. - make sure we set old hMenu's hWnd pointer to 0 when doing a SetMenu - added a small menu conformance test (dlls/user/tests/menu.c) Note that this is only a workaround for the bigger issue: a pending rewrite of the menu handling to really cope with the changes from Win 3.x to Win9x (which affect DestroyMenu, SetMenu etc.). -- Andreas Mohr Stauferstr. 6, D-71272 Renningen, Germany
Determining best CVS host... Using CVSROOT :pserver:cvs@rhlx01.fht-esslingen.de:/home/wine Index: controls/menu.c =================================================================== RCS file: /home/wine/wine/controls/menu.c,v retrieving revision 1.146 diff -u -r1.146 menu.c --- controls/menu.c 28 Aug 2002 23:31:56 -0000 1.146 +++ controls/menu.c 6 Sep 2002 21:04:27 -0000 @@ -24,6 +24,14 @@ * Note: the style MF_MOUSESELECT is used to mark popup items that * have been selected, i.e. their popup menu is currently displayed. * This is probably not the meaning this style has in MS-Windows. + * + * TODO + * - Win9x has menu changes compared to Win 3.x: + * DestroyMenu() doesn't unregister hMenu in owning hWnd in Win3.x, + * whereas Win9x does it (and many other menu handling changes + * in this respect). Roughly spoken, the current DestroyMenu, GetMenu + * code is nothing more than a bad hack designed to make as many + * programs work with the current code as possible. */ #include "config.h" @@ -3844,8 +3852,13 @@ if (!lppop) return FALSE; - /* unregister menu in owning window */ - SetWindowLongA( lppop->hWnd, GWL_ID, 0 ); + /* unregister menu in owning window. + * Note that this is problematic, since a hMenu can be assigned + * to several hWnd. Thus you can't keep track of all hWnd a menu + * is assigned to. FIXME: Win9x menu handling rewrite. */ + if (lppop->hWnd) + SetWindowLongA( lppop->hWnd, GWL_ID, 0 ); + lppop->wMagic = 0; /* Mark it as destroyed */ @@ -3950,6 +3963,15 @@ HMENU WINAPI GetMenu( HWND hWnd ) { HMENU retvalue = (HMENU)GetWindowLongA( hWnd, GWL_ID ); + /* need to check whether HMENU is still valid, + * since DestroyMenu doesn't zero hMenu of a HWND's struct + * (as a HMENU can be assigned to *several* HWNDs, + * there's no way of finding all HWNDs to zero out their hMenu !) + * FIXME: Win9x menu handling rewrite. + */ + if (!MENU_GetMenu(retvalue)) + retvalue = 0; + TRACE("for %04x returning %04x\n", hWnd, retvalue); return retvalue; } @@ -3960,6 +3982,7 @@ */ BOOL WINAPI SetMenu( HWND hWnd, HMENU hMenu ) { + HMENU hMenuOld; TRACE("(%04x, %04x);\n", hWnd, hMenu); if (hMenu && !IsMenu(hMenu)) @@ -3970,8 +3993,25 @@ if (GetWindowLongA( hWnd, GWL_STYLE ) & WS_CHILD) return FALSE; hWnd = WIN_GetFullHandle( hWnd ); + + hMenuOld = (HMENU)GetWindowLongA( hWnd, GWL_ID); + + /* bail out if no change */ + if (hMenuOld == hMenu) + return TRUE; + if (GetCapture() == hWnd) ReleaseCapture(); + if (hMenuOld) + { + LPPOPUPMENU lpmenu; + + if (!(lpmenu = MENU_GetMenu(hMenuOld))) return FALSE; + + /* set hWnd of old hMenu to 0, since we're unregistering it */ + lpmenu->hWnd = 0; + } + if (hMenu != 0) { LPPOPUPMENU lpmenu; @@ -3981,6 +4021,8 @@ lpmenu->hWnd = hWnd; lpmenu->Height = 0; /* Make sure we recalculate the size */ } + + /* register new hMenu in the window */ SetWindowLongA( hWnd, GWL_ID, hMenu ); if (IsWindowVisible(hWnd))
/* Unit test suite for some menu functions * * Copyright 2002 Andreas Mohr * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ #include <assert.h> #include <stdlib.h> #include <string.h> #include <stdio.h> #include "wine/test.h" #include "winbase.h" #include "wingdi.h" #include "winuser.h" LRESULT CALLBACK WndProc (HWND hWnd1, UINT iMsg, WPARAM wParam, LPARAM lParam) { return DefWindowProcA (hWnd1, iMsg, wParam, lParam) ; } static BOOL menutest1 (void) { WNDCLASSEXA wndclass; HMENU hMenu; HWND hWnd1 = 0, hWnd2 = 0; DWORD res; wndclass.cbSize = sizeof (wndclass) ; wndclass.style = CS_HREDRAW | CS_VREDRAW ; wndclass.lpfnWndProc = WndProc ; wndclass.cbClsExtra = 0 ; wndclass.cbWndExtra = 0 ; wndclass.hInstance = 0 ; wndclass.hIcon = 0; wndclass.hCursor = LoadCursorA ((DWORD)NULL, IDC_ARROWA) ; wndclass.hbrBackground = (HBRUSH) GetStockObject (WHITE_BRUSH) ; wndclass.lpszMenuName = NULL ; wndclass.lpszClassName = "test"; wndclass.hIconSm = 0; RegisterClassExA (&wndclass) ; hWnd1 = CreateWindowA ("test", "test2", WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, NULL, NULL, 0, NULL) ; hWnd2 = CreateWindowA ("test", "test2", WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, (DWORD)NULL, (DWORD)NULL, 0, (DWORD)NULL) ; hMenu = CreateMenu(); ok((hMenu != 0), "CreateMenu1 failed"); res = SetMenu(hWnd1, hMenu); ok((res == 1),"SetMenu1 failed"); res = GetMenu(hWnd1); ok((res != 0), "GetMenu1 failed"); res = SetMenu(hWnd2, hMenu); ok((res == 1), "SetMenu2 failed"); res = GetMenu(hWnd1); ok((res != 0), "GetMenu2 failed"); res = GetMenu(hWnd2); ok((res != 0), "GetMenu3 failed"); res = SetMenu(hWnd2, hMenu); ok((res == 1), "SetMenu3 failed"); res = DestroyMenu(hMenu); ok((res == 1), "DestroyMenu1 failed"); res = CreateMenu(); ok((res != 0), "CreateMenu2 failed"); /* the two ToDos are happening due to reuse of the same handle by * Wine at a DestroyMenu/CreateMenu combo */ todo_wine { res = GetMenu(hWnd1); ok((res == 0), "GetMenu4 failed"); } res = GetMenu(hWnd2); ok((res == 0), "GetMenu5 failed"); res = SetMenu(hWnd1, 0); ok((res == 1), "SetMenu4 failed"); todo_wine { res = DestroyMenu(hMenu); ok((res == 0), "DestroyMenu2 failed"); } res = GetMenu(hWnd1); ok((res == 0), "GetMenu6 failed"); res = CreateMenu(); ok((res != 0), "CreateMenu3 failed"); res = GetMenu(hWnd1); ok((res == 0), "GetMenu7 failed"); return TRUE; } START_TEST(menu) { menutest1(); }