Bug Fixes for PowerRename (#614)

* Bug Fixes Fixes include: * Pass parent HWND to UI so dialog is no longer appearing in top corner all the time * Fix duplicate entries for PowerRename in context menu for shortcuts * Fix crashing bug due to telemetry not getting unregistered on unload * Ensure we show the file extension in the UI even if extensions are hidden in Windows Explorer * Update PowerRenameExt.cpp Fix missed line to set parent HWND
microsoft · Oct 31, 2019 · c195727 · c195727
1 parent e6afd33
commit c195727
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 25 deletions.
diff --git a/src/modules/powerrename/dll/PowerRenameExt.cpp b/src/modules/powerrename/dll/PowerRenameExt.cpp
@@ -8,8 +8,11 @@
 
 extern HINSTANCE g_hInst;
 
-HWND g_hwndParent = 0;
-
+struct InvokeStruct
+{
+    HWND hwndParent;
+    IStream* pstrm;
+};
 
 const wchar_t powerRenameRegPath[] = L"Software\\Microsoft\\PowerRename";
 const wchar_t powerRenameRegEnabledName[] = L"Enabled";
@@ -73,15 +76,12 @@ HRESULT CPowerRenameMenu::QueryContextMenu(HMENU hMenu, UINT index, UINT uIDFirs
         return E_FAIL;
 
     HRESULT hr = E_UNEXPECTED;
-    if (m_spdo)
+    if (m_spdo && !(uFlags & (CMF_DEFAULTONLY | CMF_VERBSONLY | CMF_OPTIMIZEFORINVOKE)))
     {
-        if ((uFlags & ~CMF_OPTIMIZEFORINVOKE) && (uFlags & ~(CMF_DEFAULTONLY | CMF_VERBSONLY)))
-        {
-            wchar_t menuName[64] = { 0 };
-            LoadString(g_hInst, IDS_POWERRENAME, menuName, ARRAYSIZE(menuName));
-            InsertMenu(hMenu, index, MF_STRING | MF_BYPOSITION, uIDFirst++, menuName);
-            hr = MAKE_HRESULT(SEVERITY_SUCCESS, FACILITY_NULL, 1);
-        }
+        wchar_t menuName[64] = { 0 };
+        LoadString(g_hInst, IDS_POWERRENAME, menuName, ARRAYSIZE(menuName));
+        InsertMenu(hMenu, index, MF_STRING | MF_BYPOSITION, uIDFirst++, menuName);
+        hr = MAKE_HRESULT(SEVERITY_SUCCESS, FACILITY_NULL, 1);
     }
 
     return hr;
@@ -96,14 +96,24 @@ HRESULT CPowerRenameMenu::InvokeCommand(_In_ LPCMINVOKECOMMANDINFO pici)
         (LOWORD(pici->lpVerb) == 0))
     {
         Trace::Invoked();
-        IStream* pstrm = nullptr;
-        hr = CoMarshalInterThreadInterfaceInStream(__uuidof(m_spdo), m_spdo, &pstrm);
+        InvokeStruct* pInvokeData = new InvokeStruct;
+        hr = pInvokeData ? S_OK : E_OUTOFMEMORY;
         if (SUCCEEDED(hr))
         {
-            if (!SHCreateThread(s_PowerRenameUIThreadProc, pstrm, CTF_COINIT | CTF_PROCESS_REF, nullptr))
+            pInvokeData->hwndParent = pici->hwnd;
+            hr = CoMarshalInterThreadInterfaceInStream(__uuidof(m_spdo), m_spdo, &(pInvokeData->pstrm));
+            if (SUCCEEDED(hr))
             {
-                pstrm->Release(); // if we failed to create the thread, then we must release the stream
-                hr = E_FAIL;
+                hr = SHCreateThread(s_PowerRenameUIThreadProc, pInvokeData, CTF_COINIT | CTF_PROCESS_REF, nullptr) ? S_OK : E_FAIL;
+                if (FAILED(hr))
+                {
+                    pInvokeData->pstrm->Release(); // if we failed to create the thread, then we must release the stream
+                }
+            }
+
+            if (FAILED(hr))
+            {
+                delete pInvokeData;
             }
         }
         Trace::InvokedRet(hr);
@@ -114,9 +124,9 @@ HRESULT CPowerRenameMenu::InvokeCommand(_In_ LPCMINVOKECOMMANDINFO pici)
 
 DWORD WINAPI CPowerRenameMenu::s_PowerRenameUIThreadProc(_In_ void* pData)
 {
-    IStream* pstrm = static_cast<IStream*>(pData);
+    InvokeStruct* pInvokeData = static_cast<InvokeStruct*>(pData);
     CComPtr<IDataObject> spdo;
-    HRESULT hr = CoGetInterfaceAndReleaseStream(pstrm, IID_PPV_ARGS(&spdo));
+    HRESULT hr = CoGetInterfaceAndReleaseStream(pInvokeData->pstrm, IID_PPV_ARGS(&spdo));
     if (SUCCEEDED(hr))
     {
         // Create the smart rename manager
@@ -139,7 +149,7 @@ DWORD WINAPI CPowerRenameMenu::s_PowerRenameUIThreadProc(_In_ void* pData)
                     if (SUCCEEDED(hr))
                     {
                         // Call blocks until we are done
-                        spsrui->Show();
+                        spsrui->Show(pInvokeData->hwndParent);
                         spsrui->Close();
                     }
                 }
@@ -150,6 +160,8 @@ DWORD WINAPI CPowerRenameMenu::s_PowerRenameUIThreadProc(_In_ void* pData)
         }
     }
 
+    delete pInvokeData;
+
     Trace::UIShownRet(hr);
 
     return 0;

diff --git a/src/modules/powerrename/dll/dllmain.cpp b/src/modules/powerrename/dll/dllmain.cpp
@@ -101,7 +101,7 @@ BOOL WINAPI DllMain(HINSTANCE hInstance, DWORD dwReason, void*)
         break;
 
     case DLL_PROCESS_DETACH:
-        Trace::RegisterProvider();
+        Trace::UnregisterProvider();
         break;
     }
     return TRUE;

