diff --git a/dll/win32/shell32/CDefView.cpp b/dll/win32/shell32/CDefView.cpp index 98b597c59d4..0bff83e1c9f 100644 --- a/dll/win32/shell32/CDefView.cpp +++ b/dll/win32/shell32/CDefView.cpp @@ -58,7 +58,7 @@ typedef struct static void ClientToListView(HWND hwndLV, POINT *ppt) { - POINT Origin; + POINT Origin = {}; /* FIXME: LVM_GETORIGIN is broken. See CORE-17266 */ if (!ListView_GetOrigin(hwndLV, &Origin)) @@ -68,6 +68,33 @@ ClientToListView(HWND hwndLV, POINT *ppt) ppt->y += Origin.y; } +// Helper struct to automatically cleanup the IContextMenu +// We want to explicitly reset the Site, so there are no circular references +struct MenuCleanup +{ + CComPtr &m_pCM; + HMENU &m_hMenu; + + MenuCleanup(CComPtr &pCM, HMENU& menu) + : m_pCM(pCM), m_hMenu(menu) + { + } + ~MenuCleanup() + { + if (m_hMenu) + { + DestroyMenu(m_hMenu); + m_hMenu = NULL; + } + if (m_pCM) + { + IUnknown_SetSite(m_pCM, NULL); + m_pCM.Release(); + } + } +}; + + class CDefView : public CWindowImpl, public CComObjectRootEx, @@ -116,6 +143,7 @@ class CDefView : DWORD m_grfKeyState; // CComPtr m_pCM; + CComPtr m_pFileMenu; BOOL m_isEditing; BOOL m_isParentFolderSpecial; @@ -169,7 +197,7 @@ class CDefView : void OnDeactivate(); void DoActivate(UINT uState); HRESULT drag_notify_subitem(DWORD grfKeyState, POINTL pt, DWORD *pdwEffect); - HRESULT InvokeContextMenuCommand(UINT uCommand); + HRESULT InvokeContextMenuCommand(CComPtr &pCM, UINT uCommand); LRESULT OnExplorerCommand(UINT uCommand, BOOL bUseSelection); // *** IOleWindow methods *** @@ -1310,13 +1338,6 @@ HRESULT CDefView::FillFileMenu() if (!hFileMenu) return E_FAIL; - /* Release cached IContextMenu */ - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } - /* Cleanup the items added previously */ for (int i = GetMenuItemCount(hFileMenu) - 1; i >= 0; i--) { @@ -1327,15 +1348,20 @@ HRESULT CDefView::FillFileMenu() m_cidl = m_ListView.GetSelectedCount(); - /* Store the context menu in m_pCM and keep it in order to invoke the selected command later on */ - HRESULT hr = GetItemObject((m_cidl ? SVGIO_SELECTION : SVGIO_BACKGROUND), - IID_PPV_ARG(IContextMenu, &m_pCM)); + /* In case we still have this left over, clean it up! */ + if (m_pFileMenu) + { + IUnknown_SetSite(m_pFileMenu, NULL); + m_pFileMenu.Release(); + } + /* Store the context menu in m_pFileMenu and keep it in order to invoke the selected command later on */ + HRESULT hr = GetItemObject((m_cidl ? SVGIO_SELECTION : SVGIO_BACKGROUND), IID_PPV_ARG(IContextMenu, &m_pFileMenu)); if (FAILED_UNEXPECTEDLY(hr)) return hr; HMENU hmenu = CreatePopupMenu(); - hr = m_pCM->QueryContextMenu(hmenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, 0); + hr = m_pFileMenu->QueryContextMenu(hmenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, 0); if (FAILED_UNEXPECTEDLY(hr)) return hr; @@ -1470,7 +1496,7 @@ UINT CDefView::GetSelections() return m_cidl; } -HRESULT CDefView::InvokeContextMenuCommand(UINT uCommand) +HRESULT CDefView::InvokeContextMenuCommand(CComPtr &pCM, UINT uCommand) { CMINVOKECOMMANDINFO cmi; @@ -1485,7 +1511,11 @@ HRESULT CDefView::InvokeContextMenuCommand(UINT uCommand) if (GetKeyState(VK_CONTROL) & 0x8000) cmi.fMask |= CMIC_MASK_CONTROL_DOWN; - HRESULT hr = m_pCM->InvokeCommand(&cmi); + HRESULT hr = pCM->InvokeCommand(&cmi); + // Most of our callers will do this, but in case they don't do that (File menu!) + IUnknown_SetSite(pCM, NULL); + pCM.Release(); + if (FAILED_UNEXPECTEDLY(hr)) return hr; @@ -1513,33 +1543,24 @@ HRESULT CDefView::OpenSelectedItems() if (!hMenu) return E_FAIL; - hResult = GetItemObject(SVGIO_SELECTION, IID_PPV_ARG(IContextMenu, &m_pCM)); + CComPtr pCM; + hResult = GetItemObject(SVGIO_SELECTION, IID_PPV_ARG(IContextMenu, &pCM)); + MenuCleanup _(pCM, hMenu); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return hResult; - hResult = m_pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_DEFAULTONLY); + hResult = pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_DEFAULTONLY); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return hResult; uCommand = GetMenuDefaultItem(hMenu, FALSE, 0); if (uCommand == (UINT)-1) { - hResult = E_FAIL; - goto cleanup; + ERR("GetMenuDefaultItem returned -1\n"); + return E_FAIL; } - InvokeContextMenuCommand(uCommand); - -cleanup: - - if (hMenu) - DestroyMenu(hMenu); - - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } + InvokeContextMenuCommand(pCM, uCommand); return hResult; } @@ -1555,6 +1576,12 @@ LRESULT CDefView::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &b TRACE("(%p)\n", this); + if (m_hContextMenu != NULL) + { + ERR("HACK: Aborting context menu in nested call!\n"); + return 0; + } + m_hContextMenu = CreatePopupMenu(); if (!m_hContextMenu) return E_FAIL; @@ -1578,15 +1605,16 @@ LRESULT CDefView::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &b } m_cidl = m_ListView.GetSelectedCount(); - + /* In case we still have this left over, clean it up! */ hResult = GetItemObject( m_cidl ? SVGIO_SELECTION : SVGIO_BACKGROUND, IID_PPV_ARG(IContextMenu, &m_pCM)); + MenuCleanup _(m_pCM, m_hContextMenu); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return 0; /* Use 1 as the first id as we want 0 the mean that the user canceled the menu */ hResult = m_pCM->QueryContextMenu(m_hContextMenu, 0, CONTEXT_MENU_BASE_ID, FCIDM_SHVIEWLAST, CMF_NORMAL); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return 0; /* There is no position requested, so try to find one */ if (lParam == ~0) @@ -1624,29 +1652,17 @@ LRESULT CDefView::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &b y = pt.y; } + // This runs the message loop, calling back to us with f.e. WM_INITPOPUP (hence why m_hContextMenu and m_pCM exist) uCommand = TrackPopupMenu(m_hContextMenu, TPM_LEFTALIGN | TPM_RETURNCMD | TPM_LEFTBUTTON | TPM_RIGHTBUTTON, x, y, 0, m_hWnd, NULL); if (uCommand == 0) - goto cleanup; + return 0; if (uCommand == FCIDM_SHVIEW_OPEN && OnDefaultCommand() == S_OK) - goto cleanup; + return 0; - InvokeContextMenuCommand(uCommand - CONTEXT_MENU_BASE_ID); - -cleanup: - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } - - if (m_hContextMenu) - { - DestroyMenu(m_hContextMenu); - m_hContextMenu = NULL; - } + InvokeContextMenuCommand(m_pCM, uCommand - CONTEXT_MENU_BASE_ID); return 0; } @@ -1656,18 +1672,22 @@ LRESULT CDefView::OnExplorerCommand(UINT uCommand, BOOL bUseSelection) HRESULT hResult; HMENU hMenu = NULL; - hResult = GetItemObject( bUseSelection ? SVGIO_SELECTION : SVGIO_BACKGROUND, IID_PPV_ARG(IContextMenu, &m_pCM)); - if (FAILED_UNEXPECTEDLY( hResult)) - goto cleanup; + CComPtr pCM; + hResult = GetItemObject( bUseSelection ? SVGIO_SELECTION : SVGIO_BACKGROUND, IID_PPV_ARG(IContextMenu, &pCM)); + if (FAILED_UNEXPECTEDLY(hResult)) + return 0; + + MenuCleanup _(pCM, hMenu); + if ((uCommand != FCIDM_SHVIEW_DELETE) && (uCommand != FCIDM_SHVIEW_RENAME)) { hMenu = CreatePopupMenu(); if (!hMenu) return 0; - hResult = m_pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_NORMAL); + hResult = pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_NORMAL); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return 0; } if (bUseSelection) @@ -1690,18 +1710,7 @@ LRESULT CDefView::OnExplorerCommand(UINT uCommand, BOOL bUseSelection) return 0; } - InvokeContextMenuCommand(uCommand); - -cleanup: - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } - - if (hMenu) - DestroyMenu(hMenu); - + InvokeContextMenuCommand(pCM, uCommand); return 0; } @@ -1936,10 +1945,12 @@ LRESULT CDefView::OnCommand(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &bHand case FCIDM_SHVIEW_NEWFOLDER: return OnExplorerCommand(dwCmdID, FALSE); default: - /* WM_COMMAND messages from the file menu are routed to the CDefView so as to let m_pCM handle the command */ - if (m_pCM && dwCmd == 0) + /* WM_COMMAND messages from the file menu are routed to the CDefView so as to let m_pFileMenu handle the command */ + if (m_pFileMenu && dwCmd == 0) { - InvokeContextMenuCommand(dwCmdID); + HMENU Dummy = NULL; + MenuCleanup _(m_pFileMenu, Dummy); + InvokeContextMenuCommand(m_pFileMenu, dwCmdID); } } @@ -2370,10 +2381,10 @@ HRESULT SHSetMenuIdInMenuMsg(UINT uMsg, LPARAM lParam, UINT CmdId); */ LRESULT CDefView::OnCustomItem(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &bHandled) { - if (!m_pCM.p) + if (!m_pCM) { /* no menu */ - ERR("no menu!!!\n"); + ERR("no context menu!!!\n"); return FALSE; } @@ -2409,7 +2420,10 @@ LRESULT CDefView::OnInitMenuPopup(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL int nPos = LOWORD(lParam); UINT menuItemId; - OnCustomItem(uMsg, wParam, lParam, bHandled); + if (m_pCM) + { + OnCustomItem(uMsg, wParam, lParam, bHandled); + } HMENU hViewMenu = GetSubmenuByID(m_hMenu, FCIDM_MENU_VIEW);