Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- dlls/user32/menu.c | 544 ++++++++++++++++++++++++++++++--------------- 1 file changed, 369 insertions(+), 175 deletions(-)
diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c index 537c2ebe1a..dcec42cdf2 100644 --- a/dlls/user32/menu.c +++ b/dlls/user32/menu.c @@ -592,52 +592,56 @@ static UINT MENU_GetStartOfPrevColumn( * Find a menu item. Return a pointer on the item, and modifies *hmenu * in case the item was in a sub-menu. */ -static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) +static MENUITEM *MENU_FindItem(POPUPMENU **menu, UINT *nPos, UINT wFlags) { - POPUPMENU *menu; - MENUITEM *fallback = NULL; - UINT fallback_pos = 0; - UINT i; + UINT fallback_pos = ~0u; + + if (!*menu) + return NULL;
- if ((*hmenu == (HMENU)0xffff) || (!(menu = MENU_GetMenu(*hmenu)))) return NULL; if (wFlags & MF_BYPOSITION) { - if (*nPos >= menu->nItems) return NULL; - return &menu->items[*nPos]; + if (*nPos >= (*menu)->nItems) return NULL; + return &(*menu)->items[*nPos]; } else { - MENUITEM *item = menu->items; - for (i = 0; i < menu->nItems; i++, item++) + MENUITEM *item = (*menu)->items; + UINT i; + + for (i = 0; i < (*menu)->nItems; i++, item++) { if (item->fType & MF_POPUP) { - HMENU hsubmenu = item->hSubMenu; - MENUITEM *subitem = MENU_FindItem( &hsubmenu, nPos, wFlags ); - if (subitem) - { - *hmenu = hsubmenu; - return subitem; - } + POPUPMENU *submenu = grab_menu_ptr(item->hSubMenu); + if (MENU_FindItem(&submenu, nPos, wFlags)) + { + release_menu_ptr(*menu); + *menu = submenu; + return &(*menu)->items[*nPos]; + } else if (item->wID == *nPos) { /* fallback to this item if nothing else found */ fallback_pos = i; - fallback = item; } + release_menu_ptr(submenu); } else if (item->wID == *nPos) { *nPos = i; - return item; + return &(*menu)->items[*nPos]; } } }
- if (fallback) + if (fallback_pos != ~0u) + { *nPos = fallback_pos; + return &(*menu)->items[*nPos]; + }
- return fallback; + return NULL; }
/*********************************************************************** @@ -2135,13 +2139,9 @@ static void MENU_MoveSelection( HWND hwndOwner, HMENU hmenu, INT offset ) * * Insert (allocate) a new item into a menu. */ -static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags ) +static MENUITEM *MENU_InsertItem( POPUPMENU *menu, UINT pos, UINT flags ) { MENUITEM *newItems; - POPUPMENU *menu; - - if (!(menu = MENU_GetMenu(hMenu))) - return NULL;
/* Find where to insert new item */
@@ -2149,12 +2149,8 @@ static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags ) if (pos > menu->nItems) pos = menu->nItems; } else { - if (!MENU_FindItem( &hMenu, &pos, flags )) + if (!MENU_FindItem(&menu, &pos, flags)) pos = menu->nItems; - else { - if (!(menu = MENU_GetMenu( hMenu ))) - return NULL; - } }
/* Make sure that MDI system buttons stay on the right side. @@ -3706,13 +3702,23 @@ BOOL WINAPI ChangeMenuW( HMENU hMenu, UINT pos, LPCWSTR data, */ DWORD WINAPI CheckMenuItem( HMENU hMenu, UINT id, UINT flags ) { - MENUITEM *item; - DWORD ret; + POPUPMENU *menu; + DWORD ret = -1; + + menu = grab_menu_ptr(hMenu); + if (menu) + { + MENUITEM *item; + + if ((item = MENU_FindItem(&menu, &id, flags))) + { + ret = item->fState & MF_CHECKED; + if (flags & MF_CHECKED) item->fState |= MF_CHECKED; + else item->fState &= ~MF_CHECKED; + } + release_menu_ptr(menu); + }
- if (!(item = MENU_FindItem( &hMenu, &id, flags ))) return -1; - ret = item->fState & MF_CHECKED; - if (flags & MF_CHECKED) item->fState |= MF_CHECKED; - else item->fState &= ~MF_CHECKED; return ret; }
@@ -3726,36 +3732,41 @@ BOOL WINAPI EnableMenuItem( HMENU hMenu, UINT wItemID, UINT wFlags ) MENUITEM *item; POPUPMENU *menu;
- TRACE("(%p, %04x, %04x) !\n", hMenu, wItemID, wFlags); + TRACE("(%p, %04x, %04x)\n", hMenu, wItemID, wFlags);
/* Get the Popupmenu to access the owner menu */ - if (!(menu = MENU_GetMenu(hMenu))) - return (UINT)-1; - - if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) - return (UINT)-1; + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &wItemID, wFlags))) + { + release_menu_ptr(menu); + return (UINT)-1; + }
oldflags = item->fState & (MF_GRAYED | MF_DISABLED); item->fState ^= (oldflags ^ wFlags) & (MF_GRAYED | MF_DISABLED);
/* If the close item in the system menu change update the close button */ - if((item->wID == SC_CLOSE) && (oldflags != wFlags)) + if ((item->wID == SC_CLOSE) && (oldflags != wFlags) && menu->hSysMenuOwner) { - if (menu->hSysMenuOwner != 0) - { - RECT rc; - POPUPMENU* parentMenu; + POPUPMENU *parent_menu; + HWND hwnd; + RECT rc;
- /* Get the parent menu to access*/ - if (!(parentMenu = MENU_GetMenu(menu->hSysMenuOwner))) - return (UINT)-1; + if (!(parent_menu = grab_menu_ptr(menu->hSysMenuOwner))) + { + release_menu_ptr(menu); + return (UINT)-1; + }
- /* Refresh the frame to reflect the change */ - WIN_GetRectangles( parentMenu->hWnd, COORDS_CLIENT, &rc, NULL ); - rc.bottom = 0; - RedrawWindow(parentMenu->hWnd, &rc, 0, RDW_FRAME | RDW_INVALIDATE | RDW_NOCHILDREN); - } + hwnd = parent_menu->hWnd; + release_menu_ptr(parent_menu); + + /* Refresh the frame to reflect the change */ + WIN_GetRectangles(hwnd, COORDS_CLIENT, &rc, NULL); + rc.bottom = 0; + RedrawWindow(hwnd, &rc, 0, RDW_FRAME | RDW_INVALIDATE | RDW_NOCHILDREN); } + release_menu_ptr(menu);
return oldflags; } @@ -3770,21 +3781,39 @@ INT WINAPI GetMenuStringA( LPSTR str, /* [out] outbuffer. If NULL, func returns entry length*/ INT nMaxSiz, /* [in] length of buffer. if 0, func returns entry len*/ UINT wFlags /* [in] MF_ flags */ -) { +) +{ + POPUPMENU *menu; MENUITEM *item; + INT ret;
TRACE("menu=%p item=%04x ptr=%p len=%d flags=%04x\n", hMenu, wItemID, str, nMaxSiz, wFlags ); + if (str && nMaxSiz) str[0] = '\0'; - if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) { + + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &wItemID, wFlags))) + { + release_menu_ptr(menu); SetLastError( ERROR_MENU_ITEM_NOT_FOUND); return 0; } - if (!item->text) return 0; - if (!str || !nMaxSiz) return WideCharToMultiByte( CP_ACP, 0, item->text, -1, NULL, 0, NULL, NULL ); - if (!WideCharToMultiByte( CP_ACP, 0, item->text, -1, str, nMaxSiz, NULL, NULL )) - str[nMaxSiz-1] = 0; + + if (!item->text) + ret = 0; + else if (!str || !nMaxSiz) + ret = WideCharToMultiByte( CP_ACP, 0, item->text, -1, NULL, 0, NULL, NULL ); + else + { + if (!WideCharToMultiByte( CP_ACP, 0, item->text, -1, str, nMaxSiz, NULL, NULL )) + str[nMaxSiz-1] = 0; + ret = strlen(str); + } + release_menu_ptr(menu); + TRACE("returning %s\n", debugstr_a(str)); - return strlen(str); + + return ret; }
@@ -3794,22 +3823,38 @@ INT WINAPI GetMenuStringA( INT WINAPI GetMenuStringW( HMENU hMenu, UINT wItemID, LPWSTR str, INT nMaxSiz, UINT wFlags ) { + POPUPMENU *menu; MENUITEM *item; + INT ret;
TRACE("menu=%p item=%04x ptr=%p len=%d flags=%04x\n", hMenu, wItemID, str, nMaxSiz, wFlags ); + if (str && nMaxSiz) str[0] = '\0'; - if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) { + + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &wItemID, wFlags))) + { + release_menu_ptr(menu); SetLastError( ERROR_MENU_ITEM_NOT_FOUND); return 0; } - if (!str || !nMaxSiz) return item->text ? strlenW(item->text) : 0; - if( !(item->text)) { + + if (!str || !nMaxSiz) + ret = item->text ? strlenW(item->text) : 0; + else if (!(item->text)) + { str[0] = 0; - return 0; + ret = 0; } - lstrcpynW( str, item->text, nMaxSiz ); + else + { + lstrcpynW( str, item->text, nMaxSiz ); + ret = strlenW(str); + } + release_menu_ptr(menu); + TRACE("returning %s\n", debugstr_w(str)); - return strlenW(str); + return ret; }
@@ -3819,13 +3864,27 @@ INT WINAPI GetMenuStringW( HMENU hMenu, UINT wItemID, BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID, UINT wHilite ) { - LPPOPUPMENU menu; + POPUPMENU *menu; + BOOL changed; + TRACE("(%p, %p, %04x, %04x);\n", hWnd, hMenu, wItemID, wHilite); - if (!MENU_FindItem( &hMenu, &wItemID, wHilite )) return FALSE; - if (!(menu = MENU_GetMenu(hMenu))) return FALSE; - if (menu->FocusedItem == wItemID) return TRUE; - MENU_HideSubPopups( hWnd, hMenu, FALSE, 0 ); - MENU_SelectItem( hWnd, hMenu, wItemID, TRUE, 0 ); + + menu = grab_menu_ptr(hMenu); + if (!MENU_FindItem(&menu, &wItemID, wHilite)) + { + release_menu_ptr(menu); + return FALSE; + } + + changed = menu->FocusedItem != wItemID; + release_menu_ptr(menu); + + if (changed) + { + MENU_HideSubPopups( hWnd, hMenu, FALSE, 0 ); + MENU_SelectItem( hWnd, hMenu, wItemID, TRUE, 0 ); + } + return TRUE; }
@@ -3835,23 +3894,35 @@ BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID, */ UINT WINAPI GetMenuState( HMENU hMenu, UINT wItemID, UINT wFlags ) { + POPUPMENU *menu; MENUITEM *item; + UINT state; + TRACE("(menu=%p, id=%04x, flags=%04x);\n", hMenu, wItemID, wFlags); - if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) return -1; + + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &wItemID, wFlags))) + { + release_menu_ptr(menu); + return -1; + } debug_print_menuitem (" item: ", item, ""); if (item->fType & MF_POPUP) { - POPUPMENU *menu = MENU_GetMenu( item->hSubMenu ); - if (!menu) return -1; - else return (menu->nItems << 8) | ((item->fState|item->fType) & 0xff); + POPUPMENU *submenu = grab_menu_ptr(item->hSubMenu); + state = submenu ? (submenu->nItems << 8) | ((item->fState | item->fType) & 0xff) : -1; + release_menu_ptr(submenu); } else { /* We used to (from way back then) mask the result to 0xff. */ /* I don't know why and it seems wrong as the documented */ /* return flag MF_SEPARATOR is outside that mask. */ - return (item->fType | item->fState); + state = (item->fType | item->fState); } + release_menu_ptr(menu); + + return state; }
@@ -3877,12 +3948,19 @@ INT WINAPI GetMenuItemCount( HMENU hMenu ) */ UINT WINAPI GetMenuItemID( HMENU hMenu, INT nPos ) { - MENUITEM * lpmi; - - if (!(lpmi = MENU_FindItem(&hMenu,(UINT*)&nPos,MF_BYPOSITION))) return -1; - if (lpmi->fType & MF_POPUP) return -1; - return lpmi->wID; + POPUPMENU *menu; + MENUITEM *item; + UINT id;
+ menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, (UINT *)&nPos, MF_BYPOSITION))) + { + release_menu_ptr(menu); + return -1; + } + id = item->fType & MF_POPUP ? -1 : item->wID; + release_menu_ptr(menu); + return id; }
@@ -3937,6 +4015,8 @@ BOOL WINAPI InsertMenuW( HMENU hMenu, UINT pos, UINT flags, { MENUITEM *item; MENUITEMINFOW mii; + POPUPMENU *menu; + BOOL ret;
if (IS_STRING_ITEM(flags) && str) TRACE("hMenu %p, pos %d, flags %08x, id %04lx, str %s\n", @@ -3944,16 +4024,25 @@ BOOL WINAPI InsertMenuW( HMENU hMenu, UINT pos, UINT flags, else TRACE("hMenu %p, pos %d, flags %08x, id %04lx, str %p (not a string)\n", hMenu, pos, flags, id, str );
- if (!(item = MENU_InsertItem( hMenu, pos, flags ))) return FALSE; - MENU_mnu2mnuii( flags, id, str, &mii); - if (!(SetMenuItemInfo_common( item, &mii, TRUE))) + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_InsertItem( menu, pos, flags ))) { - RemoveMenu( hMenu, pos, flags ); + release_menu_ptr(menu); return FALSE; }
- item->hCheckBit = item->hUnCheckBit = 0; - return TRUE; + MENU_mnu2mnuii( flags, id, str, &mii); + + ret = SetMenuItemInfo_common(item, &mii, TRUE); + + if (ret) + item->hCheckBit = item->hUnCheckBit = 0; + else + RemoveMenu( hMenu, pos, flags ); + + release_menu_ptr(menu); + + return ret; }
@@ -4006,16 +4095,20 @@ BOOL WINAPI AppendMenuW( HMENU hMenu, UINT flags, */ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) { - LPPOPUPMENU menu; + POPUPMENU *menu; MENUITEM *item;
TRACE("(menu=%p pos=%04x flags=%04x)\n",hMenu, nPos, wFlags); - if (!(item = MENU_FindItem( &hMenu, &nPos, wFlags ))) return FALSE; - if (!(menu = MENU_GetMenu(hMenu))) return FALSE;
- /* Remove item */ + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &nPos, wFlags))) + { + release_menu_ptr(menu); + return FALSE; + }
- MENU_FreeItemData( item ); + /* Remove item */ + MENU_FreeItemData(item);
if (--menu->nItems == 0) { @@ -4035,6 +4128,8 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) menu->nItems * sizeof(MENUITEM) ); if (new_items) menu->items = new_items; } + release_menu_ptr(menu); + return TRUE; }
@@ -4044,10 +4139,20 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) */ BOOL WINAPI DeleteMenu( HMENU hMenu, UINT nPos, UINT wFlags ) { - MENUITEM *item = MENU_FindItem( &hMenu, &nPos, wFlags ); - if (!item) return FALSE; + POPUPMENU *menu; + MENUITEM *item; + + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &nPos, wFlags))) + { + release_menu_ptr(menu); + return FALSE; + } + if (item->fType & MF_POPUP) DestroyMenu( item->hSubMenu ); - /* nPos is now the position of the item */ + release_menu_ptr(menu); + + /* nPos is now the position of the item */ RemoveMenu( hMenu, nPos, wFlags | MF_BYPOSITION ); return TRUE; } @@ -4059,23 +4164,28 @@ BOOL WINAPI DeleteMenu( HMENU hMenu, UINT nPos, UINT wFlags ) BOOL WINAPI ModifyMenuW( HMENU hMenu, UINT pos, UINT flags, UINT_PTR id, LPCWSTR str ) { - MENUITEM *item; MENUITEMINFOW mii; + POPUPMENU *menu; + MENUITEM *item; + BOOL ret;
if (IS_STRING_ITEM(flags)) TRACE("%p %d %04x %04lx %s\n", hMenu, pos, flags, id, debugstr_w(str) ); else TRACE("%p %d %04x %04lx %p\n", hMenu, pos, flags, id, str );
- if (!(item = MENU_FindItem( &hMenu, &pos, flags ))) + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &pos, flags))) { + release_menu_ptr(menu); /* workaround for Word 95: pretend that SC_TASKLIST item exists */ - if (pos == SC_TASKLIST && !(flags & MF_BYPOSITION)) return TRUE; - return FALSE; + return pos == SC_TASKLIST && !(flags & MF_BYPOSITION); } - MENU_GetMenu(hMenu)->Height = 0; /* force size recalculate */ + menu->Height = 0; /* force size recalculate */ MENU_mnu2mnuii( flags, id, str, &mii); - return SetMenuItemInfo_common( item, &mii, TRUE); + ret = SetMenuItemInfo_common(item, &mii, TRUE); + release_menu_ptr(menu); + return ret; }
@@ -4134,9 +4244,15 @@ DWORD WINAPI GetMenuCheckMarkDimensions(void) BOOL WINAPI SetMenuItemBitmaps( HMENU hMenu, UINT nPos, UINT wFlags, HBITMAP hNewUnCheck, HBITMAP hNewCheck) { + POPUPMENU *menu; MENUITEM *item;
- if (!(item = MENU_FindItem( &hMenu, &nPos, wFlags ))) return FALSE; + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &nPos, wFlags))) + { + release_menu_ptr(menu); + return FALSE; + }
if (!hNewCheck && !hNewUnCheck) { @@ -4148,6 +4264,7 @@ BOOL WINAPI SetMenuItemBitmaps( HMENU hMenu, UINT nPos, UINT wFlags, item->hUnCheckBit = hNewUnCheck; item->fState |= MF_USECHECKBITMAPS; } + release_menu_ptr(menu); return TRUE; }
@@ -4418,11 +4535,22 @@ BOOL WINAPI SetMenu( HWND hWnd, HMENU hMenu ) */ HMENU WINAPI GetSubMenu( HMENU hMenu, INT nPos ) { - MENUITEM * lpmi; + HMENU submenu = 0; + POPUPMENU *menu; + MENUITEM *item; + + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, (UINT *)&nPos, MF_BYPOSITION))) + { + release_menu_ptr(menu); + return 0; + }
- if (!(lpmi = MENU_FindItem(&hMenu,(UINT*)&nPos,MF_BYPOSITION))) return 0; - if (!(lpmi->fType & MF_POPUP)) return 0; - return lpmi->hSubMenu; + if (item->fType & MF_POPUP) + submenu = item->hSubMenu; + release_menu_ptr(menu); + + return submenu; }
@@ -4631,31 +4759,39 @@ BOOL WINAPI IsMenu(HMENU hmenu) * GetMenuItemInfo_common */
-static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos, +static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT id, BOOL bypos, LPMENUITEMINFOW lpmii, BOOL unicode) { - MENUITEM *menu = MENU_FindItem (&hmenu, &item, bypos ? MF_BYPOSITION : 0); - - debug_print_menuitem("GetMenuItemInfo_common: ", menu, ""); + POPUPMENU *menu; + MENUITEM *item;
- if (!menu) { + menu = grab_menu_ptr(hmenu); + if (!(item = MENU_FindItem(&menu, &id, bypos ? MF_BYPOSITION : 0))) + { + release_menu_ptr(menu); SetLastError( ERROR_MENU_ITEM_NOT_FOUND); return FALSE; }
- if( lpmii->fMask & MIIM_TYPE) { - if( lpmii->fMask & ( MIIM_STRING | MIIM_FTYPE | MIIM_BITMAP)) { - WARN("invalid combination of fMask bits used\n"); - /* this does not happen on Win9x/ME */ - SetLastError( ERROR_INVALID_PARAMETER); - return FALSE; - } - lpmii->fType = menu->fType & MENUITEMINFO_TYPE_MASK; - if (menu->hbmpItem && !IS_MAGIC_BITMAP(menu->hbmpItem)) + debug_print_menuitem("GetMenuItemInfo_common: ", item, ""); + + if ((lpmii->fMask & MIIM_TYPE) && (lpmii->fMask & (MIIM_STRING | MIIM_FTYPE | MIIM_BITMAP))) + { + release_menu_ptr(menu); + WARN("invalid combination of fMask bits used\n"); + /* this does not happen on Win9x/ME */ + SetLastError( ERROR_INVALID_PARAMETER); + return FALSE; + } + + if (lpmii->fMask & MIIM_TYPE) + { + lpmii->fType = item->fType & MENUITEMINFO_TYPE_MASK; + if (item->hbmpItem && !IS_MAGIC_BITMAP(item->hbmpItem)) lpmii->fType |= MFT_BITMAP; - lpmii->hbmpItem = menu->hbmpItem; /* not on Win9x/ME */ + lpmii->hbmpItem = item->hbmpItem; /* not on Win9x/ME */ if( lpmii->fType & MFT_BITMAP) { - lpmii->dwTypeData = (LPWSTR) menu->hbmpItem; + lpmii->dwTypeData = (LPWSTR)item->hbmpItem; lpmii->cch = 0; } else if( lpmii->fType & (MFT_OWNERDRAW | MFT_SEPARATOR)) { /* this does not happen on Win9x/ME */ @@ -4666,7 +4802,8 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,
/* copy the text string */ if ((lpmii->fMask & (MIIM_TYPE|MIIM_STRING))) { - if( !menu->text ) { + if (!item->text) + { if(lpmii->dwTypeData && lpmii->cch) { if( unicode) *((WCHAR *)lpmii->dwTypeData) = 0; @@ -4678,16 +4815,16 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos, int len; if (unicode) { - len = strlenW(menu->text); + len = strlenW(item->text); if(lpmii->dwTypeData && lpmii->cch) - lstrcpynW(lpmii->dwTypeData, menu->text, lpmii->cch); + lstrcpynW(lpmii->dwTypeData, item->text, lpmii->cch); } else { - len = WideCharToMultiByte( CP_ACP, 0, menu->text, -1, NULL, + len = WideCharToMultiByte( CP_ACP, 0, item->text, -1, NULL, 0, NULL, NULL ) - 1; if(lpmii->dwTypeData && lpmii->cch) - if (!WideCharToMultiByte( CP_ACP, 0, menu->text, -1, + if (!WideCharToMultiByte( CP_ACP, 0, item->text, -1, (LPSTR)lpmii->dwTypeData, lpmii->cch, NULL, NULL )) ((LPSTR)lpmii->dwTypeData)[lpmii->cch - 1] = 0; } @@ -4706,33 +4843,35 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos, }
if (lpmii->fMask & MIIM_FTYPE) - lpmii->fType = menu->fType & MENUITEMINFO_TYPE_MASK; + lpmii->fType = item->fType & MENUITEMINFO_TYPE_MASK;
if (lpmii->fMask & MIIM_BITMAP) - lpmii->hbmpItem = menu->hbmpItem; + lpmii->hbmpItem = item->hbmpItem;
if (lpmii->fMask & MIIM_STATE) - lpmii->fState = menu->fState & MENUITEMINFO_STATE_MASK; + lpmii->fState = item->fState & MENUITEMINFO_STATE_MASK;
if (lpmii->fMask & MIIM_ID) - lpmii->wID = menu->wID; + lpmii->wID = item->wID;
if (lpmii->fMask & MIIM_SUBMENU) - lpmii->hSubMenu = menu->hSubMenu; + lpmii->hSubMenu = item->hSubMenu; else { /* hSubMenu is always cleared * (not on Win9x/ME ) */ lpmii->hSubMenu = 0; }
- if (lpmii->fMask & MIIM_CHECKMARKS) { - lpmii->hbmpChecked = menu->hCheckBit; - lpmii->hbmpUnchecked = menu->hUnCheckBit; + if (lpmii->fMask & MIIM_CHECKMARKS) + { + lpmii->hbmpChecked = item->hCheckBit; + lpmii->hbmpUnchecked = item->hUnCheckBit; } if (lpmii->fMask & MIIM_DATA) - lpmii->dwItemData = menu->dwItemData; + lpmii->dwItemData = item->dwItemData;
- return TRUE; + release_menu_ptr(menu); + return TRUE; }
/********************************************************************** @@ -4946,44 +5085,56 @@ static BOOL MENU_NormalizeMenuItemInfoStruct( const MENUITEMINFOW *pmii_in, /********************************************************************** * SetMenuItemInfoA (USER32.@) */ -BOOL WINAPI SetMenuItemInfoA(HMENU hmenu, UINT item, BOOL bypos, +BOOL WINAPI SetMenuItemInfoA(HMENU hmenu, UINT id, BOOL bypos, const MENUITEMINFOA *lpmii) { - MENUITEM *menuitem; MENUITEMINFOW mii; + POPUPMENU *menu; + MENUITEM *item; + BOOL ret;
- TRACE("hmenu %p, item %u, by pos %d, info %p\n", hmenu, item, bypos, lpmii); + TRACE("hmenu %p, id %u, by pos %d, info %p\n", hmenu, id, bypos, lpmii);
if (!MENU_NormalizeMenuItemInfoStruct( (const MENUITEMINFOW *)lpmii, &mii )) return FALSE;
- if (!(menuitem = MENU_FindItem( &hmenu, &item, bypos? MF_BYPOSITION : 0 ))) + menu = grab_menu_ptr(hmenu); + if (!(item = MENU_FindItem(&menu, &id, bypos ? MF_BYPOSITION : 0))) { + release_menu_ptr(menu); /* workaround for Word 95: pretend that SC_TASKLIST item exists */ - if (item == SC_TASKLIST && !bypos) return TRUE; - return FALSE; + return id == SC_TASKLIST && !bypos; } - return SetMenuItemInfo_common( menuitem, &mii, FALSE ); + + ret = SetMenuItemInfo_common( item, &mii, FALSE ); + release_menu_ptr(menu); + return ret; }
/********************************************************************** * SetMenuItemInfoW (USER32.@) */ -BOOL WINAPI SetMenuItemInfoW(HMENU hmenu, UINT item, BOOL bypos, +BOOL WINAPI SetMenuItemInfoW(HMENU hmenu, UINT id, BOOL bypos, const MENUITEMINFOW *lpmii) { - MENUITEM *menuitem; MENUITEMINFOW mii; + POPUPMENU *menu; + MENUITEM *item; + BOOL ret;
- TRACE("hmenu %p, item %u, by pos %d, info %p\n", hmenu, item, bypos, lpmii); + TRACE("hmenu %p, id %u, by pos %d, info %p\n", hmenu, id, bypos, lpmii);
if (!MENU_NormalizeMenuItemInfoStruct( lpmii, &mii )) return FALSE; - if (!(menuitem = MENU_FindItem( &hmenu, &item, bypos? MF_BYPOSITION : 0 ))) + + menu = grab_menu_ptr(hmenu); + if (!(item = MENU_FindItem(&menu, &id, bypos ? MF_BYPOSITION : 0))) { + release_menu_ptr(menu); /* workaround for Word 95: pretend that SC_TASKLIST item exists */ - if (item == SC_TASKLIST && !bypos) return TRUE; - return FALSE; + return id == SC_TASKLIST && !bypos; } - return SetMenuItemInfo_common( menuitem, &mii, TRUE ); + ret = SetMenuItemInfo_common( item, &mii, TRUE ); + release_menu_ptr(menu); + return ret; }
static BOOL set_menu_default_item(POPUPMENU *menu, UINT uItem, UINT bypos) @@ -5092,13 +5243,18 @@ BOOL WINAPI InsertMenuItemA(HMENU hMenu, UINT uItem, BOOL bypos, { MENUITEM *item; MENUITEMINFOW mii; + POPUPMENU *menu; + BOOL ret;
TRACE("hmenu %p, item %04x, by pos %d, info %p\n", hMenu, uItem, bypos, lpmii);
if (!MENU_NormalizeMenuItemInfoStruct( (const MENUITEMINFOW *)lpmii, &mii )) return FALSE;
- item = MENU_InsertItem(hMenu, uItem, bypos ? MF_BYPOSITION : 0 ); - return SetMenuItemInfo_common(item, &mii, FALSE); + menu = grab_menu_ptr(hMenu); + item = MENU_InsertItem(menu, uItem, bypos ? MF_BYPOSITION : 0 ); + ret = SetMenuItemInfo_common(item, &mii, FALSE); + release_menu_ptr(menu); + return ret; }
@@ -5110,13 +5266,18 @@ BOOL WINAPI InsertMenuItemW(HMENU hMenu, UINT uItem, BOOL bypos, { MENUITEM *item; MENUITEMINFOW mii; + POPUPMENU *menu; + BOOL ret;
TRACE("hmenu %p, item %04x, by pos %d, info %p\n", hMenu, uItem, bypos, lpmii);
if (!MENU_NormalizeMenuItemInfoStruct( lpmii, &mii )) return FALSE;
- item = MENU_InsertItem(hMenu, uItem, bypos ? MF_BYPOSITION : 0 ); - return SetMenuItemInfo_common(item, &mii, TRUE); + menu = grab_menu_ptr(hMenu); + item = MENU_InsertItem(menu, uItem, bypos ? MF_BYPOSITION : 0 ); + ret = SetMenuItemInfo_common(item, &mii, TRUE); + release_menu_ptr(menu); + return ret; }
/********************************************************************** @@ -5130,7 +5291,7 @@ BOOL WINAPI CheckMenuRadioItem(HMENU hMenu, BOOL done = FALSE; UINT i; MENUITEM *mi_first = NULL, *mi_check; - HMENU m_first, m_check; + POPUPMENU *m_first = NULL, *m_check = NULL;
for (i = first; i <= last; i++) { @@ -5138,17 +5299,26 @@ BOOL WINAPI CheckMenuRadioItem(HMENU hMenu,
if (!mi_first) { - m_first = hMenu; - mi_first = MENU_FindItem(&m_first, &pos, bypos); - if (!mi_first) continue; + m_first = grab_menu_ptr(hMenu); + if (!(mi_first = MENU_FindItem(&m_first, &pos, bypos))) + { + release_menu_ptr(m_first); + m_first = NULL; + continue; + } mi_check = mi_first; - m_check = m_first; + m_check = grab_menu_ptr(m_first->obj.handle); } else { - m_check = hMenu; - mi_check = MENU_FindItem(&m_check, &pos, bypos); - if (!mi_check) continue; + release_menu_ptr(m_check); + m_check = grab_menu_ptr(hMenu); + if (!(mi_check = MENU_FindItem(&m_check, &pos, bypos))) + { + release_menu_ptr(m_check); + m_check = NULL; + continue; + } }
if (m_first != m_check) continue; @@ -5166,6 +5336,8 @@ BOOL WINAPI CheckMenuRadioItem(HMENU hMenu, mi_check->fState &= ~MFS_CHECKED; } } + release_menu_ptr(m_first); + release_menu_ptr(m_check);
return done; } @@ -5186,21 +5358,30 @@ BOOL WINAPI GetMenuItemRect(HWND hwnd, HMENU hMenu, UINT uItem, RECT *rect)
TRACE("(%p,%p,%d,%p)\n", hwnd, hMenu, uItem, rect);
- item = MENU_FindItem (&hMenu, &uItem, MF_BYPOSITION); - if ((rect == NULL) || (item == NULL)) + if (!rect) return FALSE;
- menu = MENU_GetMenu(hMenu); - if (!menu) return FALSE; + menu = grab_menu_ptr(hMenu); + if (!(item = MENU_FindItem(&menu, &uItem, MF_BYPOSITION))) + { + release_menu_ptr(menu); + return FALSE; + }
if (!hwnd) hwnd = menu->hWnd; - if (!hwnd) return FALSE; + + if (!hwnd) + { + release_menu_ptr(menu); + return FALSE; + }
*rect = item->rect;
OffsetRect(rect, menu->items_rect.left, menu->items_rect.top); - MapWindowPoints(hwnd, 0, (POINT *)rect, 2); + release_menu_ptr(menu);
+ MapWindowPoints(hwnd, 0, (POINT *)rect, 2); return TRUE; }
@@ -5415,6 +5596,8 @@ static BOOL translate_accelerator( HWND hWnd, UINT message, WPARAM wParam, LPARA { HMENU hMenu, hSubMenu, hSysMenu; UINT uSysStat = (UINT)-1, uStat = (UINT)-1, nPos; + POPUPMENU *submenu; + BOOL res;
hMenu = (GetWindowLongW( hWnd, GWL_STYLE ) & WS_CHILD) ? 0 : GetMenu(hWnd); hSysMenu = get_win_sys_menu( hWnd ); @@ -5423,7 +5606,12 @@ static BOOL translate_accelerator( HWND hWnd, UINT message, WPARAM wParam, LPARA /* 1. in the system menu */ hSubMenu = hSysMenu; nPos = cmd; - if(MENU_FindItem(&hSubMenu, &nPos, MF_BYCOMMAND)) + submenu = grab_menu_ptr(hSubMenu); + res = MENU_FindItem(&submenu, &nPos, MF_BYCOMMAND) != NULL; + if (res) + hSubMenu = submenu->obj.handle; + release_menu_ptr(submenu); + if (res) { if (GetCapture()) mesg = 2; @@ -5445,7 +5633,13 @@ static BOOL translate_accelerator( HWND hWnd, UINT message, WPARAM wParam, LPARA { hSubMenu = hMenu; nPos = cmd; - if(MENU_FindItem(&hSubMenu, &nPos, MF_BYCOMMAND)) + + submenu = grab_menu_ptr(hSubMenu); + res = MENU_FindItem(&submenu, &nPos, MF_BYCOMMAND) != NULL; + if (res) + hSubMenu = submenu->obj.handle; + release_menu_ptr(submenu); + if (res) { if (GetCapture()) mesg = 2;
On Mon, Apr 23, 2018 at 12:31:21PM +0300, Nikolay Sivov wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/user32/menu.c | 544 ++++++++++++++++++++++++++++++--------------- 1 file changed, 369 insertions(+), 175 deletions(-)
diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c index 537c2ebe1a..dcec42cdf2 100644 --- a/dlls/user32/menu.c +++ b/dlls/user32/menu.c @@ -592,52 +592,56 @@ static UINT MENU_GetStartOfPrevColumn(
- Find a menu item. Return a pointer on the item, and modifies *hmenu
- in case the item was in a sub-menu.
*/ -static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) +static MENUITEM *MENU_FindItem(POPUPMENU **menu, UINT *nPos, UINT wFlags) {
Perhaps it would be better to introduce some new functions here - the [in, out] nature of the first two params is ugly.
Many callers don't need the menu or the position back, so how about:
MENUITEM *find_item( POPUPMENU *menu, UINT id, UINT flags )
for those cases. That could be written in terms of MENU_FindItem() (for now) and callers that only need the item could be moved first.
The rest could call something like:
MENUITEM *find_item_and_popup( POPUPMENU *menu, UINT id, UINT flags, POPUPMENU **popup, UINT *pos )
which on success would always return a properly ref counted menu in *popup (either the original or a submenu).
Huw.
On 04/24/2018 11:56 AM, Huw Davies wrote:
On Mon, Apr 23, 2018 at 12:31:21PM +0300, Nikolay Sivov wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/user32/menu.c | 544 ++++++++++++++++++++++++++++++--------------- 1 file changed, 369 insertions(+), 175 deletions(-)
diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c index 537c2ebe1a..dcec42cdf2 100644 --- a/dlls/user32/menu.c +++ b/dlls/user32/menu.c @@ -592,52 +592,56 @@ static UINT MENU_GetStartOfPrevColumn(
- Find a menu item. Return a pointer on the item, and modifies *hmenu
- in case the item was in a sub-menu.
*/ -static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) +static MENUITEM *MENU_FindItem(POPUPMENU **menu, UINT *nPos, UINT wFlags) {
Perhaps it would be better to introduce some new functions here - the [in, out] nature of the first two params is ugly.
Many callers don't need the menu or the position back, so how about:
MENUITEM *find_item( POPUPMENU *menu, UINT id, UINT flags )
for those cases. That could be written in terms of MENU_FindItem() (for now) and callers that only need the item could be moved first.
The issue is that it's recursive to traverse submenus. Not returning potentially new menu pointer limits such helper to MF_BYPOSITION case. Limited like that it will become more get_menu_item() than find_menu_item(). Another way, I didn't check how intrusive that would be, is to add back link from MENUITEM to its POPUPMENU. Doing so will make calling site more complex, because you'll have to release conditionally or have two release calls for 'menu' and MENUITEM->menu to cover both cases.
The rest could call something like:
MENUITEM *find_item_and_popup( POPUPMENU *menu, UINT id, UINT flags, POPUPMENU **popup, UINT *pos )
which on success would always return a properly ref counted menu in *popup (either the original or a submenu).
Huw.
Nikolay Sivov nsivov@codeweavers.com writes:
On 04/24/2018 11:56 AM, Huw Davies wrote:
On Mon, Apr 23, 2018 at 12:31:21PM +0300, Nikolay Sivov wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/user32/menu.c | 544 ++++++++++++++++++++++++++++++--------------- 1 file changed, 369 insertions(+), 175 deletions(-)
diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c index 537c2ebe1a..dcec42cdf2 100644 --- a/dlls/user32/menu.c +++ b/dlls/user32/menu.c @@ -592,52 +592,56 @@ static UINT MENU_GetStartOfPrevColumn(
- Find a menu item. Return a pointer on the item, and modifies *hmenu
- in case the item was in a sub-menu.
*/ -static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags ) +static MENUITEM *MENU_FindItem(POPUPMENU **menu, UINT *nPos, UINT wFlags) {
Perhaps it would be better to introduce some new functions here - the [in, out] nature of the first two params is ugly.
Many callers don't need the menu or the position back, so how about:
MENUITEM *find_item( POPUPMENU *menu, UINT id, UINT flags )
for those cases. That could be written in terms of MENU_FindItem() (for now) and callers that only need the item could be moved first.
The issue is that it's recursive to traverse submenus. Not returning potentially new menu pointer limits such helper to MF_BYPOSITION case. Limited like that it will become more get_menu_item() than find_menu_item(). Another way, I didn't check how intrusive that would be, is to add back link from MENUITEM to its POPUPMENU. Doing so will make calling site more complex, because you'll have to release conditionally or have two release calls for 'menu' and MENUITEM->menu to cover both cases.
I'm not sure we want a back link. You could potentially return a struct containing both MENUITEM and POPUPMENU. I also feel you'd want to keep a helper that takes an HMENU as input, to avoid adding complexity to most callers.
-
Alexandre Julliard -
Huw Davies -
Nikolay Sivov