[PATCH v11 0/4] MR566: shell32: Implement SHBindToFolderIDListParent.
Called by IE11. Signed-off-by: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> v4: Don't reimplement SHBindToParent. -- v11: shell32: Fix last parameter behavior in SHBindToFolderIDListParent(). shell32/tests: Test SHBindToParent() last parameter behavior. shell32/tests: Add SHBindToFolderIDListParent() tests. shell32: Implement SHBindToFolderIDListParent(). https://gitlab.winehq.org/wine/wine/-/merge_requests/566
From: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> Called by IE11. Signed-off-by: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> --- dlls/shell32/pidl.c | 35 ++++++++++++++++++++++++----------- dlls/shell32/shell32.spec | 1 + include/shlobj.h | 2 ++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/dlls/shell32/pidl.c b/dlls/shell32/pidl.c index f9c91a79132..a95e4c5f295 100644 --- a/dlls/shell32/pidl.c +++ b/dlls/shell32/pidl.c @@ -1290,12 +1290,20 @@ BOOL WINAPI SHGetPathFromIDListEx(LPCITEMIDLIST pidl, WCHAR *path, DWORD path_si */ HRESULT WINAPI SHBindToParent(LPCITEMIDLIST pidl, REFIID riid, LPVOID *ppv, LPCITEMIDLIST *ppidlLast) { - IShellFolder * psfDesktop; - HRESULT hr=E_FAIL; + return SHBindToFolderIDListParent(NULL, pidl, riid, ppv, ppidlLast); +} + +/************************************************************************* + * SHBindToFolderIDListParent [SHELL32.@] + */ +HRESULT WINAPI SHBindToFolderIDListParent(IShellFolder *psf, LPCITEMIDLIST pidl, REFIID riid, LPVOID *ppv, LPCITEMIDLIST *ppidlLast) +{ + IShellFolder *psfDesktop = NULL; + HRESULT hr; - TRACE_(shell)("pidl=%p\n", pidl); + TRACE_(shell)("%p,%p,%s\n", psf, pidl, debugstr_guid(riid)); pdump(pidl); - + if (!pidl || !ppv) return E_INVALIDARG; @@ -1303,29 +1311,34 @@ HRESULT WINAPI SHBindToParent(LPCITEMIDLIST pidl, REFIID riid, LPVOID *ppv, LPCI if (ppidlLast) *ppidlLast = NULL; - hr = SHGetDesktopFolder(&psfDesktop); - if (FAILED(hr)) - return hr; + if (!psf) + { + hr = SHGetDesktopFolder(&psfDesktop); + if (FAILED(hr)) + return hr; + psf = psfDesktop; + } if (_ILIsPidlSimple(pidl)) { /* we are on desktop level */ - hr = IShellFolder_QueryInterface(psfDesktop, riid, ppv); + hr = IShellFolder_QueryInterface(psf, riid, ppv); } else { LPITEMIDLIST pidlParent = ILClone(pidl); ILRemoveLastID(pidlParent); - hr = IShellFolder_BindToObject(psfDesktop, pidlParent, NULL, riid, ppv); + hr = IShellFolder_BindToObject(psf, pidlParent, NULL, riid, ppv); SHFree (pidlParent); } - IShellFolder_Release(psfDesktop); + if (psfDesktop) + IShellFolder_Release(psfDesktop); if (SUCCEEDED(hr) && ppidlLast) *ppidlLast = ILFindLastID(pidl); - TRACE_(shell)("-- psf=%p pidl=%p ret=0x%08lx\n", *ppv, (ppidlLast)?*ppidlLast:NULL, hr); + TRACE_(shell)("-- ppv=%p pidl=%p ret=0x%08lx\n", *ppv, (ppidlLast)?*ppidlLast:NULL, hr); return hr; } diff --git a/dlls/shell32/shell32.spec b/dlls/shell32/shell32.spec index 4a64b80ddd9..aba9166920a 100644 --- a/dlls/shell32/shell32.spec +++ b/dlls/shell32/shell32.spec @@ -334,6 +334,7 @@ @ stdcall SHAssocEnumHandlers(wstr long ptr) @ stdcall SHBindToObject(ptr ptr ptr ptr ptr) @ stdcall SHBindToParent(ptr ptr ptr ptr) +@ stdcall SHBindToFolderIDListParent(ptr ptr ptr ptr ptr) @ stdcall SHBrowseForFolder(ptr) SHBrowseForFolderA @ stdcall SHBrowseForFolderA(ptr) @ stdcall SHBrowseForFolderW(ptr) diff --git a/include/shlobj.h b/include/shlobj.h index c7452cf5f0a..4cb7375bb59 100644 --- a/include/shlobj.h +++ b/include/shlobj.h @@ -1748,6 +1748,8 @@ WINSHELLAPI HRESULT WINAPI SHGetFolderPathW(HWND hwnd, int nFolder, HANDLE hToke */ WINSHELLAPI HRESULT WINAPI SHGetDesktopFolder(IShellFolder * *); +HRESULT WINAPI SHBindToFolderIDListParent(IShellFolder *psf, LPCITEMIDLIST pidl, REFIID riid, LPVOID *ppv, LPCITEMIDLIST *ppidlLast); + /**************************************************************************** * SHBindToParent API */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/566
From: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/shell32/tests/shlfolder.c | 53 ++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index dc0b6cb2ee2..0891b25dab4 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -60,6 +60,7 @@ static BOOL (WINAPI *pIsWow64Process)(HANDLE, PBOOL); static HRESULT (WINAPI *pSHCreateDefaultContextMenu)(const DEFCONTEXTMENU*,REFIID,void**); static BOOL (WINAPI *pSHGetPathFromIDListEx)(PCIDLIST_ABSOLUTE,WCHAR*,DWORD,GPFIDL_FLAGS); static HRESULT (WINAPI *pSHGetSetFolderCustomSettings)(LPSHFOLDERCUSTOMSETTINGS,PCWSTR,DWORD); +static HRESULT (WINAPI *pSHBindToFolderIDListParent)(IShellFolder*,LPCITEMIDLIST,REFIID,void **,LPCITEMIDLIST*); static WCHAR *make_wstr(const char *str) { @@ -120,6 +121,7 @@ static void init_function_pointers(void) hmod = GetModuleHandleA("shell32.dll"); #define MAKEFUNC(f) (p##f = (void*)GetProcAddress(hmod, #f)) + MAKEFUNC(SHBindToFolderIDListParent); MAKEFUNC(SHCreateItemFromIDList); MAKEFUNC(SHCreateItemFromParsingName); MAKEFUNC(SHCreateItemFromRelativeName); @@ -5550,6 +5552,56 @@ static void test_SHOpenFolderAndSelectItems(void) ILFree(folder); } +static void test_SHBindToFolderIDListParent(void) +{ + IShellFolder *psf_desktop; + LPITEMIDLIST pidl; + HRESULT hr; + WCHAR path[MAX_PATH]; + SHITEMID empty_item = { 0, { 0 } }; + LPITEMIDLIST pidl_empty = (LPITEMIDLIST)&empty_item; + LPCITEMIDLIST pidl_last; + IShellFolder *psf; + + if (!pSHBindToFolderIDListParent) + { + win_skip("SHBindToFolderIDListParent not available\n"); + return; + } + + GetTempPathW(ARRAY_SIZE(path), path); + SHGetDesktopFolder(&psf_desktop); + + hr = IShellFolder_ParseDisplayName(psf_desktop, NULL, NULL, path, NULL, &pidl, 0); + ok(hr == S_OK, "got %#lx\n", hr); + + pidl_last = NULL; + hr = pSHBindToFolderIDListParent(psf_desktop, pidl, &IID_IShellFolder, (void **)&psf, &pidl_last); + ok(hr == S_OK, "got %#lx\n", hr); + ok(pidl_last != NULL, "got %p\n", pidl_last); + IShellFolder_Release(psf); + + hr = pSHBindToFolderIDListParent(NULL, pidl_empty, &IID_IShellFolder, (void **)&psf, &pidl_last); + ok(hr == S_OK, "got %#lx\n", hr); + ok(pidl_last == pidl_empty, "got %p\n", pidl_last); + IShellFolder_Release(psf); + + hr = pSHBindToFolderIDListParent(NULL, pidl, &IID_IShellFolder, (void **)&psf, NULL); + ok(hr == S_OK, "got %#lx\n", hr); + IShellFolder_Release(psf); + + if (0) /* crashes under Windows */ + hr = pSHBindToFolderIDListParent(NULL, pidl, &IID_IShellFolder, NULL, NULL); + + ILFree(pidl); + IShellFolder_Release(psf_desktop); + + pidl_last = (LPITEMIDLIST)0xdeadbeef; + hr = pSHBindToFolderIDListParent(NULL, NULL, &IID_IShellFolder, (void **)&psf, &pidl_last); + ok(hr == E_INVALIDARG, "got %#lx\n", hr); + todo_wine ok(pidl_last == NULL, "got %p\n", pidl_last); +} + START_TEST(shlfolder) { init_function_pointers(); @@ -5557,6 +5609,7 @@ START_TEST(shlfolder) CO_E_NOTINITIALIZED for malformed directory names */ OleInitialize(NULL); + test_SHBindToFolderIDListParent(); test_ParseDisplayName(); test_SHParseDisplayName(); test_BindToObject(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/566
From: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> --- dlls/shell32/tests/shlfolder.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index 0891b25dab4..5a5eb9fead8 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -775,9 +775,11 @@ static void test_GetDisplayName(void) ok (!lstrcmpiW(wszTestFile, wszTestFile2), "SHGetPathFromIDListW returns incorrect path!\n"); /* SHBindToParent fails, if called with a NULL PIDL. */ + pidlLast = (LPITEMIDLIST)0xdeadbeef; hr = SHBindToParent(NULL, &IID_IShellFolder, (void **)&psfPersonal, &pidlLast); ok (hr == E_INVALIDARG || broken(hr == E_OUTOFMEMORY) /* XP */, "SHBindToParent(NULL) should fail! hr = %08lx\n", hr); + todo_wine ok(pidlLast == NULL, "got %p\n", pidlLast); /* But it succeeds with an empty PIDL. */ hr = SHBindToParent(pidlEmpty, &IID_IShellFolder, (void **)&psfPersonal, &pidlLast); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/566
From: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> --- dlls/shell32/pidl.c | 7 ++++--- dlls/shell32/tests/shlfolder.c | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/shell32/pidl.c b/dlls/shell32/pidl.c index a95e4c5f295..c9ff5acbfc5 100644 --- a/dlls/shell32/pidl.c +++ b/dlls/shell32/pidl.c @@ -1304,12 +1304,13 @@ HRESULT WINAPI SHBindToFolderIDListParent(IShellFolder *psf, LPCITEMIDLIST pidl, TRACE_(shell)("%p,%p,%s\n", psf, pidl, debugstr_guid(riid)); pdump(pidl); + if (ppidlLast) + *ppidlLast = NULL; + if (!pidl || !ppv) return E_INVALIDARG; - + *ppv = NULL; - if (ppidlLast) - *ppidlLast = NULL; if (!psf) { diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index 5a5eb9fead8..be22e577c8b 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -779,7 +779,7 @@ static void test_GetDisplayName(void) hr = SHBindToParent(NULL, &IID_IShellFolder, (void **)&psfPersonal, &pidlLast); ok (hr == E_INVALIDARG || broken(hr == E_OUTOFMEMORY) /* XP */, "SHBindToParent(NULL) should fail! hr = %08lx\n", hr); - todo_wine ok(pidlLast == NULL, "got %p\n", pidlLast); + ok(pidlLast == NULL, "got %p\n", pidlLast); /* But it succeeds with an empty PIDL. */ hr = SHBindToParent(pidlEmpty, &IID_IShellFolder, (void **)&psfPersonal, &pidlLast); @@ -5601,7 +5601,7 @@ static void test_SHBindToFolderIDListParent(void) pidl_last = (LPITEMIDLIST)0xdeadbeef; hr = pSHBindToFolderIDListParent(NULL, NULL, &IID_IShellFolder, (void **)&psf, &pidl_last); ok(hr == E_INVALIDARG, "got %#lx\n", hr); - todo_wine ok(pidl_last == NULL, "got %p\n", pidl_last); + ok(pidl_last == NULL, "got %p\n", pidl_last); } START_TEST(shlfolder) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/566
On Tue Apr 25 00:52:33 2023 +0000, Huw Davies wrote:
I've not really been following this, but why doesn't `SHBindToParent()` call `SHBindToFolderIDListParent()`? What you want to do is to implement `SHBindToFolderIDListParent()` using the code in `SHBindToParent()` as a start, adding the code to deal with the passed in `IShellFolder`. Then **in the same commit** change `SHBindToParent()` to call `SHBindToFolderIDListParent()`. With careful choice of the positioning of the second function with respect to first one, the diff should be quite small. There were some comments regarding how the changes should be committed and it wasn't really clear to me what was requested so I ultimately decided to just implement `SHBindToFolderIDListParent()` on its own.
I've done as you suggested and implemented `SHBindToFolderIDListParent()` using the code in `SHBindToParent()` in the first commit. I added tests for how the last parameter is handled in Windows in the third commit and fixed it in the last commit. Regarding the diff, there weren't any significant differences if `SHBindToFolderIDListParent()` was added above `SHBindToParent()` or below it. Thanks so much for the review! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/566#note_31066
Huw Davies (@huw) commented about dlls/shell32/shell32.spec:
@ stdcall SHAssocEnumHandlers(wstr long ptr) @ stdcall SHBindToObject(ptr ptr ptr ptr ptr) @ stdcall SHBindToParent(ptr ptr ptr ptr) +@ stdcall SHBindToFolderIDListParent(ptr ptr ptr ptr ptr) Please keep these in alphabetical order.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/566#note_31072
Huw Davies (@huw) commented about include/shlobj.h:
*/ WINSHELLAPI HRESULT WINAPI SHGetDesktopFolder(IShellFolder * *);
+HRESULT WINAPI SHBindToFolderIDListParent(IShellFolder *psf, LPCITEMIDLIST pidl, REFIID riid, LPVOID *ppv, LPCITEMIDLIST *ppidlLast); `WINSHELLAPI` like the others (you may then want to wrap the line into two lines).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/566#note_31073
participants (4)
-
Dmitry Timoshkov -
Huw Davies (＠huw)
-
Mohamad Al-Jaf
-
Mohamad Al-Jaf (＠maljaf)