[SHELL32] Improve CFSDropTarget::CopyItems logic (#5481)

The previous version resolved the path of the parent then did logic
assuming simple pidls. This now resolves each source directly.

This change addresses a longstanding TODO in copying and moving to shell folders.
It's a simple change, but it removes a bit of code and makes things simpler.

Corresponding Wine PR: https://gitlab.winehq.org/wine/wine/-/merge_requests/3360
This commit is contained in:
Huw Campbell 2023-08-05 20:12:36 +10:00 committed by GitHub
parent 9bc5b8357a
commit 1273bbe417
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -24,82 +24,65 @@
WINE_DEFAULT_DEBUG_CHANNEL (shell);
/****************************************************************************
* BuildPathsList
*
* Builds a list of paths like the one used in SHFileOperation from a table of
* PIDLs relative to the given base folder
*/
static WCHAR* BuildPathsList(LPCWSTR wszBasePath, int cidl, LPCITEMIDLIST *pidls)
{
WCHAR *pwszPathsList = (WCHAR *)HeapAlloc(GetProcessHeap(), 0, MAX_PATH * sizeof(WCHAR) * cidl + 1);
WCHAR *pwszListPos = pwszPathsList;
for (int i = 0; i < cidl; i++)
{
FileStructW* pDataW = _ILGetFileStructW(pidls[i]);
if (!pDataW)
{
ERR("Mistreating a pidl:\n");
pdump_always(pidls[i]);
continue;
}
PathCombineW(pwszListPos, wszBasePath, pDataW->wszName);
pwszListPos += wcslen(pwszListPos) + 1;
}
*pwszListPos = 0;
return pwszPathsList;
}
/****************************************************************************
* CFSDropTarget::_CopyItems
*
* copies items to this folder
* FIXME: We should not ask the parent folder: 'What is your path', and then manually build paths assuming everything is a simple pidl!
* We should be asking the parent folder: Give me a full name for this pidl (for each child!)
* copies or moves items to this folder
*/
HRESULT CFSDropTarget::_CopyItems(IShellFolder * pSFFrom, UINT cidl,
LPCITEMIDLIST * apidl, BOOL bCopy)
{
LPWSTR pszSrcList;
HRESULT hr;
WCHAR wszTargetPath[MAX_PATH + 1];
wcscpy(wszTargetPath, m_sPathTarget);
//Double NULL terminate.
wszTargetPath[wcslen(wszTargetPath) + 1] = '\0';
TRACE ("(%p)->(%p,%u,%p)\n", this, pSFFrom, cidl, apidl);
STRRET strretFrom;
hr = pSFFrom->GetDisplayNameOf(NULL, SHGDN_FORPARSING, &strretFrom);
if (hr != S_OK)
return hr;
pszSrcList = BuildPathsList(strretFrom.pOleStr, cidl, apidl);
TRACE("Source file (just the first) = %s, target path = %s, bCopy: %d\n", debugstr_w(pszSrcList), debugstr_w(m_sPathTarget), bCopy);
CoTaskMemFree(strretFrom.pOleStr);
if (!pszSrcList)
HRESULT ret;
WCHAR wszDstPath[MAX_PATH + 1] = {0};
PWCHAR pwszSrcPathsList = (PWCHAR) HeapAlloc(GetProcessHeap(), 0, MAX_PATH * sizeof(WCHAR) * cidl + 1);
if (!pwszSrcPathsList)
return E_OUTOFMEMORY;
SHFILEOPSTRUCTW op = {0};
op.pFrom = pszSrcList;
op.pTo = wszTargetPath;
op.hwnd = m_hwndSite;
op.wFunc = bCopy ? FO_COPY : FO_MOVE;
op.fFlags = FOF_ALLOWUNDO | FOF_NOCONFIRMMKDIR;
PWCHAR pwszListPos = pwszSrcPathsList;
STRRET strretFrom;
SHFILEOPSTRUCTW fop;
int res = SHFileOperationW(&op);
HeapFree(GetProcessHeap(), 0, pszSrcList);
if (res)
/* Build a double null terminated list of C strings from source paths */
for (UINT i = 0; i < cidl; i++)
{
ERR("SHFileOperationW failed with 0x%x\n", res);
return E_FAIL;
ret = pSFFrom->GetDisplayNameOf(apidl[i], SHGDN_FORPARSING, &strretFrom);
if (FAILED(ret))
goto cleanup;
ret = StrRetToBufW(&strretFrom, NULL, pwszListPos, MAX_PATH);
if (FAILED(ret))
goto cleanup;
pwszListPos += lstrlenW(pwszListPos) + 1;
}
return S_OK;
/* Append the final null. */
*pwszListPos = L'\0';
/* Build a double null terminated target (this path) */
ret = StringCchCopyW(wszDstPath, MAX_PATH, m_sPathTarget);
if (FAILED(ret))
goto cleanup;
wszDstPath[lstrlenW(wszDstPath) + 1] = L'\0';
ZeroMemory(&fop, sizeof(fop));
fop.hwnd = m_hwndSite;
fop.wFunc = bCopy ? FO_COPY : FO_MOVE;
fop.pFrom = pwszSrcPathsList;
fop.pTo = wszDstPath;
fop.fFlags = FOF_ALLOWUNDO | FOF_NOCONFIRMMKDIR;
ret = S_OK;
if (SHFileOperationW(&fop))
{
ERR("SHFileOperationW failed\n");
ret = E_FAIL;
}
cleanup:
HeapFree(GetProcessHeap(), 0, pwszSrcPathsList);
return ret;
}
CFSDropTarget::CFSDropTarget():