[dquintana] 65537: [RSHELL] * Partially rewrite CMergedFolder to fix the invalid assumption that the results of enumerating a folder are ordered. Fixes the folder merging on certain languages not w...
Author: dquintana Date: Mon Dec 1 04:23:44 2014 New Revision: 65537
URL: http://svn.reactos.org/svn/reactos?rev=65537&view=rev Log: [RSHELL] * Partially rewrite CMergedFolder to fix the invalid assumption that the results of enumerating a folder are ordered. Fixes the folder merging on certain languages not working as expected, and also some other weaknesses of the previous implementation.
CORE-8835 #resolve #comment Should be fixed in r65537. Thanks for reporting.
Modified: trunk/reactos/base/shell/rshell/CMergedFolder.cpp trunk/reactos/base/shell/rshell/CMergedFolder.h trunk/reactos/base/shell/rshell/CStartMenu.cpp
Modified: trunk/reactos/base/shell/rshell/CMergedFolder.cpp URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/shell/rshell/CMergedFo... ============================================================================== --- trunk/reactos/base/shell/rshell/CMergedFolder.cpp [iso-8859-1] (original) +++ trunk/reactos/base/shell/rshell/CMergedFolder.cpp [iso-8859-1] Mon Dec 1 04:23:44 2014 @@ -30,6 +30,8 @@ BOOL shared; IShellFolder * parent; LPITEMIDLIST pidl; + LPITEMIDLIST pidl2; + LPCWSTR parseName; };
class CEnumMergedFolder : @@ -66,6 +68,7 @@ HRESULT SetSources(IShellFolder * userLocal, IShellFolder * allUSers); HRESULT Begin(HWND hwndOwner, SHCONTF flags); HRESULT FindPidlInList(HWND hwndOwner, LPCITEMIDLIST pcidl, LocalPidlInfo * pinfo); + HRESULT FindByName(HWND hwndOwner, LPCWSTR strParsingName, LocalPidlInfo * pinfo);
virtual HRESULT STDMETHODCALLTYPE Next( ULONG celt, @@ -96,6 +99,9 @@ int CEnumMergedFolder::DsaDeleteCallback(LocalPidlInfo * info) { ILFree(info->pidl); + if (info->pidl2) + ILFree(info->pidl2); + CoTaskMemFree((LPVOID)info->parseName); return 0; }
@@ -118,7 +124,8 @@ HRESULT CEnumMergedFolder::Begin(HWND hwndOwner, SHCONTF flags) { HRESULT hr; - + LPITEMIDLIST pidl = NULL; + if (m_hDsa && m_HwndOwner == hwndOwner && m_Flags == flags) { return Reset(); @@ -127,16 +134,14 @@ TRACE("Search conditions changed, recreating list...\n");
CComPtr<IEnumIDList> userLocal; - CComPtr<IEnumIDList> allUSers; + CComPtr<IEnumIDList> allUsers; + hr = m_UserLocalFolder->EnumObjects(hwndOwner, flags, &userLocal); if (FAILED_UNEXPECTEDLY(hr)) return hr; - hr = m_AllUSersFolder->EnumObjects(hwndOwner, flags, &allUSers); - if (FAILED_UNEXPECTEDLY(hr)) - { - userLocal = NULL; - return hr; - } + hr = m_AllUSersFolder->EnumObjects(hwndOwner, flags, &allUsers); + if (FAILED_UNEXPECTEDLY(hr)) + return hr;
if (!m_hDsa) { @@ -147,143 +152,131 @@ DSA_DeleteAllItems(m_hDsa); m_hDsaCount = 0;
- HRESULT hr1 = S_OK; - HRESULT hr2 = S_OK; - LPITEMIDLIST pidl1 = NULL; - LPITEMIDLIST pidl2 = NULL; - int order = 0; - do - { - if (order <= 0) + TRACE("Loading Local entries...\n"); + for (;;) + { + hr = userLocal->Next(1, &pidl, NULL); + if (FAILED_UNEXPECTEDLY(hr)) + return hr; + + if (hr == S_FALSE) + break; + + LPWSTR name; + STRRET str = { STRRET_WSTR }; + hr = m_UserLocalFolder->GetDisplayNameOf(pidl, SHGDN_FORPARSING | SHGDN_INFOLDER, &str); + if (FAILED(hr)) + return hr; + StrRetToStrW(&str, pidl, &name); + + LocalPidlInfo info = { + FALSE, + m_UserLocalFolder, + ILClone(pidl), + NULL, + name + }; + + ILFree(pidl); + + TRACE("Inserting item %d with name %S\n", m_hDsaCount, name); + int idx = DSA_InsertItem(m_hDsa, DSA_APPEND, &info); + TRACE("New index: %d\n", idx); + + m_hDsaCount++; + } + + TRACE("Loading Common entries...\n"); + for (;;) + { + hr = allUsers->Next(1, &pidl, NULL); + if (FAILED_UNEXPECTEDLY(hr)) + return hr; + + if (hr == S_FALSE) + break; + + LPWSTR name; + STRRET str = { STRRET_WSTR }; + hr = m_AllUSersFolder->GetDisplayNameOf(pidl, SHGDN_FORPARSING | SHGDN_INFOLDER, &str); + if (FAILED(hr)) + return hr; + StrRetToStrW(&str, pidl, &name); + + LocalPidlInfo info = { + FALSE, + m_AllUSersFolder, + ILClone(pidl), + NULL, + name + }; + + ILFree(pidl); + + BOOL bShared = FALSE; + for (int i = 0; i < (int)m_hDsaCount; i++) { - if (hr1 == S_OK) + LocalPidlInfo *pInfo = (LocalPidlInfo *) DSA_GetItemPtr(m_hDsa, i); + + int order = CompareStringW(GetThreadLocale(), NORM_IGNORECASE, + pInfo->parseName, lstrlenW(pInfo->parseName), + info.parseName, lstrlenW(info.parseName)); + switch (order) { - hr1 = userLocal->Next(1, &pidl1, NULL); - if (FAILED_UNEXPECTEDLY(hr1)) - return hr1; - } - else - { - pidl1 = NULL; + case CSTR_EQUAL: + TRACE("Item name already exists! Marking '%S' as shared ...\n", name); + bShared = TRUE; + pInfo->shared = TRUE; + pInfo->pidl2 = info.pidl; + CoTaskMemFree(name); + break; + default: + continue; } } - if (order >= 0) + + if (!bShared) { - if (hr2 == S_OK) - { - hr2 = allUSers->Next(1, &pidl2, NULL); - if (FAILED_UNEXPECTEDLY(hr2)) - return hr2; - } - else - { - pidl2 = NULL; - } + TRACE("Inserting item %d with name %S\n", m_hDsaCount, name); + int idx = DSA_InsertItem(m_hDsa, DSA_APPEND, &info); + TRACE("New index: %d\n", idx); + + m_hDsaCount++; } - - if (hr1 == S_OK && hr2 == S_OK) - { - LPWSTR name1; - LPWSTR name2; - STRRET str1 = { STRRET_WSTR }; - STRRET str2 = { STRRET_WSTR }; - hr = m_UserLocalFolder->GetDisplayNameOf(pidl1, SHGDN_FORPARSING | SHGDN_INFOLDER, &str1); - if (FAILED(hr)) - return hr; - hr = m_AllUSersFolder->GetDisplayNameOf(pidl2, SHGDN_FORPARSING | SHGDN_INFOLDER, &str2); - if (FAILED(hr)) - return hr; - StrRetToStrW(&str1, pidl1, &name1); - StrRetToStrW(&str2, pidl2, &name2); - order = StrCmpW(name1, name2); - - TRACE("Both sources are S_OK, comparison between %S and %S returns %d\n", name1, name2, order); - - CoTaskMemFree(name1); - CoTaskMemFree(name2); - } - else if (hr1 == S_OK) - { - order = -1; - - TRACE("Both sources are S_OK, forcing %d\n", order); - } - else if (hr2 == S_OK) - { - order = 1; - - TRACE("Both sources are S_OK, forcing %d\n", order); - } - else - { - TRACE("None of the sources\n"); - break; - } - - LocalPidlInfo info = { FALSE }; - if (order < 0) - { - info.parent = m_UserLocalFolder; - info.pidl = ILClone(pidl1); - ILFree(pidl1); - } - else if (order > 0) - { - info.parent = m_AllUSersFolder; - info.pidl = ILClone(pidl2); - ILFree(pidl2); - } - else // if (order == 0) - { - info.shared = TRUE; - info.parent = m_UserLocalFolder; - info.pidl = ILClone(pidl1); - ILFree(pidl1); - ILFree(pidl2); - } - - TRACE("Inserting item %d with parent %p and pidl { cb=%d }\n", m_hDsaCount, info.parent, info.pidl->mkid.cb); - int idx = DSA_InsertItem(m_hDsa, DSA_APPEND, &info); - TRACE("New index: %d\n", idx); - - m_hDsaCount++; - - } while (hr1 == S_OK || hr2 == S_OK); + }
m_HwndOwner = hwndOwner; m_Flags = flags; + + return Reset(); +} + +HRESULT CEnumMergedFolder::FindPidlInList(HWND hwndOwner, LPCITEMIDLIST pcidl, LocalPidlInfo * pinfo) +{ + HRESULT hr; + + if (!m_hDsa) + { + Begin(hwndOwner, SHCONTF_FOLDERS | SHCONTF_NONFOLDERS); + } + + TRACE("Searching for pidl { cb=%d } in a list of %d items\n", pcidl->mkid.cb, m_hDsaCount); + + for (int i = 0; i < (int)m_hDsaCount; i++) + { + LocalPidlInfo * pInfo = (LocalPidlInfo *) DSA_GetItemPtr(m_hDsa, i); + if (!pInfo) + return E_FAIL;
- return Reset(); -} - -HRESULT CEnumMergedFolder::FindPidlInList(HWND hwndOwner, LPCITEMIDLIST pcidl, LocalPidlInfo * pinfo) -{ - HRESULT hr; - - if (!m_hDsa) - { - Begin(hwndOwner, SHCONTF_FOLDERS | SHCONTF_NONFOLDERS); - } - - TRACE("Searching for pidl { cb=%d } in a list of %d items\n", pcidl->mkid.cb, m_hDsaCount); - - for (int i = 0; i < (int)m_hDsaCount; i++) - { - LocalPidlInfo * tinfo = (LocalPidlInfo *)DSA_GetItemPtr(m_hDsa, i); - if (!tinfo) - return E_FAIL; - - LocalPidlInfo info = *tinfo; - - TRACE("Comparing with item at %d with parent %p and pidl { cb=%d }\n", i, info.parent, info.pidl->mkid.cb); - - hr = info.parent->CompareIDs(0, info.pidl, pcidl); + TRACE("Comparing with item at %d with parent %p and pidl { cb=%d }\n", i, pInfo->parent, pInfo->pidl->mkid.cb); + + hr = pInfo->parent->CompareIDs(0, pInfo->pidl, pcidl); if (FAILED_UNEXPECTEDLY(hr)) return hr;
if (hr == S_OK) { - *pinfo = info; + *pinfo = *pInfo; return S_OK; } else @@ -296,12 +289,45 @@ return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); }
+HRESULT CEnumMergedFolder::FindByName(HWND hwndOwner, LPCWSTR strParsingName, LocalPidlInfo * pinfo) +{ + if (!m_hDsa) + { + Begin(hwndOwner, SHCONTF_FOLDERS | SHCONTF_NONFOLDERS); + } + + TRACE("Searching for '%S' in a list of %d items\n", strParsingName, m_hDsaCount); + + for (int i = 0; i < (int) m_hDsaCount; i++) + { + LocalPidlInfo * pInfo = (LocalPidlInfo *) DSA_GetItemPtr(m_hDsa, i); + if (!pInfo) + return E_FAIL; + + int order = CompareStringW(GetThreadLocale(), NORM_IGNORECASE, + pInfo->parseName, lstrlenW(pInfo->parseName), + strParsingName, lstrlenW(strParsingName)); + switch (order) + { + case CSTR_EQUAL: + *pinfo = *pInfo; + return S_OK; + default: + continue; + } + } + + TRACE("Pidl not found\n"); + return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); +} + HRESULT STDMETHODCALLTYPE CEnumMergedFolder::Next( ULONG celt, LPITEMIDLIST *rgelt, ULONG *pceltFetched) { - if (pceltFetched) *pceltFetched = 0; + if (pceltFetched) + *pceltFetched = 0;
if (m_hDsaIndex == m_hDsaCount) return S_FALSE; @@ -323,7 +349,8 @@ m_hDsaIndex++; if (m_hDsaIndex == m_hDsaCount) { - if (pceltFetched) *pceltFetched = i; + if (pceltFetched) + *pceltFetched = i; return (i == (int)celt) ? S_OK : S_FALSE; } } @@ -361,7 +388,7 @@
CMergedFolder::CMergedFolder() : m_UserLocal(NULL), - m_AllUSers(NULL), + m_AllUsers(NULL), m_EnumSource(NULL), m_UserLocalPidl(NULL), m_AllUsersPidl(NULL), @@ -384,7 +411,7 @@ return E_NOTIMPL; }
- TRACE("AddNameSpace %p %p\n", m_UserLocal.p, m_AllUSers.p); + TRACE("AddNameSpace %p %p\n", m_UserLocal.p, m_AllUsers.p);
// FIXME: Use a DSA to store the list of merged namespaces, together with their related info (psf, pidl, ...) // For now, assume only 2 will ever be used, and ignore all the other data. @@ -395,14 +422,14 @@ return S_OK; }
- if (m_AllUSers) + if (m_AllUsers) return E_FAIL;
- m_AllUSers = psf; + m_AllUsers = psf; m_AllUsersPidl = ILClone(pcidl);
m_EnumSource = new CComObject<CEnumMergedFolder>(); - return m_EnumSource->SetSources(m_UserLocal, m_AllUSers); + return m_EnumSource->SetSources(m_UserLocal, m_AllUsers); }
HRESULT STDMETHODCALLTYPE CMergedFolder::GetNameSpaceID(LPCITEMIDLIST pcidl, LPGUID lpGuid) @@ -440,45 +467,33 @@ { HRESULT hr; LocalPidlInfo info; - LPITEMIDLIST pidl; - - if (!ppidl) return E_FAIL; - - if (pchEaten) *pchEaten = 0; - if (pdwAttributes) *pdwAttributes = 0; + + if (!ppidl) + return E_FAIL; + + if (pchEaten) + *pchEaten = 0; + + if (pdwAttributes) + *pdwAttributes = 0;
TRACE("ParseDisplayName name=%S\n", lpszDisplayName);
- hr = m_UserLocal->ParseDisplayName(hwndOwner, pbcReserved, lpszDisplayName, pchEaten, &pidl, pdwAttributes); - if (SUCCEEDED(hr)) - { - TRACE("ParseDisplayName result local\n"); - hr = m_EnumSource->FindPidlInList(hwndOwner, pidl, &info); - if (SUCCEEDED(hr)) - { - ILFree(pidl); - *ppidl = ILClone(info.pidl); - return hr; - } - } - - hr = m_AllUSers->ParseDisplayName(hwndOwner, pbcReserved, lpszDisplayName, pchEaten, &pidl, pdwAttributes); - if (SUCCEEDED(hr)) - { - TRACE("ParseDisplayName result common\n"); - hr = m_EnumSource->FindPidlInList(hwndOwner, pidl, &info); - if (SUCCEEDED(hr)) - { - ILFree(pidl); - *ppidl = ILClone(info.pidl); - return hr; - } - } - - if (ppidl) *ppidl = NULL; - if (pchEaten) *pchEaten = 0; - if (pdwAttributes) *pdwAttributes = 0; - return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); + hr = m_EnumSource->FindByName(hwndOwner, lpszDisplayName, &info); + if (FAILED(hr)) + { + return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); + } + + *ppidl = ILClone(info.pidl); + + if (pchEaten) + *pchEaten = lstrlenW(info.parseName); + + if (pdwAttributes) + *pdwAttributes = info.parent->GetAttributesOf(1, (LPCITEMIDLIST*)ppidl, pdwAttributes); + + return S_OK; }
HRESULT STDMETHODCALLTYPE CMergedFolder::EnumObjects( @@ -514,14 +529,16 @@ if (riid != IID_IShellFolder) return E_FAIL;
+ // Construct a child MergedFolder and return it CComPtr<IShellFolder> fld1; CComPtr<IShellFolder> fld2;
+ // In shared folders, the user one takes precedence over the common one, so it will always be on pidl1 hr = m_UserLocal->BindToObject(info.pidl, pbcReserved, IID_PPV_ARG(IShellFolder, &fld1)); if (FAILED_UNEXPECTEDLY(hr)) return hr;
- hr = m_AllUSers->BindToObject(info.pidl, pbcReserved, IID_PPV_ARG(IShellFolder, &fld2)); + hr = m_AllUsers->BindToObject(info.pidl2, pbcReserved, IID_PPV_ARG(IShellFolder, &fld2)); if (FAILED_UNEXPECTEDLY(hr)) return hr;
@@ -538,7 +555,7 @@ if (FAILED_UNEXPECTEDLY(hr)) return hr;
- hr = pasf->AddNameSpace(NULL, fld2, info.pidl, 0x0000); + hr = pasf->AddNameSpace(NULL, fld2, info.pidl2, 0x0000); if (FAILED_UNEXPECTEDLY(hr)) return hr;
@@ -786,4 +803,11 @@ { UNIMPLEMENTED; return E_NOTIMPL; +} + +// IAugmentedShellFolder3 +HRESULT STDMETHODCALLTYPE CMergedFolder::QueryNameSpace2(ULONG, QUERYNAMESPACEINFO *) +{ + UNIMPLEMENTED; + return E_NOTIMPL; }
Modified: trunk/reactos/base/shell/rshell/CMergedFolder.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/shell/rshell/CMergedFo... ============================================================================== --- trunk/reactos/base/shell/rshell/CMergedFolder.h [iso-8859-1] (original) +++ trunk/reactos/base/shell/rshell/CMergedFolder.h [iso-8859-1] Mon Dec 1 04:23:44 2014 @@ -21,6 +21,7 @@
extern IID IID_IAugmentedShellFolder; extern IID IID_IAugmentedShellFolder2; +extern IID IID_IAugmentedShellFolder3; extern CLSID CLSID_MergedFolder;
interface IAugmentedShellFolder : public IShellFolder @@ -36,33 +37,35 @@ virtual HRESULT STDMETHODCALLTYPE UnWrapIDList(LPCITEMIDLIST, LONG, IShellFolder **, LPITEMIDLIST *, LPITEMIDLIST *, LONG *) = 0; };
-/* No idea what QUERYNAMESPACEINFO struct contains -- the prototype comes from the PDB info +/* No idea what QUERYNAMESPACEINFO struct contains, yet */ +struct QUERYNAMESPACEINFO +{ + BYTE unknown[1]; +}; + interface IAugmentedShellFolder3 : public IAugmentedShellFolder2 { virtual HRESULT STDMETHODCALLTYPE QueryNameSpace2(ULONG, QUERYNAMESPACEINFO *) = 0; }; -*/
class CEnumMergedFolder;
class CMergedFolder : public CComObjectRootEx<CComMultiThreadModelNoCS>, public IShellFolder2, - //public IStorage, - public IAugmentedShellFolder2, // -- undocumented - //public IAugmentedShellFolder3, // -- undocumented + public IPersistFolder2, + public IAugmentedShellFolder3 // -- undocumented //public IShellService, // -- undocumented //public ITranslateShellChangeNotify,// -- undocumented - public IPersistFolder2 + //public IStorage, //public IPersistPropertyBag, //public IShellIconOverlay, // -- undocumented //public ICompositeFolder, // -- undocumented //public IItemNameLimits, // -- undocumented - { private: CComPtr<IShellFolder> m_UserLocal; - CComPtr<IShellFolder> m_AllUSers; + CComPtr<IShellFolder> m_AllUsers; CComPtr<CEnumMergedFolder> m_EnumSource;
LPITEMIDLIST m_UserLocalPidl; @@ -86,7 +89,7 @@ COM_INTERFACE_ENTRY_IID(IID_IPersistFolder2, IPersistFolder2) COM_INTERFACE_ENTRY_IID(IID_IAugmentedShellFolder, IAugmentedShellFolder) COM_INTERFACE_ENTRY_IID(IID_IAugmentedShellFolder2, IAugmentedShellFolder2) - //COM_INTERFACE_ENTRY_IID(IID_IAugmentedShellFolder3, IAugmentedShellFolder3) + COM_INTERFACE_ENTRY_IID(IID_IAugmentedShellFolder3, IAugmentedShellFolder3) //COM_INTERFACE_ENTRY_IID(IID_IStorage, IStorage) //COM_INTERFACE_ENTRY_IID(IID_IShellService, IShellService) //COM_INTERFACE_ENTRY_IID(IID_ITranslateShellChangeNotify,ITranslateShellChangeNotify) @@ -202,4 +205,7 @@ virtual HRESULT STDMETHODCALLTYPE QueryNameSpace(ULONG dwUnknown, LPGUID lpGuid, IShellFolder ** ppsf); virtual HRESULT STDMETHODCALLTYPE EnumNameSpace(ULONG dwUnknown, PULONG lpUnknown); virtual HRESULT STDMETHODCALLTYPE UnWrapIDList(LPCITEMIDLIST pcidl, LONG lUnknown, IShellFolder ** ppsf, LPITEMIDLIST * ppidl1, LPITEMIDLIST *ppidl2, LONG * lpUnknown); -}; + + // IAugmentedShellFolder3 + virtual HRESULT STDMETHODCALLTYPE QueryNameSpace2(ULONG, QUERYNAMESPACEINFO *); +};
Modified: trunk/reactos/base/shell/rshell/CStartMenu.cpp URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/shell/rshell/CStartMen... ============================================================================== --- trunk/reactos/base/shell/rshell/CStartMenu.cpp [iso-8859-1] (original) +++ trunk/reactos/base/shell/rshell/CStartMenu.cpp [iso-8859-1] Mon Dec 1 04:23:44 2014 @@ -24,8 +24,9 @@ WINE_DEFAULT_DEBUG_CHANNEL(CStartMenu);
// TODO: declare these GUIDs and interfaces in the right place (whatever that may be) -IID IID_IAugmentedShellFolder = { 0x91EA3F8C, 0xC99B, 0x11D0, { 0x98, 0x15, 0x00, 0xC0, 0x4F, 0xD9, 0x19, 0x72 } }; +IID IID_IAugmentedShellFolder = { 0x91EA3F8C, 0xC99B, 0x11D0, { 0x98, 0x15, 0x00, 0xC0, 0x4F, 0xD9, 0x19, 0x72 } }; IID IID_IAugmentedShellFolder2 = { 0x8DB3B3F4, 0x6CFE, 0x11D1, { 0x8A, 0xE9, 0x00, 0xC0, 0x4F, 0xD9, 0x18, 0xD0 } }; +IID IID_IAugmentedShellFolder3 = { 0x4F755EA8, 0x247D, 0x479B, { 0x91, 0x81, 0x22, 0x7D, 0x09, 0xC2, 0xE0, 0x01 } }; CLSID CLSID_MergedFolder = { 0x26FDC864, 0xBE88, 0x46E7, { 0x92, 0x35, 0x03, 0x2D, 0x8E, 0xA5, 0x16, 0x2E } };
//#define TEST_TRACKPOPUPMENU_SUBMENUS
-
dquintana＠svn.reactos.org