[PATCH 0/6] MR2628: Some more shell32 improvements.
From: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/shell32/tests/shlview.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/dlls/shell32/tests/shlview.c b/dlls/shell32/tests/shlview.c index 0f6ad9f4158..80e8e845156 100644 --- a/dlls/shell32/tests/shlview.c +++ b/dlls/shell32/tests/shlview.c @@ -1493,10 +1493,36 @@ static void test_newmenu(void) static void test_folder_flags(void) { + IFolderView2 *folderview; FOLDERSETTINGS settings; IShellFolder *desktop; IShellView *shellview; + DWORD flags; HRESULT hr; + int i; + static const struct + { + DWORD mask, flags; + DWORD expected; + } tests[] = + { + { FWF_NONE, FWF_AUTOARRANGE, FWF_USESEARCHFOLDER }, + { FWF_AUTOARRANGE, FWF_AUTOARRANGE, FWF_USESEARCHFOLDER | FWF_AUTOARRANGE }, + { FWF_ABBREVIATEDNAMES, FWF_ABBREVIATEDNAMES | FWF_NOICONS, + FWF_USESEARCHFOLDER | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_ABBREVIATEDNAMES | FWF_NOICONS, FWF_ABBREVIATEDNAMES | FWF_NOVISIBLE, + FWF_USESEARCHFOLDER | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NOICONS, FWF_ABBREVIATEDNAMES | FWF_NOICONS, + FWF_USESEARCHFOLDER | FWF_NOICONS | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NONE, FWF_NONE, + FWF_USESEARCHFOLDER | FWF_NOICONS | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NOICONS, FWF_ABBREVIATEDNAMES | FWF_NOICONS, + FWF_USESEARCHFOLDER | FWF_NOICONS | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NOICONS, FWF_NONE, FWF_USESEARCHFOLDER | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { ~FWF_NONE, FWF_NONE, FWF_NONE }, + { FWF_NOICONS, FWF_NONE, FWF_NONE }, + { FWF_NONE, FWF_AUTOARRANGE, FWF_NONE }, + }; create_interfaces(&desktop, &shellview); @@ -1507,6 +1533,34 @@ static void test_folder_flags(void) ok(settings.fFlags == FWF_USESEARCHFOLDER || broken(!settings.fFlags) /* pre vista */, "Got flags %#x.\n", settings.fFlags); + hr = IShellView_QueryInterface(shellview, &IID_IFolderView2, (void **)&folderview); + ok(hr == S_OK || broken(hr == E_NOINTERFACE) /* pre vista */, "Got hr %#lx.\n", hr); + if (hr != S_OK) + { + win_skip("No IFolderView2 for the desktop folder.\n"); + destroy_interfaces(desktop, shellview); + return; + } + + hr = IFolderView2_GetCurrentFolderFlags(folderview, NULL); + todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + + flags = 0xdeadbeef; + hr = IFolderView2_GetCurrentFolderFlags(folderview, &flags); + todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + todo_wine ok(flags == FWF_USESEARCHFOLDER, "Got flags %#lx.\n", flags); + + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + hr = IFolderView2_SetCurrentFolderFlags(folderview, tests[i].mask, tests[i].flags); + todo_wine ok(hr == S_OK, "Test %u: Got hr %#lx.\n", i, hr); + flags = 0xdeadbeef; + hr = IFolderView2_GetCurrentFolderFlags(folderview, &flags); + todo_wine ok(hr == S_OK, "Test %u: Got hr %#lx.\n", i, hr); + todo_wine ok(flags == tests[i].expected, "Test %u: Got flags %#lx.\n", i, flags); + } + + IFolderView2_Release(folderview); destroy_interfaces(desktop, shellview); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2628
From: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/shell32/shlview.c | 19 +++++++++++++++---- dlls/shell32/tests/shlview.c | 12 ++++++------ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/dlls/shell32/shlview.c b/dlls/shell32/shlview.c index da60283393c..2a6abf7c8cf 100644 --- a/dlls/shell32/shlview.c +++ b/dlls/shell32/shlview.c @@ -2946,15 +2946,26 @@ static HRESULT WINAPI FolderView2_SetText(IFolderView2 *iface, FVTEXTTYPE type, static HRESULT WINAPI FolderView2_SetCurrentFolderFlags(IFolderView2 *iface, DWORD mask, DWORD flags) { IShellViewImpl *This = impl_from_IFolderView2(iface); - FIXME("(%p)->(0x%08lx 0x%08lx), stub\n", This, mask, flags); - return E_NOTIMPL; + + TRACE("(%p)->(%#lx %#lx).\n", This, mask, flags); + + if (flags == FWF_NONE) + This->FolderSettings.fFlags &= ~mask; + else + This->FolderSettings.fFlags |= flags & mask; + + return S_OK; } static HRESULT WINAPI FolderView2_GetCurrentFolderFlags(IFolderView2 *iface, DWORD *flags) { IShellViewImpl *This = impl_from_IFolderView2(iface); - FIXME("(%p)->(%p), stub\n", This, flags); - return E_NOTIMPL; + + TRACE("(%p)->(%p).\n", This, flags); + + if (flags) + *flags = This->FolderSettings.fFlags; + return S_OK; } static HRESULT WINAPI FolderView2_GetSortColumnCount(IFolderView2 *iface, int *columns) diff --git a/dlls/shell32/tests/shlview.c b/dlls/shell32/tests/shlview.c index 80e8e845156..72d7411fcd5 100644 --- a/dlls/shell32/tests/shlview.c +++ b/dlls/shell32/tests/shlview.c @@ -1543,21 +1543,21 @@ static void test_folder_flags(void) } hr = IFolderView2_GetCurrentFolderFlags(folderview, NULL); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(hr == S_OK, "Got hr %#lx.\n", hr); flags = 0xdeadbeef; hr = IFolderView2_GetCurrentFolderFlags(folderview, &flags); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); - todo_wine ok(flags == FWF_USESEARCHFOLDER, "Got flags %#lx.\n", flags); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(flags == FWF_USESEARCHFOLDER, "Got flags %#lx.\n", flags); for (i = 0; i < ARRAY_SIZE(tests); ++i) { hr = IFolderView2_SetCurrentFolderFlags(folderview, tests[i].mask, tests[i].flags); - todo_wine ok(hr == S_OK, "Test %u: Got hr %#lx.\n", i, hr); + ok(hr == S_OK, "Test %u: Got hr %#lx.\n", i, hr); flags = 0xdeadbeef; hr = IFolderView2_GetCurrentFolderFlags(folderview, &flags); - todo_wine ok(hr == S_OK, "Test %u: Got hr %#lx.\n", i, hr); - todo_wine ok(flags == tests[i].expected, "Test %u: Got flags %#lx.\n", i, flags); + ok(hr == S_OK, "Test %u: Got hr %#lx.\n", i, hr); + ok(flags == tests[i].expected, "Test %u: Got flags %#lx.\n", i, flags); } IFolderView2_Release(folderview); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2628
From: Jactry Zeng <jzeng(a)codeweavers.com> Only pre-win7 systems return E_INVALIDARG for invalid values, and only pre-vista systems accept FVM_AUTO as a view mode, others fall it back to FVM_ICON. Let's switch to modern behaviours, so that we don't need additional code for handling that when using it to implement IExplorerBrowser_SetFolderSettings(). --- dlls/shell32/shlview.c | 21 ++++++++------------- dlls/shell32/tests/shlview.c | 17 ++++++++++++----- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/dlls/shell32/shlview.c b/dlls/shell32/shlview.c index 2a6abf7c8cf..d91e3d1f9a6 100644 --- a/dlls/shell32/shlview.c +++ b/dlls/shell32/shlview.c @@ -2714,20 +2714,15 @@ static HRESULT WINAPI FolderView_SetCurrentViewMode(IFolderView2 *iface, UINT mo TRACE("%p, %u.\n", This, mode); - if((mode < FVM_FIRST || mode > FVM_LAST) && - (mode != FVM_AUTO)) - return E_INVALIDARG; - - /* Windows before Vista uses LVM_SETVIEW and possibly - LVM_SETEXTENDEDLISTVIEWSTYLE to set the style of the listview, - while later versions seem to accomplish this through other - means. */ - dwStyle = ViewModeToListStyle(mode); - SetStyle(This, dwStyle, LVS_TYPEMASK); + if (mode == FVM_AUTO) + mode = FVM_ICON; - /* This will not necessarily be the actual mode set above. - This mimics the behavior of Windows XP. */ - This->FolderSettings.ViewMode = mode; + if (mode >= FVM_FIRST && mode <= FVM_LAST) + { + dwStyle = ViewModeToListStyle(mode); + SetStyle(This, dwStyle, LVS_TYPEMASK); + This->FolderSettings.ViewMode = mode; + } return S_OK; } diff --git a/dlls/shell32/tests/shlview.c b/dlls/shell32/tests/shlview.c index 72d7411fcd5..0e1374e1bf1 100644 --- a/dlls/shell32/tests/shlview.c +++ b/dlls/shell32/tests/shlview.c @@ -1110,13 +1110,20 @@ static void test_GetSetCurrentViewMode(void) hr = IFolderView_SetCurrentViewMode(fview, FVM_AUTO); ok(hr == S_OK, "got (0x%08lx)\n", hr); + hr = IFolderView_GetCurrentViewMode(fview, &viewmode); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(viewmode == FVM_ICON || broken(viewmode == FVM_AUTO) /* pre vista */, "Got view mode %d.\n", viewmode); + hr = IFolderView_SetCurrentViewMode(fview, FVM_LIST); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFolderView_GetCurrentViewMode(fview, &viewmode); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(viewmode == FVM_LIST, "Got view mode %d.\n", viewmode); hr = IFolderView_SetCurrentViewMode(fview, 0); - ok(hr == E_INVALIDARG || broken(hr == S_OK), - "got (0x%08lx)\n", hr); - + ok(hr == S_OK || broken(hr == E_INVALIDARG) /* pre win7 */, "Got hr %#lx.\n", hr); hr = IFolderView_GetCurrentViewMode(fview, &viewmode); ok(hr == S_OK, "got (0x%08lx)\n", hr); + ok(viewmode == FVM_LIST || broken(viewmode == 0) /* pre vista */, "Got view mode %d.\n", viewmode); for(i = 1; i < 9; i++) { @@ -1134,8 +1141,8 @@ static void test_GetSetCurrentViewMode(void) } hr = IFolderView_SetCurrentViewMode(fview, 9); - ok(hr == E_INVALIDARG || broken(hr == S_OK), - "got (0x%08lx)\n", hr); + ok(hr == S_OK || broken(hr == E_INVALIDARG) /* pre win7 */, "Got hr %#lx.\n", hr); + ok(viewmode == FVM_CONTENT || broken(viewmode == FVM_THUMBSTRIP) /* vista */, "Got view mode %d.\n", viewmode); /* Test messages */ hwnd_lv = subclass_listview(hwnd); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2628
From: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/shell32/tests/ebrowser.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/dlls/shell32/tests/ebrowser.c b/dlls/shell32/tests/ebrowser.c index 9402388f643..e6d1448058c 100644 --- a/dlls/shell32/tests/ebrowser.c +++ b/dlls/shell32/tests/ebrowser.c @@ -983,7 +983,6 @@ static void test_basics(void) { IExplorerBrowser *peb; IShellBrowser *psb; - FOLDERSETTINGS fs; ULONG lres; EXPLORER_BROWSER_OPTIONS flags; HDWP hdwp; @@ -1088,13 +1087,6 @@ static void test_basics(void) ebrowser_instantiate(&peb); ebrowser_initialize(peb); - /* SetFolderSettings */ - hr = IExplorerBrowser_SetFolderSettings(peb, NULL); - ok(hr == E_INVALIDARG, "got (0x%08lx)\n", hr); - fs.ViewMode = 0; fs.fFlags = 0; - hr = IExplorerBrowser_SetFolderSettings(peb, &fs); - todo_wine ok(hr == E_INVALIDARG, "got (0x%08lx)\n", hr); - /* SetPropertyBag */ hr = IExplorerBrowser_SetPropertyBag(peb, NULL); ok(hr == E_INVALIDARG, "Got 0x%08lx\n", hr); @@ -1696,6 +1688,25 @@ static void setup_window(void) ok(hwnd != NULL, "Failed to create window for tests.\n"); } +static void test_folder_settings(void) +{ + IExplorerBrowser *browser; + FOLDERSETTINGS settings; + HRESULT hr; + + ebrowser_instantiate(&browser); + ebrowser_initialize(browser); + + hr = IExplorerBrowser_SetFolderSettings(browser, NULL); + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + + settings.ViewMode = 0; settings.fFlags = 0; + hr = IExplorerBrowser_SetFolderSettings(browser, &settings); + todo_wine ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + + IExplorerBrowser_Release(browser); +} + START_TEST(ebrowser) { OleInitialize(NULL); @@ -1719,6 +1730,7 @@ START_TEST(ebrowser) test_GetCurrentView(); test_SetSite(); test_InputObject(); + test_folder_settings(); DestroyWindow(hwnd); OleUninitialize(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2628
From: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/shell32/ebrowser.c | 49 ++++++++++++++++++---------------- dlls/shell32/tests/ebrowser.c | 50 ++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/dlls/shell32/ebrowser.c b/dlls/shell32/ebrowser.c index 9704228d426..59bdd6a5d74 100644 --- a/dlls/shell32/ebrowser.c +++ b/dlls/shell32/ebrowser.c @@ -340,24 +340,6 @@ static void size_panes(ExplorerBrowserImpl *This) TRUE); } -static HRESULT change_viewmode(ExplorerBrowserImpl *This, UINT viewmode) -{ - IFolderView *pfv; - HRESULT hr; - - if(!This->psv) - return E_FAIL; - - hr = IShellView_QueryInterface(This->psv, &IID_IFolderView, (void*)&pfv); - if(SUCCEEDED(hr)) - { - hr = IFolderView_SetCurrentViewMode(pfv, This->fs.ViewMode); - IFolderView_Release(pfv); - } - - return hr; -} - static HRESULT create_new_shellview(ExplorerBrowserImpl *This, IShellItem *psi) { IShellBrowser *psb = &This->IShellBrowser_iface; @@ -989,16 +971,37 @@ static HRESULT WINAPI IExplorerBrowser_fnSetFolderSettings(IExplorerBrowser *ifa const FOLDERSETTINGS *pfs) { ExplorerBrowserImpl *This = impl_from_IExplorerBrowser(iface); + IFolderView2 *view; + HRESULT hr; + TRACE("%p (%p)\n", This, pfs); - if(!pfs) + if (!pfs) return E_INVALIDARG; - This->fs.ViewMode = pfs->ViewMode; - This->fs.fFlags = pfs->fFlags | FWF_NOCLIENTEDGE; + if (pfs->ViewMode) + This->fs.ViewMode = pfs->ViewMode; + This->fs.fFlags = pfs->fFlags; - /* Change the settings of the current view, if any. */ - return change_viewmode(This, This->fs.ViewMode); + if (!This->psv) + return E_INVALIDARG; + + hr = IShellView_QueryInterface(This->psv, &IID_IFolderView2, (void *)&view); + if (SUCCEEDED(hr)) + { + hr = IFolderView2_SetCurrentViewMode(view, This->fs.ViewMode); + if (SUCCEEDED(hr)) + { + if (SUCCEEDED(hr)) + { + hr = IFolderView2_SetCurrentFolderFlags(view, ~FWF_NONE, FWF_NONE); + if (SUCCEEDED(hr)) + hr = IFolderView2_SetCurrentFolderFlags(view, This->fs.fFlags, This->fs.fFlags); + } + } + IFolderView2_Release(view); + } + return hr; } static HRESULT WINAPI IExplorerBrowser_fnAdvise(IExplorerBrowser *iface, diff --git a/dlls/shell32/tests/ebrowser.c b/dlls/shell32/tests/ebrowser.c index e6d1448058c..c1fdbeb3364 100644 --- a/dlls/shell32/tests/ebrowser.c +++ b/dlls/shell32/tests/ebrowser.c @@ -1688,6 +1688,23 @@ static void setup_window(void) ok(hwnd != NULL, "Failed to create window for tests.\n"); } +#define CHECK_SETTINGS(browser,expected) _check_settings(browser, expected, __LINE__) +static void _check_settings(IExplorerBrowser *browser, FOLDERSETTINGS expected, int line) +{ + FOLDERSETTINGS settings; + IShellView *view; + HRESULT hr; + + hr = IExplorerBrowser_GetCurrentView(browser, &IID_IShellView, (void **)&view); + ok_(__FILE__,line)(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IShellView_GetCurrentInfo(view, &settings); + ok_(__FILE__,line)(hr == S_OK, "Got hr %#lx.\n", hr); + ok_(__FILE__,line)(!memcmp(&settings, &expected, sizeof(settings)), + "Got settings {view mode: %d, flags: %#x}, expected {view mode: %d, flags: %#x}.\n", + settings.ViewMode, settings.fFlags, expected.ViewMode, expected.fFlags); + IShellView_Release(view); +} + static void test_folder_settings(void) { IExplorerBrowser *browser; @@ -1702,7 +1719,38 @@ static void test_folder_settings(void) settings.ViewMode = 0; settings.fFlags = 0; hr = IExplorerBrowser_SetFolderSettings(browser, &settings); - todo_wine ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + + settings.ViewMode = FVM_ICON; settings.fFlags = FWF_SNAPTOGRID; + hr = IExplorerBrowser_SetFolderSettings(browser, &settings); + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + + ebrowser_browse_to_desktop(browser); + + settings.ViewMode = FVM_AUTO; settings.fFlags = FWF_AUTOARRANGE; + hr = IExplorerBrowser_SetFolderSettings(browser, &settings); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(settings.ViewMode == FVM_AUTO && settings.fFlags == FWF_AUTOARRANGE, + "Got view mode %d, flags %#x.\n", settings.ViewMode, settings.fFlags); + settings.ViewMode = FVM_ICON; settings.fFlags = FWF_AUTOARRANGE; + CHECK_SETTINGS(browser, settings); + + settings.ViewMode = FVM_LIST; settings.fFlags = FWF_AUTOARRANGE; + hr = IExplorerBrowser_SetFolderSettings(browser, &settings); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + settings.ViewMode = 0; settings.fFlags = FWF_SNAPTOGRID; + hr = IExplorerBrowser_SetFolderSettings(browser, &settings); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(settings.ViewMode == 0 && settings.fFlags == FWF_SNAPTOGRID, + "Got view mode %d, flags %#x.\n", settings.ViewMode, settings.fFlags); + settings.ViewMode = FVM_LIST; + CHECK_SETTINGS(browser, settings); + + settings.ViewMode = FVM_LAST + 1; settings.fFlags = FWF_AUTOARRANGE; + hr = IExplorerBrowser_SetFolderSettings(browser, &settings); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + settings.ViewMode = FVM_LIST; + CHECK_SETTINGS(browser, settings); IExplorerBrowser_Release(browser); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2628
From: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/shell32/ebrowser.c | 1 + dlls/shell32/tests/ebrowser.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/dlls/shell32/ebrowser.c b/dlls/shell32/ebrowser.c index 59bdd6a5d74..09122746712 100644 --- a/dlls/shell32/ebrowser.c +++ b/dlls/shell32/ebrowser.c @@ -362,6 +362,7 @@ static HRESULT create_new_shellview(ExplorerBrowserImpl *This, IShellItem *psi) This->hwnd_sv = NULL; } + This->fs.fFlags |= FWF_USESEARCHFOLDER | FWF_FULLROWSELECT | FWF_NOCLIENTEDGE; hr = IShellView_CreateViewWindow(psv, This->psv, &This->fs, psb, &This->sv_rc, &hwnd_new); if(SUCCEEDED(hr)) { diff --git a/dlls/shell32/tests/ebrowser.c b/dlls/shell32/tests/ebrowser.c index c1fdbeb3364..2fb513c3d99 100644 --- a/dlls/shell32/tests/ebrowser.c +++ b/dlls/shell32/tests/ebrowser.c @@ -1726,6 +1726,8 @@ static void test_folder_settings(void) ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); ebrowser_browse_to_desktop(browser); + settings.fFlags |= FWF_USESEARCHFOLDER | FWF_FULLROWSELECT | FWF_NOCLIENTEDGE; + CHECK_SETTINGS(browser, settings); settings.ViewMode = FVM_AUTO; settings.fFlags = FWF_AUTOARRANGE; hr = IExplorerBrowser_SetFolderSettings(browser, &settings); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2628
Nikolay Sivov (@nsivov) commented about dlls/shell32/tests/shlview.c:
+ { FWF_NONE, FWF_AUTOARRANGE, FWF_USESEARCHFOLDER }, + { FWF_AUTOARRANGE, FWF_AUTOARRANGE, FWF_USESEARCHFOLDER | FWF_AUTOARRANGE }, + { FWF_ABBREVIATEDNAMES, FWF_ABBREVIATEDNAMES | FWF_NOICONS, + FWF_USESEARCHFOLDER | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_ABBREVIATEDNAMES | FWF_NOICONS, FWF_ABBREVIATEDNAMES | FWF_NOVISIBLE, + FWF_USESEARCHFOLDER | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NOICONS, FWF_ABBREVIATEDNAMES | FWF_NOICONS, + FWF_USESEARCHFOLDER | FWF_NOICONS | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NONE, FWF_NONE, + FWF_USESEARCHFOLDER | FWF_NOICONS | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NOICONS, FWF_ABBREVIATEDNAMES | FWF_NOICONS, + FWF_USESEARCHFOLDER | FWF_NOICONS | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { FWF_NOICONS, FWF_NONE, FWF_USESEARCHFOLDER | FWF_AUTOARRANGE | FWF_ABBREVIATEDNAMES }, + { ~FWF_NONE, FWF_NONE, FWF_NONE }, + { FWF_NOICONS, FWF_NONE, FWF_NONE }, + { FWF_NONE, FWF_AUTOARRANGE, FWF_NONE }, This isn't great, because later tests depend on previous ones.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29580
Nikolay Sivov (@nsivov) commented about dlls/shell32/shlview.c:
static HRESULT WINAPI FolderView2_SetCurrentFolderFlags(IFolderView2 *iface, DWORD mask, DWORD flags) { IShellViewImpl *This = impl_from_IFolderView2(iface); - FIXME("(%p)->(0x%08lx 0x%08lx), stub\n", This, mask, flags); - return E_NOTIMPL; + + TRACE("(%p)->(%#lx %#lx).\n", This, mask, flags); Since you're changing this, let's improve formatting a bit, by getting rid of This (replace with 'view', 'shellview', 'folderview', etc), and using iface pointer in trace messages, with other arguments simply being comma-separated. So no braces or "->".
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29581
Nikolay Sivov (@nsivov) commented about dlls/shell32/shlview.c:
static HRESULT WINAPI FolderView2_SetCurrentFolderFlags(IFolderView2 *iface, DWORD mask, DWORD flags) { IShellViewImpl *This = impl_from_IFolderView2(iface); - FIXME("(%p)->(0x%08lx 0x%08lx), stub\n", This, mask, flags); - return E_NOTIMPL; + + TRACE("(%p)->(%#lx %#lx).\n", This, mask, flags); + + if (flags == FWF_NONE) + This->FolderSettings.fFlags &= ~mask; + else + This->FolderSettings.fFlags |= flags & mask; That's not how this usually works. Mask is used to specify flags that you want to change. So first you would unset bits that are in a mask but not in flags, and then set bits that are in mask and flags.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29582
Nikolay Sivov (@nsivov) commented about dlls/shell32/shlview.c:
+ + if (flags == FWF_NONE) + This->FolderSettings.fFlags &= ~mask; + else + This->FolderSettings.fFlags |= flags & mask; + + return S_OK; }
static HRESULT WINAPI FolderView2_GetCurrentFolderFlags(IFolderView2 *iface, DWORD *flags) { IShellViewImpl *This = impl_from_IFolderView2(iface); - FIXME("(%p)->(%p), stub\n", This, flags); - return E_NOTIMPL; + + TRACE("(%p)->(%p).\n", This, flags); Same regarding formatting here, and in other places.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29583
Nikolay Sivov (@nsivov) commented about dlls/shell32/tests/shlview.c:
}
hr = IFolderView_SetCurrentViewMode(fview, 9); - ok(hr == E_INVALIDARG || broken(hr == S_OK), - "got (0x%08lx)\n", hr); + ok(hr == S_OK || broken(hr == E_INVALIDARG) /* pre win7 */, "Got hr %#lx.\n", hr); + ok(viewmode == FVM_CONTENT || broken(viewmode == FVM_THUMBSTRIP) /* vista */, "Got view mode %d.\n", viewmode);
Is this missing a call to GetCurrentViewMode()? So the conclusion here is that any larger number gives last valid mode? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29584
Nikolay Sivov (@nsivov) commented about dlls/shell32/ebrowser.c:
- return change_viewmode(This, This->fs.ViewMode); + if (!This->psv) + return E_INVALIDARG; + + hr = IShellView_QueryInterface(This->psv, &IID_IFolderView2, (void *)&view); + if (SUCCEEDED(hr)) + { + hr = IFolderView2_SetCurrentViewMode(view, This->fs.ViewMode); + if (SUCCEEDED(hr)) + { + if (SUCCEEDED(hr)) + { + hr = IFolderView2_SetCurrentFolderFlags(view, ~FWF_NONE, FWF_NONE); + if (SUCCEEDED(hr)) + hr = IFolderView2_SetCurrentFolderFlags(view, This->fs.fFlags, This->fs.fFlags); + } My earlier comments affect this too. We need some more tests for setting flags, but I suspect you will simply need to call as (~0u, This->fs.flags).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29585
On Tue Apr 11 11:37:34 2023 +0000, Nikolay Sivov wrote:
Since you're changing this, let's improve formatting a bit, by getting rid of This (replace with 'view', 'shellview', 'folderview', etc), and using iface pointer in trace messages, with other arguments simply being comma-separated. So no braces or "->". I never understood the "new format" for traces, using `iface`. Is there a reason for it? It makes looking through logs harder for no reason at all. I much prefer tracing `This` (the object itself) so that I can search for the same pointer value to find use of that object in logs. Old-style traces are much easier for me to debug.
You already know which iface it's using purely from the function name in the log, anyway… -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29601
Nikolay Sivov (@nsivov) commented about dlls/shell32/ebrowser.c:
- if(!pfs) + if (!pfs) return E_INVALIDARG;
- This->fs.ViewMode = pfs->ViewMode; - This->fs.fFlags = pfs->fFlags | FWF_NOCLIENTEDGE; + if (pfs->ViewMode) + This->fs.ViewMode = pfs->ViewMode; + This->fs.fFlags = pfs->fFlags;
- /* Change the settings of the current view, if any. */ - return change_viewmode(This, This->fs.ViewMode); + if (!This->psv) + return E_INVALIDARG; + + hr = IShellView_QueryInterface(This->psv, &IID_IFolderView2, (void *)&view); This one should use void**.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29623
On Tue Apr 11 11:37:33 2023 +0000, Nikolay Sivov wrote:
This isn't great, because later tests depend on previous ones. Hi Nikolay,
It was intentional to do so. Because these tests can show that SetCurrentFolderFlags() doesn't reset its builtin flags value when it is called unless we call SetCurrentFolderFlags() to do a reset first. This means SetCurrentFolderFlags() doesn't simply apply the new flags, it will compute a new value with the current flags and given mask and flags. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2628#note_29682
participants (4)
-
Gabriel Ivăncescu -
Jactry Zeng
-
Jactry Zeng (＠jactry)
-
Nikolay Sivov (＠nsivov)