From 11ea1d61982eacc09d5fc60d2cd91c63adca7bbf Mon Sep 17 00:00:00 2001 From: Whindmar Saksit Date: Wed, 22 May 2024 00:06:46 +0200 Subject: [PATCH] [BROWSEUI][COMCTL32] Handle rename on other folders in Explorer bar tree (#6832) IContextMenu (CDefaultContextMenu) only knows how to rename when its site is an IShellView. The tree must detect the rename operation and perform it in the TreeView instead. Fixed a bug in the ListView that is triggered by activating DefView (to complete the rename): Just after DefView has been activated, the tree performs a navigation to the newly renamed folder, this causes DefView to be destroyed and in turn the ListView. The ListView tries to handle a selection change during destruction of the ListView and ends up accessing an invalid pointer. CORE-19557 --- dll/win32/browseui/explorerband.cpp | 173 ++++++++++++++++++++-------- dll/win32/browseui/explorerband.h | 6 +- dll/win32/browseui/shellbrowser.cpp | 10 +- dll/win32/browseui/utility.cpp | 57 +++++++++ dll/win32/browseui/utility.h | 4 + dll/win32/comctl32/listview.c | 16 ++- 6 files changed, 213 insertions(+), 53 deletions(-) diff --git a/dll/win32/browseui/explorerband.cpp b/dll/win32/browseui/explorerband.cpp index 833dfa0de96..f50bc08bb91 100644 --- a/dll/win32/browseui/explorerband.cpp +++ b/dll/win32/browseui/explorerband.cpp @@ -22,6 +22,7 @@ #include "precomp.h" #include #include +#include "utility.h" #if 1 #undef UNIMPLEMENTED @@ -147,7 +148,7 @@ Cleanup: CExplorerBand::CExplorerBand() : m_pSite(NULL) , m_fVisible(FALSE) - , m_bNavigating(FALSE) + , m_mtxBlockNavigate(0) , m_dwBandID(0) , m_isEditing(FALSE) , m_pidlCurrent(NULL) @@ -261,6 +262,49 @@ CExplorerBand::NodeInfo* CExplorerBand::GetNodeInfo(HTREEITEM hItem) return reinterpret_cast(tvItem.lParam); } +static HRESULT GetCurrentLocationFromView(IShellView &View, PIDLIST_ABSOLUTE &pidl) +{ + CComPtr pfv; + CComPtr psf; + HRESULT hr = View.QueryInterface(IID_PPV_ARG(IFolderView, &pfv)); + if (SUCCEEDED(hr) && SUCCEEDED(hr = pfv->GetFolder(IID_PPV_ARG(IShellFolder, &psf)))) + hr = SHELL_GetIDListFromObject(psf, &pidl); + return hr; +} + +HRESULT CExplorerBand::GetCurrentLocation(PIDLIST_ABSOLUTE &pidl) +{ + pidl = NULL; + CComPtr psb; + HRESULT hr = IUnknown_QueryService(m_pSite, SID_STopLevelBrowser, IID_PPV_ARG(IShellBrowser, &psb)); + if (FAILED_UNEXPECTEDLY(hr)) + return hr; + + CComPtr pbs; + if (SUCCEEDED(hr = psb->QueryInterface(IID_PPV_ARG(IBrowserService, &pbs)))) + if (SUCCEEDED(hr = pbs->GetPidl(&pidl)) && pidl) + return hr; + + CComPtr psv; + if (!FAILED_UNEXPECTEDLY(hr = psb->QueryActiveShellView(&psv))) + if (SUCCEEDED(hr = psv.p ? GetCurrentLocationFromView(*psv.p, pidl) : E_FAIL)) + return hr; + return hr; +} + +HRESULT CExplorerBand::IsCurrentLocation(PCIDLIST_ABSOLUTE pidl) +{ + if (!pidl) + return E_INVALIDARG; + HRESULT hr = E_FAIL; + PIDLIST_ABSOLUTE location = m_pidlCurrent; + if (location || SUCCEEDED(hr = GetCurrentLocation(location))) + hr = SHELL_IsEqualAbsoluteID(location, pidl) ? S_OK : S_FALSE; + if (location != m_pidlCurrent) + ILFree(location); + return hr; +} + HRESULT CExplorerBand::ExecuteCommand(CComPtr& menu, UINT nCmd) { CComPtr pBrowserOleWnd; @@ -301,12 +345,8 @@ HRESULT CExplorerBand::UpdateBrowser(LPITEMIDLIST pidlGoto) if (FAILED_UNEXPECTEDLY(hr)) return hr; - if (m_pidlCurrent) - { - ILFree(m_pidlCurrent); - m_pidlCurrent = ILClone(pidlGoto); - } - return hr; + ILFree(m_pidlCurrent); + return SHILClone(pidlGoto, &m_pidlCurrent); } // *** notifications handling *** @@ -362,7 +402,7 @@ void CExplorerBand::OnSelectionChanged(LPNMTREEVIEW pnmtv) NodeInfo* pNodeInfo = GetNodeInfo(pnmtv->itemNew.hItem); /* Prevents navigation if selection is initiated inside the band */ - if (m_bNavigating) + if (m_mtxBlockNavigate) return; UpdateBrowser(pNodeInfo->absolutePidl); @@ -411,13 +451,16 @@ LRESULT CExplorerBand::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BO HTREEITEM item; NodeInfo *info; HMENU treeMenu; - WORD x; - WORD y; + POINT pt; CComPtr pFolder; CComPtr contextMenu; HRESULT hr; UINT uCommand; LPITEMIDLIST pidlChild; + UINT cmdBase = max(FCIDM_SHVIEWFIRST, 1); + UINT cmf = CMF_EXPLORE; + SFGAOF attr = SFGAO_CANRENAME; + BOOL startedRename = FALSE; treeMenu = NULL; item = TreeView_GetSelection(m_hWnd); @@ -427,11 +470,17 @@ LRESULT CExplorerBand::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BO goto Cleanup; } - x = LOWORD(lParam); - y = HIWORD(lParam); - if (x == -1 && y == -1) + pt.x = LOWORD(lParam); + pt.y = HIWORD(lParam); + if ((UINT)lParam == (UINT)-1) { - // TODO: grab position of tree item and position it correctly + RECT r; + if (TreeView_GetItemRect(m_hWnd, item, &r, TRUE)) + { + pt.x = (r.left + r.right) / 2; // Center of + pt.y = (r.top + r.bottom) / 2; // item rectangle + } + ClientToScreen(&pt); } info = GetNodeInfo(item); @@ -457,28 +506,54 @@ LRESULT CExplorerBand::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BO IUnknown_SetSite(contextMenu, (IDeskBand *)this); + if (SUCCEEDED(pFolder->GetAttributesOf(1, (LPCITEMIDLIST*)&pidlChild, &attr)) && (attr & SFGAO_CANRENAME)) + cmf |= CMF_CANRENAME; + treeMenu = CreatePopupMenu(); - hr = contextMenu->QueryContextMenu(treeMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, - CMF_EXPLORE); + hr = contextMenu->QueryContextMenu(treeMenu, 0, cmdBase, FCIDM_SHVIEWLAST, cmf); if (!SUCCEEDED(hr)) { WARN("Can't get context menu for item\n"); DestroyMenu(treeMenu); goto Cleanup; } - uCommand = TrackPopupMenu(treeMenu, TPM_LEFTALIGN | TPM_RETURNCMD | TPM_LEFTBUTTON | TPM_RIGHTBUTTON, - x, y, 0, m_hWnd, NULL); - ExecuteCommand(contextMenu, uCommand); + uCommand = TrackPopupMenu(treeMenu, TPM_LEFTALIGN | TPM_RETURNCMD | TPM_LEFTBUTTON | TPM_RIGHTBUTTON, + pt.x, pt.y, 0, m_hWnd, NULL); + if (uCommand) + { + uCommand -= cmdBase; + + // Do DFM_CMD_RENAME in the treeview + if ((cmf & CMF_CANRENAME) && SHELL_IsVerb(contextMenu, uCommand, L"rename")) + { + HTREEITEM oldSelected = m_oldSelected; + SetFocus(); + startedRename = TreeView_EditLabel(m_hWnd, item) != NULL; + m_oldSelected = oldSelected; // Restore after TVN_BEGINLABELEDIT + goto Cleanup; + } + + hr = ExecuteCommand(contextMenu, uCommand); + } Cleanup: if (contextMenu) IUnknown_SetSite(contextMenu, NULL); if (treeMenu) DestroyMenu(treeMenu); - m_bNavigating = TRUE; - TreeView_SelectItem(m_hWnd, m_oldSelected); - m_bNavigating = FALSE; + if (startedRename) + { + // The treeview disables drawing of the edited item so we must make sure + // the correct item is selected (on right-click -> rename on not-current folder). + // TVN_ENDLABELEDIT becomes responsible for restoring the selection. + } + else + { + ++m_mtxBlockNavigate; + TreeView_SelectItem(m_hWnd, m_oldSelected); + --m_mtxBlockNavigate; + } return TRUE; } @@ -498,9 +573,9 @@ LRESULT CExplorerBand::ContextMenuHack(UINT uMsg, WPARAM wParam, LPARAM lParam, // Move to the item selected by the treeview (don't change right pane) TreeView_HitTest(m_hWnd, &info); - m_bNavigating = TRUE; + ++m_mtxBlockNavigate; TreeView_SelectItem(m_hWnd, info.hItem); - m_bNavigating = FALSE; + --m_mtxBlockNavigate; } return FALSE; /* let the wndproc process the message */ } @@ -825,29 +900,20 @@ BOOL CExplorerBand::NavigateToPIDL(LPITEMIDLIST dest, HTREEITEM *item, BOOL bExp BOOL CExplorerBand::NavigateToCurrentFolder() { LPITEMIDLIST explorerPidl; - CComPtr pBrowserService; - HRESULT hr; HTREEITEM dummy; BOOL result; - explorerPidl = NULL; - hr = IUnknown_QueryService(m_pSite, SID_STopLevelBrowser, IID_PPV_ARG(IBrowserService, &pBrowserService)); - if (!SUCCEEDED(hr)) - { - ERR("Can't get IBrowserService !\n"); - return FALSE; - } - - hr = pBrowserService->GetPidl(&explorerPidl); - if (!SUCCEEDED(hr) || !explorerPidl) + HRESULT hr = GetCurrentLocation(explorerPidl); + if (FAILED_UNEXPECTEDLY(hr)) { ERR("Unable to get browser PIDL !\n"); return FALSE; } - m_bNavigating = TRUE; + ++m_mtxBlockNavigate; /* find PIDL into our explorer */ result = NavigateToPIDL(explorerPidl, &dummy, TRUE, FALSE, TRUE); - m_bNavigating = FALSE; + --m_mtxBlockNavigate; + ILFree(explorerPidl); return result; } @@ -1330,6 +1396,7 @@ HRESULT STDMETHODCALLTYPE CExplorerBand::OnWinEvent(HWND hWnd, UINT uMsg, WPARAM if (theResult) *theResult = 0; m_isEditing = TRUE; + m_oldSelected = NULL; } return S_OK; } @@ -1340,6 +1407,13 @@ HRESULT STDMETHODCALLTYPE CExplorerBand::OnWinEvent(HWND hWnd, UINT uMsg, WPARAM HRESULT hr; m_isEditing = FALSE; + if (m_oldSelected) + { + ++m_mtxBlockNavigate; + TreeView_SelectItem(m_hWnd, m_oldSelected); + --m_mtxBlockNavigate; + } + if (theResult) *theResult = 0; if (dispInfo->item.pszText) @@ -1347,12 +1421,13 @@ HRESULT STDMETHODCALLTYPE CExplorerBand::OnWinEvent(HWND hWnd, UINT uMsg, WPARAM LPITEMIDLIST pidlNew; CComPtr pParent; LPCITEMIDLIST pidlChild; + BOOL RenamedCurrent = IsCurrentLocation(info->absolutePidl) == S_OK; hr = SHBindToParent(info->absolutePidl, IID_PPV_ARG(IShellFolder, &pParent), &pidlChild); if (!SUCCEEDED(hr) || !pParent.p) return E_FAIL; - hr = pParent->SetNameOf(0, pidlChild, dispInfo->item.pszText, SHGDN_INFOLDER, &pidlNew); + hr = pParent->SetNameOf(m_hWnd, pidlChild, dispInfo->item.pszText, SHGDN_INFOLDER, &pidlNew); if(SUCCEEDED(hr) && pidlNew) { CComPtr pPersist; @@ -1367,8 +1442,16 @@ HRESULT STDMETHODCALLTYPE CExplorerBand::OnWinEvent(HWND hWnd, UINT uMsg, WPARAM return E_FAIL; pidlNewAbs = ILCombine(pidlParent, pidlNew); - // Navigate to our new location - UpdateBrowser(pidlNewAbs); + if (RenamedCurrent) + { + // Navigate to our new location + UpdateBrowser(pidlNewAbs); + } + else + { + // Tell everyone in case SetNameOf forgot, this causes IShellView to update itself when we renamed a child + SHChangeNotify(SHCNE_RENAMEFOLDER, SHCNF_IDLIST, info->absolutePidl, pidlNewAbs); + } ILFree(pidlParent); ILFree(pidlNewAbs); @@ -1490,9 +1573,9 @@ HRESULT STDMETHODCALLTYPE CExplorerBand::DragOver(DWORD glfKeyState, POINTL pt, if (info.hItem) { - m_bNavigating = TRUE; + ++m_mtxBlockNavigate; TreeView_SelectItem(m_hWnd, info.hItem); - m_bNavigating = FALSE; + --m_mtxBlockNavigate; // Delegate to shell folder if (m_pDropTarget && info.hItem != m_childTargetNode) { @@ -1551,9 +1634,9 @@ HRESULT STDMETHODCALLTYPE CExplorerBand::DragOver(DWORD glfKeyState, POINTL pt, HRESULT STDMETHODCALLTYPE CExplorerBand::DragLeave() { - m_bNavigating = TRUE; + ++m_mtxBlockNavigate; TreeView_SelectItem(m_hWnd, m_oldSelected); - m_bNavigating = FALSE; + --m_mtxBlockNavigate; m_childTargetNode = NULL; if (m_pCurObject) { diff --git a/dll/win32/browseui/explorerband.h b/dll/win32/browseui/explorerband.h index ad4e6aa3830..0cdf6696613 100644 --- a/dll/win32/browseui/explorerband.h +++ b/dll/win32/browseui/explorerband.h @@ -57,14 +57,14 @@ private: // *** tree explorer band stuff *** BOOL m_fVisible; - BOOL m_bNavigating; + BYTE m_mtxBlockNavigate; // A "lock" that prevents internal selection changes to initiate a navigation to the newly selected item. BOOL m_bFocused; DWORD m_dwBandID; BOOL m_isEditing; HIMAGELIST m_hImageList; HTREEITEM m_hRoot; HTREEITEM m_oldSelected; - LPITEMIDLIST m_pidlCurrent; + LPITEMIDLIST m_pidlCurrent; // Note: This is NULL until the first user navigation! // *** notification cookies *** DWORD m_adviseCookie; @@ -103,6 +103,8 @@ private: BOOL RenameItem(HTREEITEM toRename, LPITEMIDLIST newPidl); BOOL RefreshTreePidl(HTREEITEM tree, LPITEMIDLIST pidlParent); BOOL NavigateToCurrentFolder(); + HRESULT GetCurrentLocation(PIDLIST_ABSOLUTE &pidl); + HRESULT IsCurrentLocation(PCIDLIST_ABSOLUTE pidl); // *** Tree item sorting callback *** static int CALLBACK CompareTreeItems(LPARAM p1, LPARAM p2, LPARAM p3); diff --git a/dll/win32/browseui/shellbrowser.cpp b/dll/win32/browseui/shellbrowser.cpp index 42303f64d7b..4b26884f756 100644 --- a/dll/win32/browseui/shellbrowser.cpp +++ b/dll/win32/browseui/shellbrowser.cpp @@ -2312,8 +2312,11 @@ HRESULT STDMETHODCALLTYPE CShellBrowser::QueryActiveShellView(IShellView **ppshv return E_POINTER; *ppshv = fCurrentShellView; if (fCurrentShellView.p != NULL) + { fCurrentShellView.p->AddRef(); - return S_OK; + return S_OK; + } + return E_FAIL; } HRESULT STDMETHODCALLTYPE CShellBrowser::OnViewWindowActive(IShellView *ppshv) @@ -2568,10 +2571,7 @@ HRESULT STDMETHODCALLTYPE CShellBrowser::CanNavigateNow() HRESULT STDMETHODCALLTYPE CShellBrowser::GetPidl(LPITEMIDLIST *ppidl) { // called by explorer bar to get current pidl - if (ppidl == NULL) - return E_POINTER; - *ppidl = ILClone(fCurrentDirectoryPIDL); - return S_OK; + return ppidl ? SHILClone(fCurrentDirectoryPIDL, ppidl) : E_POINTER; } HRESULT STDMETHODCALLTYPE CShellBrowser::SetReferrer(LPCITEMIDLIST pidl) diff --git a/dll/win32/browseui/utility.cpp b/dll/win32/browseui/utility.cpp index 17a2ab42968..ead564a1f6e 100644 --- a/dll/win32/browseui/utility.cpp +++ b/dll/win32/browseui/utility.cpp @@ -1,4 +1,7 @@ #include "precomp.h" +#ifndef SHCIDS_CANONICALONLY +#define SHCIDS_CANONICALONLY 0x10000000L +#endif void *operator new(size_t size) { @@ -14,3 +17,57 @@ void operator delete(void *p, UINT_PTR) { LocalFree(p); } + +HRESULT SHELL_GetIDListFromObject(IUnknown *punk, PIDLIST_ABSOLUTE *ppidl) +{ +#if DLL_EXPORT_VERSION >= _WIN32_WINNT_VISTA && 0 // FIXME: SHELL32 not ready yet + return SHGetIDListFromObject(punk, ppidl); +#else + HRESULT hr; + IPersistFolder2 *pf2; + if (SUCCEEDED(hr = punk->QueryInterface(IID_PPV_ARG(IPersistFolder2, &pf2)))) + { + hr = pf2->GetCurFolder(ppidl); + pf2->Release(); + } + IPersistIDList *pil; + if (FAILED(hr) && SUCCEEDED(hr = punk->QueryInterface(IID_PPV_ARG(IPersistIDList, &pil)))) + { + hr = pil->GetIDList(ppidl); + pil->Release(); + } + return hr; +#endif +} + +static HRESULT SHELL_CompareAbsoluteIDs(LPARAM lParam, PCIDLIST_ABSOLUTE a, PCIDLIST_ABSOLUTE b) +{ + IShellFolder *psf; + HRESULT hr = SHGetDesktopFolder(&psf); + if (FAILED(hr)) + return hr; + hr = psf->CompareIDs(lParam, a, b); + psf->Release(); + return hr; +} + +BOOL SHELL_IsEqualAbsoluteID(PCIDLIST_ABSOLUTE a, PCIDLIST_ABSOLUTE b) +{ + return !SHELL_CompareAbsoluteIDs(SHCIDS_CANONICALONLY, a, b); +} + +BOOL SHELL_IsVerb(IContextMenu *pcm, UINT_PTR idCmd, LPCWSTR Verb) +{ + HRESULT hr; + WCHAR wide[MAX_PATH]; + if (SUCCEEDED(hr = pcm->GetCommandString(idCmd, GCS_VERBW, NULL, (LPSTR)wide, _countof(wide)))) + return !lstrcmpiW(wide, Verb); + + CHAR ansi[_countof(wide)], buf[MAX_PATH]; + if (SHUnicodeToAnsi(Verb, buf, _countof(buf))) + { + if (SUCCEEDED(hr = pcm->GetCommandString(idCmd, GCS_VERBA, NULL, ansi, _countof(ansi)))) + return !lstrcmpiA(ansi, buf); + } + return FALSE; +} diff --git a/dll/win32/browseui/utility.h b/dll/win32/browseui/utility.h index 948b9abf03b..b40d18c34a4 100644 --- a/dll/win32/browseui/utility.h +++ b/dll/win32/browseui/utility.h @@ -2,3 +2,7 @@ void *operator new(size_t size); void operator delete(void *p); + +HRESULT SHELL_GetIDListFromObject(IUnknown *punk, PIDLIST_ABSOLUTE *ppidl); +BOOL SHELL_IsEqualAbsoluteID(PCIDLIST_ABSOLUTE a, PCIDLIST_ABSOLUTE b); +BOOL SHELL_IsVerb(IContextMenu *pcm, UINT_PTR idCmd, LPCWSTR Verb); diff --git a/dll/win32/comctl32/listview.c b/dll/win32/comctl32/listview.c index f53bf9b7ff8..53e8b546bf2 100644 --- a/dll/win32/comctl32/listview.c +++ b/dll/win32/comctl32/listview.c @@ -3254,7 +3254,18 @@ static RANGES ranges_clone(RANGES ranges) { RANGES clone; INT i; - + +#ifdef __REACTOS__ + if (!ranges || !ranges->hdpa) + { + /* + * If a ExplorerBand tree rename operation is completed by left-clicking in + * DefView, the navigation to the newly named item causes the ListView in DefView + * to call LISTVIEW_DeselectAllSkipItems during ListView destruction. + */ + return NULL; + } +#endif if (!(clone = ranges_create(DPA_GetPtrCount(ranges->hdpa)))) goto fail; for (i = 0; i < DPA_GetPtrCount(ranges->hdpa); i++) @@ -10575,6 +10586,9 @@ static LRESULT LISTVIEW_NCDestroy(LISTVIEW_INFO *infoPtr) Free(DPA_GetPtr(infoPtr->hdpaColumns, i)); DPA_Destroy(infoPtr->hdpaColumns); ranges_destroy(infoPtr->selectionRanges); +#ifdef __REACTOS__ + infoPtr->selectionRanges = NULL; /* See note in ranges_clone */ +#endif /* destroy image lists */ if (!(infoPtr->dwStyle & LVS_SHAREIMAGELISTS))