From 3693d55404d6ffa64dc515829ed3bb13a8336b0d Mon Sep 17 00:00:00 2001 From: Doug Lyons Date: Thu, 25 Apr 2024 17:20:28 -0500 Subject: [PATCH] [SHELL32] Fix Desktop folder details view (#5743) * [SHELL32] Fix Desktop Folder Details View JIRA issue: [CORE-19177|https://jira.reactos.org/browse/CORE-19177] * Remove Comments column from Desktop Folder Details View and simplify code. * Revise date/time sort based on reviewer comments. * Swap size and type column positions for desktop folder details view. With help from Whindmar, most (hopefully all) of the magic number for the columns have been removed in all of the shell folders. Co-authored-by: Whindmar Saksit Co-authored-by: Carl J. Bialorucki --- .../shell32/folders/CControlPanelFolder.cpp | 2 +- dll/win32/shell32/folders/CDesktopFolder.cpp | 47 ++++++---- dll/win32/shell32/folders/CDesktopFolder.h | 2 + dll/win32/shell32/folders/CDrivesFolder.cpp | 4 +- dll/win32/shell32/folders/CFSFolder.cpp | 93 ++++++++++++------- dll/win32/shell32/folders/CFSFolder.h | 5 + dll/win32/shell32/folders/CNetFolder.cpp | 4 +- dll/win32/shell32/folders/CPrinterFolder.cpp | 4 +- dll/win32/shell32/folders/CRegFolder.cpp | 34 ++++--- dll/win32/shell32/shfldr.h | 22 ++++- 10 files changed, 140 insertions(+), 77 deletions(-) diff --git a/dll/win32/shell32/folders/CControlPanelFolder.cpp b/dll/win32/shell32/folders/CControlPanelFolder.cpp index 7306862f966..58d5c36353c 100644 --- a/dll/win32/shell32/folders/CControlPanelFolder.cpp +++ b/dll/win32/shell32/folders/CControlPanelFolder.cpp @@ -550,7 +550,7 @@ HRESULT WINAPI CControlPanelFolder::GetDefaultColumnState(UINT iColumn, DWORD *p if (!pcsFlags || iColumn >= CONTROLPANEL_COL_COUNT) return E_INVALIDARG; - *pcsFlags = ControlPanelSFHeader[iColumn].pcsFlags; + *pcsFlags = ControlPanelSFHeader[iColumn].colstate; return S_OK; } diff --git a/dll/win32/shell32/folders/CDesktopFolder.cpp b/dll/win32/shell32/folders/CDesktopFolder.cpp index 54f3206ef5c..ade57b69fce 100644 --- a/dll/win32/shell32/folders/CDesktopFolder.cpp +++ b/dll/win32/shell32/folders/CDesktopFolder.cpp @@ -21,6 +21,7 @@ */ #include +#include "CFSFolder.h" // Only for CFSFolder::*FSColumn* helpers! WINE_DEFAULT_DEBUG_CHANNEL(shell); @@ -284,17 +285,6 @@ class CDesktopFolderEnum : int SHELL_ConfirmMsgBox(HWND hWnd, LPWSTR lpszText, LPWSTR lpszCaption, HICON hIcon, BOOL bYesToAll); -static const shvheader DesktopSFHeader[] = { - {IDS_SHV_COLUMN_NAME, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 15}, - {IDS_SHV_COLUMN_COMMENTS, SHCOLSTATE_TYPE_STR, LVCFMT_LEFT, 10}, - {IDS_SHV_COLUMN_TYPE, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 10}, - {IDS_SHV_COLUMN_SIZE, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_RIGHT, 10}, - {IDS_SHV_COLUMN_MODIFIED, SHCOLSTATE_TYPE_DATE | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 12}, - {IDS_SHV_COLUMN_ATTRIBUTES, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 10} -}; - -#define DESKTOPSHELLVIEWCOLUMNS 6 - static const DWORD dwDesktopAttributes = SFGAO_HASSUBFOLDER | SFGAO_FILESYSTEM | SFGAO_FOLDER | SFGAO_FILESYSANCESTOR | SFGAO_STORAGEANCESTOR | SFGAO_HASPROPSHEET | SFGAO_STORAGE; @@ -978,16 +968,25 @@ HRESULT WINAPI CDesktopFolder::GetDefaultColumn(DWORD dwRes, ULONG *pSort, ULONG return S_OK; } -HRESULT WINAPI CDesktopFolder::GetDefaultColumnState(UINT iColumn, DWORD *pcsFlags) +HRESULT WINAPI CDesktopFolder::GetDefaultColumnState(UINT iColumn, SHCOLSTATEF *pcsFlags) { + HRESULT hr; TRACE ("(%p)\n", this); - if (!pcsFlags || iColumn >= DESKTOPSHELLVIEWCOLUMNS) + if (!pcsFlags) return E_INVALIDARG; - *pcsFlags = DesktopSFHeader[iColumn].pcsFlags; - - return S_OK; + hr = CFSFolder::GetDefaultFSColumnState(iColumn, *pcsFlags); + /* + // CDesktopFolder may override the flags if desired (future) + switch(iColumn) + { + case SHFSF_COL_FATTS: + *pcsFlags &= ~SHCOLSTATE_ONBYDEFAULT; + break; + } + */ + return hr; } HRESULT WINAPI CDesktopFolder::GetDetailsEx( @@ -1000,19 +999,27 @@ HRESULT WINAPI CDesktopFolder::GetDetailsEx( return E_NOTIMPL; } +/************************************************************************* + * Column info functions. + * CFSFolder.h provides defaults for us. + */ +HRESULT CDesktopFolder::GetColumnDetails(UINT iColumn, SHELLDETAILS &sd) +{ + /* CDesktopFolder may override the flags and/or name if desired */ + return CFSFolder::GetFSColumnDetails(iColumn, sd); +} + HRESULT WINAPI CDesktopFolder::GetDetailsOf( PCUITEMID_CHILD pidl, UINT iColumn, SHELLDETAILS *psd) { - if (!psd || iColumn >= DESKTOPSHELLVIEWCOLUMNS) + if (!psd) return E_INVALIDARG; if (!pidl) { - psd->fmt = DesktopSFHeader[iColumn].fmt; - psd->cxChar = DesktopSFHeader[iColumn].cxChar; - return SHSetStrRet(&psd->str, DesktopSFHeader[iColumn].colnameid); + return GetColumnDetails(iColumn, *psd); } CComPtr psf; diff --git a/dll/win32/shell32/folders/CDesktopFolder.h b/dll/win32/shell32/folders/CDesktopFolder.h index 7a9b498d240..66eda8df0ee 100644 --- a/dll/win32/shell32/folders/CDesktopFolder.h +++ b/dll/win32/shell32/folders/CDesktopFolder.h @@ -42,6 +42,8 @@ class CDesktopFolder : HRESULT _GetSFFromPidl(LPCITEMIDLIST pidl, IShellFolder2** psf); + static HRESULT GetColumnDetails(UINT iColumn, SHELLDETAILS &sd); + HRESULT _ParseDisplayNameByParent( HWND hwndOwner, LPBC pbc, diff --git a/dll/win32/shell32/folders/CDrivesFolder.cpp b/dll/win32/shell32/folders/CDrivesFolder.cpp index 04d0c080814..748109331b6 100644 --- a/dll/win32/shell32/folders/CDrivesFolder.cpp +++ b/dll/win32/shell32/folders/CDrivesFolder.cpp @@ -1168,13 +1168,13 @@ HRESULT WINAPI CDrivesFolder::GetDefaultColumn (DWORD dwRes, ULONG *pSort, ULONG return S_OK; } -HRESULT WINAPI CDrivesFolder::GetDefaultColumnState(UINT iColumn, DWORD * pcsFlags) +HRESULT WINAPI CDrivesFolder::GetDefaultColumnState(UINT iColumn, SHCOLSTATEF * pcsFlags) { TRACE("(%p)\n", this); if (!pcsFlags || iColumn >= _countof(MyComputerSFHeader)) return E_INVALIDARG; - *pcsFlags = MyComputerSFHeader[iColumn].pcsFlags; + *pcsFlags = MyComputerSFHeader[iColumn].colstate; return S_OK; } diff --git a/dll/win32/shell32/folders/CFSFolder.cpp b/dll/win32/shell32/folders/CFSFolder.cpp index f6546dc23bf..2ec03d18298 100644 --- a/dll/win32/shell32/folders/CFSFolder.cpp +++ b/dll/win32/shell32/folders/CFSFolder.cpp @@ -527,17 +527,35 @@ CFSFolder::~CFSFolder() SHFree(m_sPathTarget); } - static const shvheader GenericSFHeader[] = { {IDS_SHV_COLUMN_NAME, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 15}, + {IDS_SHV_COLUMN_SIZE, SHCOLSTATE_TYPE_INT | SHCOLSTATE_ONBYDEFAULT, LVCFMT_RIGHT, 10}, {IDS_SHV_COLUMN_TYPE, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 10}, - {IDS_SHV_COLUMN_SIZE, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_RIGHT, 10}, - {IDS_SHV_COLUMN_MODIFIED, SHCOLSTATE_TYPE_DATE | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 12}, + {IDS_SHV_COLUMN_MODIFIED, SHCOLSTATE_TYPE_DATE | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 15}, {IDS_SHV_COLUMN_ATTRIBUTES, SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT, LVCFMT_LEFT, 10}, - {IDS_SHV_COLUMN_COMMENTS, SHCOLSTATE_TYPE_STR, LVCFMT_LEFT, 10} + {IDS_SHV_COLUMN_COMMENTS, SHCOLSTATE_TYPE_STR | SHCOLSTATE_SLOW, LVCFMT_LEFT, 10}, // We don't currently support comments but CRegFolder does }; -#define GENERICSHELLVIEWCOLUMNS 6 +#define GENERICSHELLVIEWCOLUMNS _countof(GenericSFHeader) + +HRESULT CFSFolder::GetFSColumnDetails(UINT iColumn, SHELLDETAILS &sd) +{ + if (iColumn >= _countof(GenericSFHeader)) + return E_INVALIDARG; + + sd.fmt = GenericSFHeader[iColumn].fmt; + sd.cxChar = GenericSFHeader[iColumn].cxChar; + return SHSetStrRet(&sd.str, GenericSFHeader[iColumn].colnameid); +} + +HRESULT CFSFolder::GetDefaultFSColumnState(UINT iColumn, SHCOLSTATEF &csFlags) +{ + if (iColumn >= _countof(GenericSFHeader)) + return E_INVALIDARG; + + csFlags = GenericSFHeader[iColumn].colstate; + return S_OK; +} static HRESULT SHELL32_GetCLSIDForDirectory(LPCWSTR pwszDir, LPCWSTR KeyName, CLSID* pclsidFolder) { @@ -1021,18 +1039,13 @@ HRESULT WINAPI CFSFolder::CompareIDs(LPARAM lParam, return MAKE_COMPARE_HRESULT(bIsFolder1 ? -1 : 1); } - int result; + int result = 0; switch (LOWORD(lParam)) { - case 0: /* Name */ + case SHFSF_COL_NAME: result = wcsicmp(pDataW1->wszName, pDataW2->wszName); break; - case 1: /* Type */ - pExtension1 = PathFindExtensionW(pDataW1->wszName); - pExtension2 = PathFindExtensionW(pDataW2->wszName); - result = wcsicmp(pExtension1, pExtension2); - break; - case 2: /* Size */ + case SHFSF_COL_SIZE: if (pData1->u.file.dwFileSize > pData2->u.file.dwFileSize) result = 1; else if (pData1->u.file.dwFileSize < pData2->u.file.dwFileSize) @@ -1040,16 +1053,26 @@ HRESULT WINAPI CFSFolder::CompareIDs(LPARAM lParam, else result = 0; break; - case 3: /* Modified */ + case SHFSF_COL_TYPE: + pExtension1 = PathFindExtensionW(pDataW1->wszName); + pExtension2 = PathFindExtensionW(pDataW2->wszName); + result = wcsicmp(pExtension1, pExtension2); + break; + case SHFSF_COL_MDATE: result = pData1->u.file.uFileDate - pData2->u.file.uFileDate; if (result == 0) result = pData1->u.file.uFileTime - pData2->u.file.uFileTime; break; - case 4: /* Attributes */ + case SHFSF_COL_FATTS: return SHELL32_CompareDetails(this, lParam, pidl1, pidl2); - case 5: /* Comments */ + case SHFSF_COL_COMMENT: result = 0; break; + default: + if (_ILIsPidlSimple(pidl1) || _ILIsPidlSimple(pidl2)) + ERR("Unknown column %u, can't compare\n", LOWORD(lParam)); + else + TRACE("Unknown column %u, deferring to the subfolder\n", LOWORD(lParam)); } if (result == 0) @@ -1543,16 +1566,14 @@ HRESULT WINAPI CFSFolder::GetDefaultColumn(DWORD dwRes, } HRESULT WINAPI CFSFolder::GetDefaultColumnState(UINT iColumn, - DWORD * pcsFlags) + SHCOLSTATEF *pcsFlags) { TRACE ("(%p)\n", this); - if (!pcsFlags || iColumn >= GENERICSHELLVIEWCOLUMNS) + if (!pcsFlags) return E_INVALIDARG; - - *pcsFlags = GenericSFHeader[iColumn].pcsFlags; - - return S_OK; + else + return GetDefaultFSColumnState(iColumn, *pcsFlags); } HRESULT WINAPI CFSFolder::GetDetailsEx(PCUITEMID_CHILD pidl, @@ -1576,9 +1597,7 @@ HRESULT WINAPI CFSFolder::GetDetailsOf(PCUITEMID_CHILD pidl, if (!pidl) { /* the header titles */ - psd->fmt = GenericSFHeader[iColumn].fmt; - psd->cxChar = GenericSFHeader[iColumn].cxChar; - return SHSetStrRet(&psd->str, GenericSFHeader[iColumn].colnameid); + return GetFSColumnDetails(iColumn, *psd); } else { @@ -1587,24 +1606,30 @@ HRESULT WINAPI CFSFolder::GetDetailsOf(PCUITEMID_CHILD pidl, /* the data from the pidl */ switch (iColumn) { - case 0: /* name */ + case SHFSF_COL_NAME: hr = GetDisplayNameOf (pidl, SHGDN_NORMAL | SHGDN_INFOLDER, &psd->str); break; - case 1: /* type */ - _ILGetFileType(pidl, psd->str.cStr, MAX_PATH); - break; - case 2: /* size */ + case SHFSF_COL_SIZE: _ILGetFileSize(pidl, psd->str.cStr, MAX_PATH); break; - case 3: /* date */ + case SHFSF_COL_TYPE: + _ILGetFileType(pidl, psd->str.cStr, MAX_PATH); + break; + case SHFSF_COL_MDATE: _ILGetFileDate(pidl, psd->str.cStr, MAX_PATH); break; - case 4: /* attributes */ + case SHFSF_COL_FATTS: _ILGetFileAttributes(pidl, psd->str.cStr, MAX_PATH); break; - case 5: /* FIXME: comments */ - psd->str.cStr[0] = 0; + case SHFSF_COL_COMMENT: + psd->str.cStr[0] = '\0'; // TODO: Extract comment from .lnk files? desktop.ini? break; +#if DBG + default: + ERR("Missing case for column %d\n", iColumn); +#else + DEFAULT_UNREACHABLE; +#endif } } diff --git a/dll/win32/shell32/folders/CFSFolder.h b/dll/win32/shell32/folders/CFSFolder.h index 8d29c4d3479..d74c7d915a2 100644 --- a/dll/win32/shell32/folders/CFSFolder.h +++ b/dll/win32/shell32/folders/CFSFolder.h @@ -125,6 +125,11 @@ class CFSFolder : protected: HRESULT WINAPI GetCustomViewInfo(ULONG unknown, SFVM_CUSTOMVIEWINFO_DATA *data); + + public: + // Helper functions shared with CDesktopFolder + static HRESULT GetFSColumnDetails(UINT iColumn, SHELLDETAILS &sd); + static HRESULT GetDefaultFSColumnState(UINT iColumn, SHCOLSTATEF &csFlags); }; #endif /* _CFSFOLDER_H_ */ diff --git a/dll/win32/shell32/folders/CNetFolder.cpp b/dll/win32/shell32/folders/CNetFolder.cpp index 5479deddb1a..4b1eb3c15ef 100644 --- a/dll/win32/shell32/folders/CNetFolder.cpp +++ b/dll/win32/shell32/folders/CNetFolder.cpp @@ -510,13 +510,13 @@ HRESULT WINAPI CNetFolder::GetDefaultColumn (DWORD dwRes, ULONG *pSort, ULONG *p return S_OK; } -HRESULT WINAPI CNetFolder::GetDefaultColumnState(UINT iColumn, DWORD *pcsFlags) +HRESULT WINAPI CNetFolder::GetDefaultColumnState(UINT iColumn, SHCOLSTATEF *pcsFlags) { TRACE("(%p)\n", this); if (!pcsFlags || iColumn >= NETWORKPLACESSHELLVIEWCOLUMNS) return E_INVALIDARG; - *pcsFlags = NetworkPlacesSFHeader[iColumn].pcsFlags; + *pcsFlags = NetworkPlacesSFHeader[iColumn].colstate; return S_OK; } diff --git a/dll/win32/shell32/folders/CPrinterFolder.cpp b/dll/win32/shell32/folders/CPrinterFolder.cpp index 43050b625c9..8d9553117f6 100644 --- a/dll/win32/shell32/folders/CPrinterFolder.cpp +++ b/dll/win32/shell32/folders/CPrinterFolder.cpp @@ -432,11 +432,11 @@ HRESULT WINAPI CPrinterFolder::GetDefaultColumn(DWORD dwRes, ULONG *pSort, ULONG return S_OK; } -HRESULT WINAPI CPrinterFolder::GetDefaultColumnState(UINT iColumn, DWORD *pcsFlags) +HRESULT WINAPI CPrinterFolder::GetDefaultColumnState(UINT iColumn, SHCOLSTATEF *pcsFlags) { if (!pcsFlags || iColumn >= PrinterSHELLVIEWCOLUMNS) return E_INVALIDARG; - *pcsFlags = PrinterSFHeader[iColumn].pcsFlags; + *pcsFlags = PrinterSFHeader[iColumn].colstate; return S_OK; } diff --git a/dll/win32/shell32/folders/CRegFolder.cpp b/dll/win32/shell32/folders/CRegFolder.cpp index 57e0dc9aa3a..9a74ab6fa7a 100644 --- a/dll/win32/shell32/folders/CRegFolder.cpp +++ b/dll/win32/shell32/folders/CRegFolder.cpp @@ -233,14 +233,6 @@ class CRegFolderEnum : END_COM_MAP() }; -enum registry_columns -{ - REGISTRY_COL_NAME, - REGISTRY_COL_TYPE, - REGISTRY_COL_VALUE, - REGISTRY_COL_COUNT, -}; - CRegFolderEnum::CRegFolderEnum() { } @@ -302,6 +294,18 @@ HRESULT CRegFolderEnum::AddItemsFromKey(HKEY hkey_root, LPCWSTR szRepPath) return S_OK; } +/* + * These columns try to map to CFSFolder's columns because the CDesktopFolder + * displays CFSFolder and CRegFolder items in the same view. + */ +enum REGFOLDERCOLUMNINDEX +{ + COL_NAME = SHFSF_COL_NAME, + COL_TYPE = SHFSF_COL_TYPE, + COL_INFOTIP = SHFSF_COL_COMMENT, + REGFOLDERCOLUMNCOUNT = max(COL_INFOTIP, COL_TYPE) + 1 +}; + class CRegFolder : public CComObjectRootEx, public IShellFolder2 @@ -808,7 +812,7 @@ HRESULT WINAPI CRegFolder::GetDefaultColumn(DWORD dwRes, ULONG *pSort, ULONG *pD HRESULT WINAPI CRegFolder::GetDefaultColumnState(UINT iColumn, DWORD *pcsFlags) { - if (iColumn >= REGISTRY_COL_COUNT) + if (iColumn >= REGFOLDERCOLUMNCOUNT) return E_INVALIDARG; *pcsFlags = SHCOLSTATE_TYPE_STR | SHCOLSTATE_ONBYDEFAULT; return S_OK; @@ -824,6 +828,12 @@ HRESULT WINAPI CRegFolder::GetDetailsOf(PCUITEMID_CHILD pidl, UINT iColumn, SHEL if (!psd) return E_INVALIDARG; + if (!pidl) + { + TRACE("CRegFolder has no column info\n"); + return E_INVALIDARG; + } + GUID const *clsid = _ILGetGUIDPointer (pidl); if (!clsid) @@ -834,11 +844,11 @@ HRESULT WINAPI CRegFolder::GetDetailsOf(PCUITEMID_CHILD pidl, UINT iColumn, SHEL switch(iColumn) { - case REGISTRY_COL_NAME: + case COL_NAME: return GetDisplayNameOf(pidl, SHGDN_NORMAL | SHGDN_INFOLDER, &psd->str); - case REGISTRY_COL_TYPE: + case COL_TYPE: return SHSetStrRet(&psd->str, IDS_SYSTEMFOLDER); - case REGISTRY_COL_VALUE: + case COL_INFOTIP: HKEY hKey; if (!HCR_RegOpenClassIDKey(*clsid, &hKey)) return SHSetStrRet(&psd->str, ""); diff --git a/dll/win32/shell32/shfldr.h b/dll/win32/shell32/shfldr.h index 64e236a5d73..f2b1de470cc 100644 --- a/dll/win32/shell32/shfldr.h +++ b/dll/win32/shell32/shfldr.h @@ -27,11 +27,25 @@ #define CHARS_IN_GUID 39 typedef struct { - int colnameid; - int pcsFlags; - int fmt; - int cxChar; + WORD colnameid; // Column title text resource id passed to LoadString + WORD colstate; // SHCOLSTATEF returned by IShellFolder2::GetDefaultColumnState + // HACK: SHCOLSTATEF truncated to WORD to reduce .rdata section size + WORD fmt; // LVCFMT_* + WORD cxChar; // Column width hint } shvheader; +/* + * CFSFolder column indices. CDesktopFolder MUST use the same indices! + * According to the documentation for IShellFolder2::GetDetailsOf, + * the first 4 columns for SFGAO_FILESYSTEM items must be Name, Size, Type, Modified date +For Details See: +https://learn.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-ishellfolder2-getdetailsof + */ +#define SHFSF_COL_NAME 0 +#define SHFSF_COL_SIZE 1 +#define SHFSF_COL_TYPE 2 // SHGFI_TYPENAME +#define SHFSF_COL_MDATE 3 // Modified date +#define SHFSF_COL_FATTS 4 // File attributes +#define SHFSF_COL_COMMENT 5 #define GET_SHGDN_FOR(dwFlags) ((DWORD)dwFlags & (DWORD)0x0000FF00) #define GET_SHGDN_RELATION(dwFlags) ((DWORD)dwFlags & (DWORD)0x000000FF)