diff --git a/src/modules/powerrename/lib/PowerRenameInterfaces.h b/src/modules/powerrename/lib/PowerRenameInterfaces.h
@@ -102,7 +102,7 @@ interface __declspec(uuid("001BBD88-53D2-4FA6-95D2-F9A9FA4F9F70")) IPowerRenameM
 interface __declspec(uuid("E6679DEB-460D-42C1-A7A8-E25897061C99")) IPowerRenameUI : public IUnknown
 {
 public:
-    IFACEMETHOD(Show)() = 0;
+    IFACEMETHOD(Show)(_In_opt_ HWND hwndParent) = 0;
     IFACEMETHOD(Close)() = 0;
     IFACEMETHOD(Update)() = 0;
 };

diff --git a/src/modules/powerrename/lib/PowerRenameItem.cpp b/src/modules/powerrename/lib/PowerRenameItem.cpp
@@ -203,7 +203,7 @@ HRESULT CPowerRenameItem::_Init(_In_ IShellItem* psi)
     HRESULT hr = psi->GetDisplayName(SIGDN_FILESYSPATH, &m_path);
     if (SUCCEEDED(hr))
     {
-        hr = psi->GetDisplayName(SIGDN_NORMALDISPLAY, &m_originalName);
+        hr = SHStrDup(PathFindFileName(m_path), &m_originalName);
         if (SUCCEEDED(hr))
         {
             // Check if we are a folder now so we can check this attribute quickly later

diff --git a/src/modules/powerrename/testapp/PowerRenameTest.cpp b/src/modules/powerrename/testapp/PowerRenameTest.cpp
@@ -47,7 +47,7 @@ int APIENTRY wWinMain(
                     if (SUCCEEDED(CPowerRenameUI::s_CreateInstance(spsrm, nullptr, true, &spsrui)))
                     {
                         // Call blocks until we are done
-                        spsrui->Show();
+                        spsrui->Show(NULL);
                         spsrui->Close();
 
                         // Need to call shutdown to break circular dependencies

diff --git a/src/modules/powerrename/ui/PowerRenameUI.cpp b/src/modules/powerrename/ui/PowerRenameUI.cpp
@@ -89,9 +89,9 @@ HRESULT CPowerRenameUI::s_CreateInstance(_In_ IPowerRenameManager* psrm, _In_opt
 }
 
 // IPowerRenameUI
-IFACEMETHODIMP CPowerRenameUI::Show()
+IFACEMETHODIMP CPowerRenameUI::Show(_In_opt_ HWND hwndParent)
 {
-    return _DoModal(NULL);
+    return _DoModal(hwndParent);
 }
 
 IFACEMETHODIMP CPowerRenameUI::Close()

diff --git a/src/modules/powerrename/ui/PowerRenameUI.h b/src/modules/powerrename/ui/PowerRenameUI.h
@@ -45,7 +45,7 @@ class CPowerRenameUI :
     IFACEMETHODIMP_(ULONG) Release();
 
     // IPowerRenameUI
-    IFACEMETHODIMP Show();
+    IFACEMETHODIMP Show(_In_opt_ HWND hwndParent);
     IFACEMETHODIMP Close();
     IFACEMETHODIMP Update();
     IFACEMETHODIMP get_hwnd(_Out_ HWND* hwnd);