[SHELL32] Correctly compare pidls for SHBrowseForFolder BFFM_SETEXPANDED (reactos#7499)

_ILIsEqualSimple just does a memcmp and that does not work for all items.

Should improve shell change notifications (CORE-13950).

Author: whindsaks

dll/win32/shell32/CDefView.cpp

@@ -4608,9 +4608,9 @@ HRESULT WINAPI CDefView::GetAdvise(DWORD *pAspects, DWORD *pAdvf, IAdviseSink **
 
 HRESULT STDMETHODCALLTYPE CDefView::QueryService(REFGUID guidService, REFIID riid, void **ppvObject)
 {
-    if (IsEqualIID(guidService, SID_IShellBrowser))
+    if (IsEqualIID(guidService, SID_IShellBrowser) && m_pShellBrowser)
         return m_pShellBrowser->QueryInterface(riid, ppvObject);
-    else if(IsEqualIID(guidService, SID_IFolderView))
+    else if (IsEqualIID(guidService, SID_IFolderView))
         return QueryInterface(riid, ppvObject);
 
     return E_NOINTERFACE;

dll/win32/shell32/brfolder.cpp

@@ -11,11 +11,36 @@
 #include "precomp.h"
 
 #include <ui/layout.h> // Resizable window
+#include <compat_undoc.h> // RosGetProcessEffectiveVersion
 
 WINE_DEFAULT_DEBUG_CHANNEL(shell);
 
 #define SHV_CHANGE_NOTIFY (WM_USER + 0x1111)
 
+static LPITEMIDLIST
+ILCloneToDepth(LPCITEMIDLIST pidlSrc, UINT depth)
+{
+    SIZE_T cb = 0;
+    for (LPCITEMIDLIST pidl = pidlSrc; pidl && depth--; pidl = ILGetNext(pidl))
+        cb += pidl->mkid.cb;
+
+    LPITEMIDLIST pidlOut = (LPITEMIDLIST)SHAlloc(cb + sizeof(WORD));
+    if (pidlOut)
+    {
+        CopyMemory(pidlOut, pidlSrc, cb);
+        ZeroMemory(((BYTE*)pidlOut) + cb, sizeof(WORD));
+    }
+    return pidlOut;
+}
+
+static INT
+GetIconIndex(PCIDLIST_ABSOLUTE pidl, UINT uFlags)
+{
+    SHFILEINFOW sfi;
+    SHGetFileInfoW((LPCWSTR)pidl, 0, &sfi, sizeof(sfi), uFlags);
+    return sfi.iIcon;
+}
+
 struct BrFolder
 {
     LPBROWSEINFOW    lpBrowseInfo;
@@ -145,12 +170,9 @@ BrFolder_InitTreeView(BrFolder *info)
 {
     HIMAGELIST hImageList;
     HRESULT hr;
-    CComPtr<IShellFolder> lpsfParent;
-    CComPtr<IEnumIDList> pEnumChildren;
     HTREEITEM hItem;
 
     Shell_GetImageLists(NULL, &hImageList);
-
     if (hImageList)
         TreeView_SetImageList(info->hwndTreeView, hImageList, 0);
 
@@ -171,54 +193,25 @@ BrFolder_InitTreeView(BrFolder *info)
     ILRemoveLastID(pidlParent);
     PCIDLIST_RELATIVE pidlChild = ILFindLastID(pidlRoot);
 
-    if (_ILIsDesktop(pidlParent))
-    {
-        hr = SHGetDesktopFolder(&lpsfParent);
-        if (FAILED_UNEXPECTEDLY(hr))
-            return;
-    }
-    else
-    {
-        CComPtr<IShellFolder> lpsfDesktop;
-        hr = SHGetDesktopFolder(&lpsfDesktop);
-        if (FAILED_UNEXPECTEDLY(hr))
-            return;
-
-        hr = lpsfDesktop->BindToObject(pidlParent, NULL, IID_PPV_ARG(IShellFolder, &lpsfParent));
-        if (FAILED_UNEXPECTEDLY(hr))
-            return;
-    }
+    CComPtr<IShellFolder> lpsfParent;
+    hr = SHBindToObject(NULL, pidlParent, /*NULL, */ IID_PPV_ARG(IShellFolder, &lpsfParent));
+    if (FAILED_UNEXPECTEDLY(hr))
+        return;
 
     TreeView_DeleteItem(info->hwndTreeView, TVI_ROOT);
     hItem = BrFolder_InsertItem(info, lpsfParent, pidlChild, pidlParent, TVI_ROOT);
     TreeView_Expand(info->hwndTreeView, hItem, TVE_EXPAND);
 }
 
-static INT
-BrFolder_GetIcon(PCIDLIST_ABSOLUTE pidl, UINT uFlags)
-{
-    SHFILEINFOW sfi;
-    SHGetFileInfoW((LPCWSTR)pidl, 0, &sfi, sizeof(sfi), uFlags);
-    return sfi.iIcon;
-}
-
 static void
 BrFolder_GetIconPair(PCIDLIST_ABSOLUTE pidl, LPTVITEMW pItem)
 {
-    DWORD flags;
-
-    CComHeapPtr<ITEMIDLIST_ABSOLUTE> pidlDesktop;
+    static const ITEMIDLIST idlDesktop = { };
     if (!pidl)
-    {
-        pidlDesktop.Attach(_ILCreateDesktop());
-        pidl = pidlDesktop;
-    }
-
-    flags = SHGFI_PIDL | SHGFI_SYSICONINDEX | SHGFI_SMALLICON;
-    pItem->iImage = BrFolder_GetIcon(pidl, flags);
-
-    flags = SHGFI_PIDL | SHGFI_SYSICONINDEX | SHGFI_SMALLICON | SHGFI_OPENICON;
-    pItem->iSelectedImage = BrFolder_GetIcon(pidl, flags);
+        pidl = &idlDesktop;
+    DWORD flags = SHGFI_PIDL | SHGFI_SYSICONINDEX | SHGFI_SMALLICON;
+    pItem->iImage = GetIconIndex(pidl, flags);
+    pItem->iSelectedImage = GetIconIndex(pidl, flags | SHGFI_OPENICON);
 }
 
 /******************************************************************************
@@ -766,6 +759,19 @@ BrFolder_OnInitDialog(HWND hWnd, BrFolder *info)
                                                  SHCNE_ALLEVENTS,
                                                  SHV_CHANGE_NOTIFY, 1, &ntreg);
 
+    if (!lpBrowseInfo->pidlRoot)
+    {
+        UINT csidl = (lpBrowseInfo->ulFlags & BIF_NEWDIALOGSTYLE) ? CSIDL_PERSONAL : CSIDL_DRIVES;
+        LPITEMIDLIST pidl = SHCloneSpecialIDList(NULL, csidl, TRUE);
+        if (pidl)
+        {
+            SendMessageW(info->hWnd, BFFM_SETSELECTION, FALSE, (LPARAM)pidl);
+            if (csidl == CSIDL_DRIVES)
+                SendMessageW(info->hWnd, BFFM_SETEXPANDED, FALSE, (LPARAM)pidl);
+            ILFree(pidl);
+        }
+    }
+
     BrFolder_Callback(info->lpBrowseInfo, hWnd, BFFM_INITIALIZED, 0);
 
     SHAutoComplete(GetDlgItem(hWnd, IDC_BROWSE_FOR_FOLDER_FOLDER_TEXT),
@@ -985,60 +991,56 @@ BrFolder_OnContextMenu(BrFolder &info, LPARAM lParam)
 }
 
 static BOOL
-BrFolder_OnSetExpandedPidl(BrFolder *info, LPITEMIDLIST pidlSelection, HTREEITEM *phItem)
+BrFolder_ExpandToPidl(BrFolder *info, LPITEMIDLIST pidlSelection, HTREEITEM *phItem)
 {
+    LPITEMIDLIST pidlCurrent = pidlSelection;
     if (_ILIsDesktop(pidlSelection))
     {
         if (phItem)
             *phItem = TVI_ROOT;
         return TRUE;
     }
 
-    // Move pidlCurrent behind the SHITEMIDs in pidlSelection, which are the root of
-    // the sub-tree currently displayed.
-    PCIDLIST_ABSOLUTE pidlRoot = info->lpBrowseInfo->pidlRoot;
-    LPITEMIDLIST pidlCurrent = pidlSelection;
-    while (!_ILIsEmpty(pidlRoot) && _ILIsEqualSimple(pidlRoot, pidlCurrent))
-    {
-        pidlRoot = ILGetNext(pidlRoot);
-        pidlCurrent = ILGetNext(pidlCurrent);
-    }
-
-    // The given ID List is not part of the SHBrowseForFolder's current sub-tree.
-    if (!_ILIsEmpty(pidlRoot))
-    {
-        if (phItem)
-            *phItem = NULL;
-        return FALSE;
-    }
-
     // Initialize item to point to the first child of the root folder.
     TVITEMEXW item = { TVIF_PARAM };
     item.hItem = TreeView_GetRoot(info->hwndTreeView);
     if (item.hItem)
         item.hItem = TreeView_GetChild(info->hwndTreeView, item.hItem);
 
     // Walk the tree along the nodes corresponding to the remaining ITEMIDLIST
-    while (item.hItem && !_ILIsEmpty(pidlCurrent))
+    UINT depth = _ILGetDepth(info->lpBrowseInfo->pidlRoot);
+    while (item.hItem && pidlCurrent)
     {
+        LPITEMIDLIST pidlNeedle = ILCloneToDepth(pidlSelection, ++depth);
+        if (_ILIsEmpty(pidlNeedle))
+        {
+            ILFree(pidlNeedle);
+            item.hItem = NULL; // Failure
+            break;
+        }
+next:
         TreeView_GetItem(info->hwndTreeView, &item);
-        BrItemData *pItemData = (BrItemData *)item.lParam;
-
-        if (_ILIsEqualSimple(pItemData->pidlChild, pidlCurrent))
+        const BrItemData *pItemData = (BrItemData *)item.lParam;
+        if (ILIsEqual(pItemData->pidlFull, pidlNeedle))
         {
-            pidlCurrent = ILGetNext(pidlCurrent);
-            if (!_ILIsEmpty(pidlCurrent))
+            BOOL done = _ILGetDepth(pidlSelection) == _ILGetDepth(pidlNeedle);
+            if (done)
+            {
+                pidlCurrent = NULL; // Success
+            }
+            else
             {
-                // Only expand current node and move on to its first child,
-                // if we didn't already reach the last SHITEMID
                 TreeView_Expand(info->hwndTreeView, item.hItem, TVE_EXPAND);
                 item.hItem = TreeView_GetChild(info->hwndTreeView, item.hItem);
             }
         }
         else
         {
             item.hItem = TreeView_GetNextSibling(info->hwndTreeView, item.hItem);
+            if (item.hItem)
+                goto next;
         }
+        ILFree(pidlNeedle);
     }
 
     if (phItem)
@@ -1048,19 +1050,26 @@ BrFolder_OnSetExpandedPidl(BrFolder *info, LPITEMIDLIST pidlSelection, HTREEITEM
 }
 
 static BOOL
-BrFolder_OnSetExpandedString(BrFolder *info, LPWSTR pszString, HTREEITEM *phItem)
+BrFolder_ExpandToString(BrFolder *info, LPWSTR pszString, HTREEITEM *phItem)
 {
-    CComPtr<IShellFolder> psfDesktop;
-    HRESULT hr = SHGetDesktopFolder(&psfDesktop);
-    if (FAILED_UNEXPECTEDLY(hr))
-        return FALSE;
-
     CComHeapPtr<ITEMIDLIST_ABSOLUTE> pidlSelection;
-    hr = psfDesktop->ParseDisplayName(NULL, NULL, pszString, NULL, &pidlSelection, NULL);
-    if (FAILED_UNEXPECTEDLY(hr))
-        return FALSE;
+    HRESULT hr = SHParseDisplayName(pszString, NULL, &pidlSelection, 0, NULL);
+    return SUCCEEDED(hr) && BrFolder_ExpandToPidl(info, pidlSelection, phItem);
+}
+
+static BOOL
+BrFolder_OnSetExpanded(BrFolder *info, LPITEMIDLIST pidlSelection, LPWSTR pszString)
+{
+    HTREEITEM hItem;
+    BOOL ret;
+    if (pszString)
+        ret = BrFolder_ExpandToString(info, pszString, &hItem);
+    else
+        ret = BrFolder_ExpandToPidl(info, pidlSelection, &hItem);
 
-    return BrFolder_OnSetExpandedPidl(info, pidlSelection, phItem);
+    if (ret)
+        TreeView_Expand(info->hwndTreeView, hItem, TVE_EXPAND);
+    return ret;
 }
 
 static BOOL
@@ -1070,7 +1079,7 @@ BrFolder_OnSetSelectionPidl(BrFolder *info, LPITEMIDLIST pidlSelection)
         return FALSE;
 
     HTREEITEM hItem;
-    BOOL ret = BrFolder_OnSetExpandedPidl(info, pidlSelection, &hItem);
+    BOOL ret = BrFolder_ExpandToPidl(info, pidlSelection, &hItem);
     if (ret)
         TreeView_SelectItem(info->hwndTreeView, hItem);
     return ret;
@@ -1083,7 +1092,7 @@ BrFolder_OnSetSelectionW(BrFolder *info, LPWSTR pszSelection)
         return FALSE;
 
     HTREEITEM hItem;
-    BOOL ret = BrFolder_OnSetExpandedString(info, pszSelection, &hItem);
+    BOOL ret = BrFolder_ExpandToString(info, pszSelection, &hItem);
     if (ret)
         TreeView_SelectItem(info->hwndTreeView, hItem);
     return ret;
@@ -1304,9 +1313,9 @@ BrFolderDlgProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
 
         case BFFM_SETEXPANDED: // Unicode only
             if (wParam) // String
-                return BrFolder_OnSetExpandedString(info, (LPWSTR)lParam, NULL);
+                return BrFolder_OnSetExpanded(info, NULL, (LPWSTR)lParam);
             else // PIDL
-                return BrFolder_OnSetExpandedPidl(info, (LPITEMIDLIST)lParam, NULL);
+                return BrFolder_OnSetExpanded(info, (LPITEMIDLIST)lParam, NULL);
 
         case SHV_CHANGE_NOTIFY:
             BrFolder_OnChange(info, wParam, lParam);
@@ -1362,16 +1371,23 @@ SHBrowseForFolderW(LPBROWSEINFOW lpbi)
 {
     TRACE("%p\n", lpbi);
 
+    // MSDN says the caller must initialize COM. We do it anyway in case the caller forgot.
+    COleInit OleInit;
     BrFolder info = { lpbi };
 
-    HRESULT hr = OleInitialize(NULL);
-
     INT id = ((lpbi->ulFlags & BIF_USENEWUI) ? IDD_BROWSE_FOR_FOLDER_NEW : IDD_BROWSE_FOR_FOLDER);
     INT_PTR ret = DialogBoxParamW(shell32_hInstance, MAKEINTRESOURCEW(id), lpbi->hwndOwner,
                                   BrFolderDlgProc, (LPARAM)&info);
-    if (SUCCEEDED(hr))
-        OleUninitialize();
-
+    if (ret == IDOK && !(lpbi->ulFlags & BIF_NOTRANSLATETARGETS) &&
+        RosGetProcessEffectiveVersion() >= _WIN32_WINNT_WINXP)
+    {
+        PIDLIST_ABSOLUTE pidlTarget;
+        if (SHELL_GetIDListTarget(info.pidlRet, &pidlTarget) == S_OK)
+        {
+            ILFree(info.pidlRet);
+            info.pidlRet = pidlTarget;
+        }
+    }
     if (ret != IDOK)
     {
         ILFree(info.pidlRet);

dll/win32/shell32/folders/CFSFolder.cpp

@@ -20,13 +20,20 @@ static LPCWSTR GetItemFileName(PCUITEMID_CHILD pidl, LPWSTR Buf, UINT cchMax)
     if (pDataW)
         return pDataW->wszName;
     LPPIDLDATA pdata = _ILGetDataPointer(pidl);
-    if ((pdata->type & PT_VALUEW) == PT_VALUEW)
+    if (_ILGetFSType(pidl) & PT_FS_UNICODE_FLAG)
         return (LPWSTR)pdata->u.file.szNames;
     if (_ILSimpleGetTextW(pidl, Buf, cchMax))
         return Buf;
     return NULL;
 }
 
+static BOOL IsRealItem(const ITEMIDLIST &idl)
+{
+    // PIDLs created with SHSimpleIDListFromPath contain no data, otherwise, the item is "real"
+    FileStruct &fsitem = ((PIDLDATA*)idl.mkid.abID)->u.file;
+    return fsitem.dwFileSize | fsitem.uFileDate;
+}
+
 static HKEY OpenKeyFromFileType(LPCWSTR pExtension, LPCWSTR KeyName)
 {
     HKEY hkey;
@@ -1079,7 +1086,10 @@ HRESULT WINAPI CFSFolder::CompareIDs(LPARAM lParam,
     /* When sorting between a File and a Folder, the Folder gets sorted first */
     if (bIsFolder1 != bIsFolder2)
     {
-        return MAKE_COMPARE_HRESULT(bIsFolder1 ? -1 : 1);
+        // ...but only if neither of them were generated by SHSimpleIDListFromPath
+        // because in that case we cannot tell if it's a file or a folder.
+        if (IsRealItem(*pidl1) && IsRealItem(*pidl2))
+            return MAKE_COMPARE_HRESULT(bIsFolder1 ? -1 : 1);
     }
 
     int result = 0;

dll/win32/shell32/precomp.h

@@ -219,6 +219,7 @@ SHBindToObjectEx(
 
 DWORD
 SHGetAttributes(_In_ IShellFolder *psf, _In_ LPCITEMIDLIST pidl, _In_ DWORD dwAttributes);
+HRESULT SHELL_GetIDListTarget(_In_ LPCITEMIDLIST pidl, _Out_ PIDLIST_ABSOLUTE *ppidl);
 HRESULT SHCoInitializeAnyApartment(VOID);
 
 HRESULT

dll/win32/shell32/shlfileop.cpp

@@ -1633,7 +1633,8 @@ static HRESULT delete_files(FILE_OPERATION *op, const FILE_LIST *flFrom)
             BOOL bDelete;
             if (TRASH_TrashFile(fileEntry->szFullPath))
             {
-                SHChangeNotify(SHCNE_DELETE, SHCNF_PATHW, fileEntry->szFullPath, NULL);
+                UINT event = IsAttribFile(fileEntry->attributes) ? SHCNE_DELETE : SHCNE_RMDIR;
+                SHChangeNotify(event, SHCNF_PATHW, fileEntry->szFullPath, NULL);
                 continue;
             }
 
@@ -1652,9 +1653,7 @@ static HRESULT delete_files(FILE_OPERATION *op, const FILE_LIST *flFrom)
 
         /* delete the file or directory */
         if (IsAttribFile(fileEntry->attributes))
-        {
             bPathExists = (ERROR_SUCCESS == SHNotifyDeleteFileW(op, fileEntry->szFullPath));
-        }
         else
             bPathExists = SHELL_DeleteDirectoryW(op, fileEntry->szFullPath, FALSE);