[PATCH 0/4] MR5252: shell32: Properly implement drop effect for cut/copy/paste.
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/shell32/shlview_cmenu.c | 172 +++++++++++++++++------------------ 1 file changed, 86 insertions(+), 86 deletions(-) diff --git a/dlls/shell32/shlview_cmenu.c b/dlls/shell32/shlview_cmenu.c index cb7ce331897..fa29ac2dc02 100644 --- a/dlls/shell32/shlview_cmenu.c +++ b/dlls/shell32/shlview_cmenu.c @@ -345,60 +345,65 @@ static void DoCopyOrCut(ContextMenu *This, HWND hwnd, BOOL cut) } } -static HRESULT paste_pidls(IShellFolder *dst_folder, - const ITEMIDLIST *src_parent, ITEMIDLIST **pidls, unsigned int count) +static HRESULT get_data_format(IDataObject *data, UINT cf, STGMEDIUM *medium) { - IShellFolder *src_folder; - HRESULT hr = S_OK; + FORMATETC format; + + InitFormatEtc(format, cf, TYMED_HGLOBAL); + return IDataObject_GetData(data, &format, medium); +} + +static WCHAR *build_source_paths(ITEMIDLIST *root_pidl, ITEMIDLIST **pidls, unsigned int count) +{ + WCHAR root_path[MAX_PATH], pidl_path[MAX_PATH]; + size_t size = 1, pos = 0, root_len; + WCHAR *paths; - if (FAILED(hr = SHBindToObject(NULL, src_parent, NULL, &IID_IShellFolder, (void **)&src_folder))) + if (!SHGetPathFromIDListW(root_pidl, root_path)) { - ERR("Failed to get folder from source PIDL, hr %#lx.\n", hr); - return hr; + ERR("Failed to get source root path.\n"); + return NULL; } + root_len = wcslen(root_path); - for (unsigned int i = 0; SUCCEEDED(hr) && i < count; i++) + for (unsigned int i = 0; i < count; ++i) { - ISFHelper *psfhlpdst = NULL, *psfhlpsrc = NULL; + if (!_ILIsValue(pidls[i]) && !_ILIsFolder(pidls[i])) + ERR("Unexpected child pidl type.\n"); - hr = IShellFolder_QueryInterface(dst_folder, &IID_ISFHelper, (void **)&psfhlpdst); - if (SUCCEEDED(hr)) - hr = IShellFolder_QueryInterface(src_folder, &IID_ISFHelper, (void **)&psfhlpsrc); - - if (psfhlpdst && psfhlpsrc) - { - hr = ISFHelper_CopyItems(psfhlpdst, src_folder, 1, (LPCITEMIDLIST *)&pidls[i]); - /* FIXME handle move - ISFHelper_DeleteItems(psfhlpsrc, 1, &pidl_item); - */ - } - if (psfhlpdst) - ISFHelper_Release(psfhlpdst); - if (psfhlpsrc) - ISFHelper_Release(psfhlpsrc); + _ILSimpleGetTextW(pidls[i], pidl_path, ARRAY_SIZE(pidl_path)); + size += root_len + 1 + wcslen(pidl_path) + 1; } - IShellFolder_Release(src_folder); - return hr; -} + paths = malloc(size * sizeof(WCHAR)); -static HRESULT get_data_format(IDataObject *data, UINT cf, STGMEDIUM *medium) -{ - FORMATETC format; + for (unsigned int i = 0; i < count; ++i) + { + if (!_ILIsValue(pidls[i]) && !_ILIsFolder(pidls[i])) + ERR("Unexpected child pidl type.\n"); + + memcpy(paths + pos, root_path, root_len * sizeof(WCHAR)); + pos += root_len; + paths[pos++] = '\\'; + _ILSimpleGetTextW(pidls[i], paths + pos, size - pos); + pos += wcslen(paths + pos) + 1; + } + paths[pos++] = 0; - InitFormatEtc(format, cf, TYMED_HGLOBAL); - return IDataObject_GetData(data, &format, medium); + return paths; } static HRESULT do_paste(ContextMenu *menu, HWND hwnd) { + IPersistFolder2 *dst_persist; IShellFolder *dst_folder; + WCHAR dst_path[MAX_PATH]; + SHFILEOPSTRUCTW op = {0}; + ITEMIDLIST *dst_pidl; IDataObject *data; HRESULT hr; STGMEDIUM medium; - - if (FAILED(hr = OleGetClipboard(&data))) - return hr; + int ret; if (menu->cidl) { @@ -415,71 +420,66 @@ static HRESULT do_paste(ContextMenu *menu, HWND hwnd) IShellFolder_AddRef(dst_folder); } + if (FAILED(hr = IShellFolder_QueryInterface(dst_folder, &IID_IPersistFolder2, (void **)&dst_persist))) + { + WARN("Failed to get IPersistFolder2, hr %#lx.\n", hr); + return hr; + } + + hr = IPersistFolder2_GetCurFolder(dst_persist, &dst_pidl); + IPersistFolder2_Release(dst_persist); + if (FAILED(hr)) + { + ERR("Failed to get dst folder pidl, hr %#lx.\n", hr); + return hr; + } + + if (!SHGetPathFromIDListW(dst_pidl, dst_path)) + { + ERR("Failed to get path, hr %#lx.\n", hr); + ILFree(dst_pidl); + return E_FAIL; + } + ILFree(dst_pidl); + + op.hwnd = hwnd; + op.wFunc = FO_COPY; + op.pTo = dst_path; + op.fFlags = FOF_ALLOWUNDO; + + if (FAILED(hr = OleGetClipboard(&data))) + return hr; + if (SUCCEEDED(get_data_format(data, RegisterClipboardFormatW(CFSTR_SHELLIDLISTW), &medium))) { - CIDA *cida = GlobalLock(medium.hGlobal); - ITEMIDLIST **pidls; - ITEMIDLIST *pidl; + const CIDA *cida = GlobalLock(medium.hGlobal); + ITEMIDLIST **pidls, *root_pidl; + WCHAR *src_paths; + + pidls = _ILCopyCidaToaPidl(&root_pidl, cida); - if (cida) + if ((src_paths = build_source_paths(root_pidl, pidls, cida->cidl))) { - pidls = _ILCopyCidaToaPidl(&pidl, cida); - if (pidls) - { - hr = paste_pidls(dst_folder, pidl, pidls, cida->cidl); - _ILFreeaPidl(pidls, cida->cidl); - SHFree(pidl); - } - else + op.pFrom = src_paths; + if ((ret = SHFileOperationW(&op))) { - hr = HRESULT_FROM_WIN32(GetLastError()); + WARN("Failed to copy, ret %d.\n", ret); + hr = E_FAIL; } - GlobalUnlock(medium.hGlobal); - } - else - { - hr = HRESULT_FROM_WIN32(GetLastError()); } + + free(src_paths); + _ILFreeaPidl(pidls, cida->cidl); + ILFree(root_pidl); + + GlobalUnlock(medium.hGlobal); ReleaseStgMedium(&medium); } else if (SUCCEEDED(get_data_format(data, CF_HDROP, &medium))) { const DROPFILES *dropfiles = GlobalLock(medium.hGlobal); - IPersistFolder2 *dst_persist; - SHFILEOPSTRUCTW op = {0}; - WCHAR dst_path[MAX_PATH]; - ITEMIDLIST *dst_pidl; - int ret; - - if (FAILED(hr = IShellFolder_QueryInterface(dst_folder, &IID_IPersistFolder2, (void **)&dst_persist))) - { - WARN("Failed to get IPersistFolder2, hr %#lx.\n", hr); - IDataObject_Release(data); - return hr; - } - - hr = IPersistFolder2_GetCurFolder(dst_persist, &dst_pidl); - IPersistFolder2_Release(dst_persist); - if (FAILED(hr)) - { - ERR("Failed to get dst folder pidl, hr %#lx.\n", hr); - IDataObject_Release(data); - return hr; - } - - if (!SHGetPathFromIDListW(dst_pidl, dst_path)) - { - ERR("Failed to get path, hr %#lx.\n", hr); - ILFree(dst_pidl); - IDataObject_Release(data); - return E_FAIL; - } - op.hwnd = hwnd; - op.wFunc = FO_COPY; op.pFrom = (const WCHAR *)((const char *)dropfiles + dropfiles->pFiles); - op.pTo = dst_path; - op.fFlags = FOF_ALLOWUNDO; if ((ret = SHFileOperationW(&op))) { WARN("Failed to copy, ret %d.\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5252
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/shell32/recyclebin.c | 8 ------ dlls/shell32/shellfolder.h | 2 -- dlls/shell32/shfldr_fs.c | 55 -------------------------------------- 3 files changed, 65 deletions(-) diff --git a/dlls/shell32/recyclebin.c b/dlls/shell32/recyclebin.c index 91f13413dce..b6d9879fa39 100644 --- a/dlls/shell32/recyclebin.c +++ b/dlls/shell32/recyclebin.c @@ -1200,13 +1200,6 @@ static HRESULT WINAPI RecycleBin_DeleteItems(ISFHelper * iface, UINT cidl, return erase_items(GetActiveWindow(),apidl,cidl,TRUE); } -static HRESULT WINAPI RecycleBin_CopyItems(ISFHelper * iface, - IShellFolder * pSFFrom, - UINT cidl, LPCITEMIDLIST * apidl) -{ - return E_NOTIMPL; -} - static const ISFHelperVtbl sfhelperVtbl = { RecycleBin_ISFHelper_QueryInterface, @@ -1215,7 +1208,6 @@ static const ISFHelperVtbl sfhelperVtbl = RecycleBin_GetUniqueName, RecycleBin_AddFolder, RecycleBin_DeleteItems, - RecycleBin_CopyItems }; HRESULT WINAPI SHQueryRecycleBinA(LPCSTR pszRootPath, LPSHQUERYRBINFO pSHQueryRBInfo) diff --git a/dlls/shell32/shellfolder.h b/dlls/shell32/shellfolder.h index 942e0e7ea28..da3e7b568e6 100644 --- a/dlls/shell32/shellfolder.h +++ b/dlls/shell32/shellfolder.h @@ -49,7 +49,6 @@ DECLARE_INTERFACE_(ISFHelper,IUnknown) STDMETHOD(GetUniqueName)(THIS_ LPWSTR lpName, UINT uLen) PURE; STDMETHOD(AddFolder)(THIS_ HWND hwnd, LPCWSTR lpName, LPITEMIDLIST * ppidlOut) PURE; STDMETHOD(DeleteItems)(THIS_ UINT cidl, LPCITEMIDLIST * apidl) PURE; - STDMETHOD(CopyItems)(THIS_ IShellFolder * pSFFrom, UINT cidl, LPCITEMIDLIST * apidl) PURE; }; #undef INTERFACE @@ -62,7 +61,6 @@ DECLARE_INTERFACE_(ISFHelper,IUnknown) #define ISFHelper_GetUniqueName(p,a,b) (p)->lpVtbl->GetUniqueName(p,a,b) #define ISFHelper_AddFolder(p,a,b,c) (p)->lpVtbl->AddFolder(p,a,b,c) #define ISFHelper_DeleteItems(p,a,b) (p)->lpVtbl->DeleteItems(p,a,b) -#define ISFHelper_CopyItems(p,a,b,c) (p)->lpVtbl->CopyItems(p,a,b,c) #endif #endif /* __WINE_SHELLFOLDER_HELP_H */ diff --git a/dlls/shell32/shfldr_fs.c b/dlls/shell32/shfldr_fs.c index 54828f99acc..808b20c9cae 100644 --- a/dlls/shell32/shfldr_fs.c +++ b/dlls/shell32/shfldr_fs.c @@ -1314,60 +1314,6 @@ ISFHelper_fnDeleteItems (ISFHelper * iface, UINT cidl, LPCITEMIDLIST * apidl) return ret; } -/**************************************************************************** - * ISFHelper_fnCopyItems - * - * copies items to this folder - */ -static HRESULT WINAPI -ISFHelper_fnCopyItems (ISFHelper * iface, IShellFolder * pSFFrom, UINT cidl, - LPCITEMIDLIST * apidl) -{ - HRESULT ret=E_FAIL; - IPersistFolder2 *ppf2 = NULL; - WCHAR wszSrcPathRoot[MAX_PATH], - wszDstPath[MAX_PATH+1]; - WCHAR *wszSrcPathsList; - IGenericSFImpl *This = impl_from_ISFHelper(iface); - - SHFILEOPSTRUCTW fop; - - TRACE ("(%p)->(%p,%u,%p)\n", This, pSFFrom, cidl, apidl); - - IShellFolder_QueryInterface (pSFFrom, &IID_IPersistFolder2, - (LPVOID *) & ppf2); - if (ppf2) { - LPITEMIDLIST pidl; - - if (SUCCEEDED (IPersistFolder2_GetCurFolder (ppf2, &pidl))) { - SHGetPathFromIDListW (pidl, wszSrcPathRoot); - if (This->sPathTarget) - lstrcpynW(wszDstPath, This->sPathTarget, MAX_PATH); - else - wszDstPath[0] = 0; - PathAddBackslashW(wszSrcPathRoot); - PathAddBackslashW(wszDstPath); - wszSrcPathsList = build_paths_list(wszSrcPathRoot, cidl, apidl); - ZeroMemory(&fop, sizeof(fop)); - fop.hwnd = GetActiveWindow(); - fop.wFunc = FO_COPY; - fop.pFrom = wszSrcPathsList; - fop.pTo = wszDstPath; - fop.fFlags = FOF_ALLOWUNDO; - ret = S_OK; - if(SHFileOperationW(&fop)) - { - WARN("Copy failed\n"); - ret = E_FAIL; - } - free(wszSrcPathsList); - } - SHFree(pidl); - IPersistFolder2_Release(ppf2); - } - return ret; -} - static const ISFHelperVtbl shvt = { ISFHelper_fnQueryInterface, @@ -1376,7 +1322,6 @@ static const ISFHelperVtbl shvt = ISFHelper_fnGetUniqueName, ISFHelper_fnAddFolder, ISFHelper_fnDeleteItems, - ISFHelper_fnCopyItems }; /************************************************************************ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5252
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/shell32/shlview_cmenu.c | 37 ++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/dlls/shell32/shlview_cmenu.c b/dlls/shell32/shlview_cmenu.c index fa29ac2dc02..7fee1c1c2df 100644 --- a/dlls/shell32/shlview_cmenu.c +++ b/dlls/shell32/shlview_cmenu.c @@ -327,19 +327,32 @@ static void DoDelete(ContextMenu *This) } } -/************************************************************************** - * DoCopyOrCut - * - * copies the currently selected items into the clipboard - */ -static void DoCopyOrCut(ContextMenu *This, HWND hwnd, BOOL cut) +static void do_copy(ContextMenu *This, HWND hwnd, DWORD drop_effect) { IDataObject *dataobject; - TRACE("(%p)->(wnd=%p, cut=%d)\n", This, hwnd, cut); - if (SUCCEEDED(IShellFolder_GetUIObjectOf(This->parent, hwnd, This->cidl, (LPCITEMIDLIST*)This->apidl, &IID_IDataObject, 0, (void**)&dataobject))) { + FORMATETC format; + STGMEDIUM medium; + DWORD *effect_ptr; + HRESULT hr; + + InitFormatEtc(format, RegisterClipboardFormatW(CFSTR_PREFERREDDROPEFFECTW), TYMED_HGLOBAL); + medium.tymed = TYMED_HGLOBAL; + medium.pUnkForRelease = NULL; + if (!(medium.hGlobal = GlobalAlloc(GMEM_MOVEABLE, sizeof(DWORD)))) + { + IDataObject_Release(dataobject); + return; + } + effect_ptr = GlobalLock(medium.hGlobal); + *effect_ptr = drop_effect; + GlobalUnlock(medium.hGlobal); + + if (FAILED(hr = IDataObject_SetData(dataobject, &format, &medium, TRUE))) + ERR("Failed to set data, hr %#lx.\n", hr); + OleSetClipboard(dataobject); IDataObject_Release(dataobject); } @@ -973,11 +986,11 @@ static HRESULT WINAPI ItemMenu_InvokeCommand( break; case FCIDM_SHVIEW_COPY: TRACE("Verb FCIDM_SHVIEW_COPY\n"); - DoCopyOrCut(This, lpcmi->hwnd, FALSE); + do_copy(This, lpcmi->hwnd, DROPEFFECT_COPY | DROPEFFECT_LINK); break; case FCIDM_SHVIEW_CUT: TRACE("Verb FCIDM_SHVIEW_CUT\n"); - DoCopyOrCut(This, lpcmi->hwnd, TRUE); + do_copy(This, lpcmi->hwnd, DROPEFFECT_MOVE); break; case FCIDM_SHVIEW_INSERT: do_paste(This, lpcmi->hwnd); @@ -997,9 +1010,9 @@ static HRESULT WINAPI ItemMenu_InvokeCommand( if (strcmp(lpcmi->lpVerb,"delete")==0) DoDelete(This); else if (strcmp(lpcmi->lpVerb,"copy")==0) - DoCopyOrCut(This, lpcmi->hwnd, FALSE); + do_copy(This, lpcmi->hwnd, DROPEFFECT_COPY | DROPEFFECT_LINK); else if (strcmp(lpcmi->lpVerb,"cut")==0) - DoCopyOrCut(This, lpcmi->hwnd, TRUE); + do_copy(This, lpcmi->hwnd, DROPEFFECT_MOVE); else if (!strcmp(lpcmi->lpVerb, "paste")) do_paste(This, lpcmi->hwnd); else if (strcmp(lpcmi->lpVerb,"properties")==0) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5252
From: Zebediah Figura <zfigura(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=34322 --- dlls/shell32/shlview_cmenu.c | 17 ++++++++++++++++- dlls/shell32/tests/shlfolder.c | 22 +++------------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/dlls/shell32/shlview_cmenu.c b/dlls/shell32/shlview_cmenu.c index 7fee1c1c2df..cac33c39af7 100644 --- a/dlls/shell32/shlview_cmenu.c +++ b/dlls/shell32/shlview_cmenu.c @@ -409,6 +409,7 @@ static WCHAR *build_source_paths(ITEMIDLIST *root_pidl, ITEMIDLIST **pidls, unsi static HRESULT do_paste(ContextMenu *menu, HWND hwnd) { IPersistFolder2 *dst_persist; + const DWORD *drop_effect; IShellFolder *dst_folder; WCHAR dst_path[MAX_PATH]; SHFILEOPSTRUCTW op = {0}; @@ -456,13 +457,27 @@ static HRESULT do_paste(ContextMenu *menu, HWND hwnd) ILFree(dst_pidl); op.hwnd = hwnd; - op.wFunc = FO_COPY; op.pTo = dst_path; op.fFlags = FOF_ALLOWUNDO; if (FAILED(hr = OleGetClipboard(&data))) return hr; + if (FAILED(hr = get_data_format(data, RegisterClipboardFormatW(CFSTR_PREFERREDDROPEFFECTW), &medium))) + { + ERR("Failed to get drop effect.\n"); + IDataObject_Release(data); + return hr; + } + drop_effect = GlobalLock(medium.hGlobal); + if (*drop_effect & DROPEFFECT_COPY) + op.wFunc = FO_COPY; + else if (*drop_effect & DROPEFFECT_MOVE) + op.wFunc = FO_MOVE; + else + FIXME("Unhandled drop effect %#lx.\n", *drop_effect); + GlobalUnlock(medium.hGlobal); + if (SUCCEEDED(get_data_format(data, RegisterClipboardFormatW(CFSTR_SHELLIDLISTW), &medium))) { const CIDA *cida = GlobalLock(medium.hGlobal); diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index b2448938728..06ccfe2f553 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -5890,14 +5890,7 @@ static void test_copy_paste(void) format.cfFormat = RegisterClipboardFormatW(CFSTR_PREFERREDDROPEFFECTW); hr = IDataObject_GetData(data_obj, &format, &medium); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); - if (hr == S_OK) - { - effect = GlobalLock(medium.hGlobal); - ok(*effect == DROPEFFECT_MOVE, "Got effect %#lx.\n", *effect); - GlobalUnlock(medium.hGlobal); - ReleaseStgMedium(&medium); - } + ok(hr == S_OK, "Got hr %#lx.\n", hr); IDataObject_Release(data_obj); @@ -5913,9 +5906,7 @@ static void test_copy_paste(void) ok(hr == S_OK, "Got hr %#lx.\n", hr); ret = MoveFileExW(L"testcopy_dst/testcopy_src", L"testcopy_src", 0); - todo_wine ok(ret, "Got error %lu.\n", GetLastError()); - if (!ret && GetLastError() == ERROR_ALREADY_EXISTS) - RemoveDirectoryW(L"testcopy_dst/testcopy_src"); + ok(ret, "Got error %lu.\n", GetLastError()); /* Copy. */ @@ -5933,14 +5924,7 @@ static void test_copy_paste(void) format.cfFormat = RegisterClipboardFormatW(CFSTR_PREFERREDDROPEFFECTW); hr = IDataObject_GetData(data_obj, &format, &medium); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); - if (hr == S_OK) - { - effect = GlobalLock(medium.hGlobal); - ok(*effect == (DROPEFFECT_COPY | DROPEFFECT_LINK), "Got effect %#lx.\n", *effect); - GlobalUnlock(medium.hGlobal); - ReleaseStgMedium(&medium); - } + ok(hr == S_OK, "Got hr %#lx.\n", hr); IDataObject_Release(data_obj); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5252
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=143788 Your paranoid android. === w1064v1507 (64 bit report) === shell32: shlfolder.c:5460: Test failed: Got hr 0x80004001. === w1064v1809 (64 bit report) === shell32: shlfolder.c:5460: Test failed: Got hr 0x80004001. === w1064_2qxl (64 bit report) === shell32: shlfolder.c:5460: Test failed: Got hr 0x80004001. === w1064_adm (64 bit report) === shell32: shlfolder.c:5460: Test failed: Got hr 0x80004001. === w10pro64 (64 bit report) === shell32: shlfolder.c:5460: Test failed: Got hr 0x80004001. === w10pro64_ja (64 bit report) === shell32: shlfolder.c:5460: Test failed: Got hr 0x80004001. === w10pro64_zh_CN (64 bit report) === shell32: shlfolder.c:5460: Test failed: Got hr 0x80004001.
participants (3)
-
Marvin -
Zebediah Figura -
Zebediah Figura (＠zfigura)