[PATCH v3 0/1] MR6123: shdocvw: Add IEParseDisplayNameWithBCW semi-stub.
-- v3: shdocvw: Add IEParseDisplayNameWithBCW semi-stub. https://gitlab.winehq.org/wine/wine/-/merge_requests/6123
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/shdocvw/Makefile.in | 2 +- dlls/shdocvw/shdocvw_main.c | 56 +++++++++++++++++++++++-- dlls/shdocvw/tests/shdocvw.c | 74 ++++++++++++++++++++++++++++++++++ dlls/shell32/tests/shlfolder.c | 8 ++-- 4 files changed, 132 insertions(+), 8 deletions(-) diff --git a/dlls/shdocvw/Makefile.in b/dlls/shdocvw/Makefile.in index d38782eaf22..bea902a7357 100644 --- a/dlls/shdocvw/Makefile.in +++ b/dlls/shdocvw/Makefile.in @@ -1,7 +1,7 @@ MODULE = shdocvw.dll IMPORTLIB = shdocvw IMPORTS = uuid shlwapi advapi32 -DELAYIMPORTS = version ole32 oleaut32 ieframe +DELAYIMPORTS = version ole32 oleaut32 ieframe shell32 SOURCES = \ shdocvw.rc \ diff --git a/dlls/shdocvw/shdocvw_main.c b/dlls/shdocvw/shdocvw_main.c index 6556223e1d7..c1be3d10fb6 100644 --- a/dlls/shdocvw/shdocvw_main.c +++ b/dlls/shdocvw/shdocvw_main.c @@ -431,11 +431,61 @@ DWORD WINAPI ParseURLFromOutsideSourceA(LPCSTR url, LPSTR out, LPDWORD plen, LPD /****************************************************************** * IEParseDisplayNameWithBCW (SHDOCVW.218) */ -HRESULT WINAPI IEParseDisplayNameWithBCW(DWORD codepage, LPCWSTR lpszDisplayName, LPBC pbc, LPITEMIDLIST *ppidl) +HRESULT WINAPI IEParseDisplayNameWithBCW(DWORD codepage, LPCWSTR name, IBindCtx *pbc, LPITEMIDLIST *ppidl) { +#include <pshpack1.h> + struct ie_pidl_data + { + BYTE type; /* 0x61 - PT_IESPECIAL1 */ + BYTE dummy1; /* 0x80 */ + DWORD dummy2; /* 0 */ + WCHAR name[1]; /* terminated by double \0 */ + } *ie_data; +#include <poppack.h> + HRESULT hr; + LPITEMIDLIST parent, child, next, ret; + DWORD size, len; + PARSEDURLW res; + /* Guessing at parameter 3 based on IShellFolder's ParseDisplayName */ - FIXME("stub: 0x%lx %s %p %p\n",codepage,debugstr_w(lpszDisplayName),pbc,ppidl); - return E_FAIL; + FIXME("%lu %s %p %p: semi-stub\n", codepage, debugstr_w(name), pbc, ppidl); + + /* Make sure that it's correct URL */ + res.cbSize = sizeof(res); + hr = ParseURLW(name, &res); + if (hr != S_OK) return E_FAIL; + + hr = SHGetFolderLocation(0, CSIDL_INTERNET, 0, 0, &parent); + if (hr != S_OK) return hr; + + len = wcslen(name) + 2; + size = sizeof(WORD) + sizeof(*ie_data) + len * sizeof(WCHAR) + sizeof(*next); + + child = SHAlloc(size); + if (!child) return E_OUTOFMEMORY; + + child->mkid.cb = FIELD_OFFSET(ITEMIDLIST, mkid.abID[1]) + len * sizeof(WCHAR); + ie_data = (struct ie_pidl_data *)child->mkid.abID; + ie_data->type = 0x61; + ie_data->dummy1 = 0x80; + ie_data->dummy2 = 0; + wcscpy(ie_data->name, name); + ie_data->name[len - 2] = 0; + ie_data->name[len - 1] = 0; + + next = (LPITEMIDLIST)((char *)child + child->mkid.cb); + next->mkid.cb = 0; + next->mkid.abID[0] = 0; + + ret = ILCombine(parent, child); + + SHFree(parent); + SHFree(child); + + if (!ret) return E_OUTOFMEMORY; + + *ppidl = ret; + return S_OK; } /****************************************************************** diff --git a/dlls/shdocvw/tests/shdocvw.c b/dlls/shdocvw/tests/shdocvw.c index fcfd4a644db..a97da709ed5 100644 --- a/dlls/shdocvw/tests/shdocvw.c +++ b/dlls/shdocvw/tests/shdocvw.c @@ -18,6 +18,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#define COBJMACROS #include <stdarg.h> @@ -26,6 +27,8 @@ #include "winreg.h" #include "wininet.h" #include "winnls.h" +#include "shtypes.h" +#include "shlobj.h" #include "wine/test.h" @@ -35,6 +38,7 @@ static HMODULE hshdocvw; static HRESULT (WINAPI *pURLSubRegQueryA)(LPCSTR, LPCSTR, DWORD, LPVOID, DWORD, DWORD); static DWORD (WINAPI *pParseURLFromOutsideSourceA)(LPCSTR, LPSTR, LPDWORD, LPDWORD); static DWORD (WINAPI *pParseURLFromOutsideSourceW)(LPCWSTR, LPWSTR, LPDWORD, LPDWORD); +static HRESULT (WINAPI *pIEParseDisplayNameWithBCW)(DWORD, LPCWSTR, LPBC, LPITEMIDLIST *); static const char appdata[] = "AppData"; static const char common_appdata[] = "Common AppData"; @@ -76,6 +80,7 @@ static void init_functions(void) pURLSubRegQueryA = (void *) GetProcAddress(hshdocvw, (LPSTR) 151); pParseURLFromOutsideSourceA = (void *) GetProcAddress(hshdocvw, (LPSTR) 169); pParseURLFromOutsideSourceW = (void *) GetProcAddress(hshdocvw, (LPSTR) 170); + pIEParseDisplayNameWithBCW = (void *) GetProcAddress(hshdocvw, (LPSTR) 218); } /* ################ */ @@ -351,11 +356,80 @@ static void test_ParseURLFromOutsideSourceW(void) /* ################ */ +static void test_IE_pidl(LPITEMIDLIST pidl, const WCHAR *name) +{ +#include <pshpack1.h> + struct ie_pidl_data + { + BYTE type; /* 0x61 - PT_IESPECIAL1 */ + BYTE dummy1; /* 0x80 */ + DWORD dummy2; /* 0 */ + WCHAR name[1]; /* terminated by double \0 */ + } *data; +#include <poppack.h> + + while (pidl->mkid.cb) + { + trace("pidl->mkid.cb = %u, pidl->mkid.abID[0] = %#x\n", pidl->mkid.cb, pidl->mkid.abID[0]); + + if (pidl->mkid.abID[0] == 0x1f) + { + GUID *guid = (GUID *)&pidl->mkid.abID[2]; + ok(IsEqualGUID(guid, &CLSID_Internet), "got %s\n", wine_dbgstr_guid(guid)); + ok(pidl->mkid.abID[1] == 0, "got %#x\n", pidl->mkid.abID[1]); + } + else if (pidl->mkid.abID[0] == 0x61) + { + data = (struct ie_pidl_data *)pidl->mkid.abID; + ok(data->dummy1 == 0x80, "got %#x\n", data->dummy1); + ok(data->dummy2 == 0, "got %#lx\n", data->dummy2); + ok(!lstrcmpW(data->name, name), "got %s\n", wine_dbgstr_w(data->name)); + } + + pidl = (LPITEMIDLIST)((char *)pidl + pidl->mkid.cb); + } +} + +static void test_IEParseDisplayNameWithBCW(void) +{ + HRESULT hr; + IBindCtx *ctx; + LPITEMIDLIST pidl; + + if (!pIEParseDisplayNameWithBCW) + { + skip("IEParseDisplayNameWithBCW not found\n"); + return; + } + + hr = CreateBindCtx(0, &ctx); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = pIEParseDisplayNameWithBCW(0, L"custom://url", ctx, &pidl); + ok(hr == S_OK, "got %#lx\n", hr); + test_IE_pidl(pidl, L"custom://url"); + SHFree(pidl); + + hr = pIEParseDisplayNameWithBCW(0, L"readme.txt", ctx, &pidl); + ok(hr == E_FAIL, "got %#lx\n", hr); + + hr = pIEParseDisplayNameWithBCW(0, L"ftp://url.org/readme.txt", ctx, &pidl); + ok(hr == S_OK, "got %#lx\n", hr); + test_IE_pidl(pidl, L"ftp://url.org/readme.txt"); + SHFree(pidl); + + if (0) /* hangs */ + hr = pIEParseDisplayNameWithBCW(0, L"file://readme.txt", ctx, &pidl); + + IBindCtx_Release(ctx); +} + START_TEST(shdocvw) { init_functions(); test_URLSubRegQueryA(); test_ParseURLFromOutsideSourceA(); test_ParseURLFromOutsideSourceW(); + test_IEParseDisplayNameWithBCW(); FreeLibrary(hshdocvw); } diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index f1cae84257a..21152f98484 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -213,10 +213,10 @@ static struct {{'t','e','s','t','\\',0}, 0x80070002}, {{'s','u','b','\\','d','i','r',0}, 0x80070002}, {{'s','u','b','/','d','i','r',0}, E_INVALIDARG, 1}, - {{'h','t','t','p',':',0}, S_OK, 1}, - {{'h','t','t','p',':','t','e','s','t',0}, S_OK, 1}, - {{'h','t','t','p',':','\\','t','e','s','t',0}, S_OK, 1}, - {{'x','x',':',0}, S_OK, 1}, + {{'h','t','t','p',':',0}, S_OK}, + {{'h','t','t','p',':','t','e','s','t',0}, S_OK}, + {{'h','t','t','p',':','\\','t','e','s','t',0}, S_OK}, + {{'x','x',':',0}, S_OK}, }; static void test_ParseDisplayName(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6123
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=147240 Your paranoid android. === debian11 (32 bit report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\0300$\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\0300$\0dad" === debian11 (32 bit ar:MA report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\0300$\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\0300$\0daa" === debian11 (32 bit fr report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\0300$\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\0300$\0daa" === debian11 (32 bit he:IL report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\0300$\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\0300$\0daa" === debian11 (32 bit hi:IN report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\0300$\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\0300$\0daa" === debian11 (32 bit ja:JP report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\6900\656e\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\0100$\0016" === debian11 (32 bit zh:CN report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\6900\656e\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\0100$\0016" === debian11b (32 bit WoW report) === shdocvw: shdocvw.c:386: Test failed: got L"custom://url\03005\0008" shdocvw.c:386: Test failed: got L"ftp://url.org/readme.txt\03005\0d74"
On Fri Jul 19 15:25:14 2024 +0000, Fabian Maurer wrote:
Might be a stupid question, but why +3, isn't this triple null terminated? Also I noticed you do a sizeof(WORD) for the child ITEMIDLIST, I assume to not waste the "BYTE abID[1];". Later you do something similar for ie_pidl_data, but using "FIELD_OFFSET(struct ie_pidl_data, name)". I'd find it easier to understand if you used the same concept for ITEMIDLIST, like FIELD_OFFSET(ITEMIDLIST, mkid.abID). I have to admit it took me a while to get where the WORD was coming from. Thanks for the review. Does new version of the patch make this clearer?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6123#note_76641
On Fri Jul 19 17:57:05 2024 +0000, Dmitry Timoshkov wrote:
Thanks for the review. Does new version of the patch make this clearer? No, I don't understand that change - I was taking about the line
size = sizeof(WORD) + sizeof(*ie_data) + len * sizeof(WCHAR) + sizeof(*next);
Your new change also breaks the tests. IMHO it would be helpful to add tests for `cb`, the double termination and the `next` part. I.e. have the complete layout tested. Does some application need that or where did you get that from? All that said, please keep in mind that I'm just hobby contributor, it's just my 2 cents. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6123#note_76651
participants (4)
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Fabian Maurer (＠DarkShadow44)
-
Marvin