From d8307d0f7258f35919abebf47d6c33a61e47dce4 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Tue, 20 Sep 2022 16:06:11 -0500 Subject: [PATCH] OpenHere: Replace the fxplorer Window lookup code with site lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we first introduced the shell extension, it didn't work properly for some folders (such as the Desktop, or perhaps any "background" click) due to a bug in Windows. We worked around that bug with the help of an awesome community member, who contributed code that would pull up the topmost Explorer window and query its location. That Windows bug was eventually fixed, but we still had trouble with items appearing correctly. On Windows 11, the Open in Terminal menu item appears and disappears at random when you right-click the desktop, but it always appears when you right-click a folder. It sometimes appears for Quick Access, even though it shouldn't. We tried to fix that in #13206, but the fix caused more issues than it solved. We reverted it for 1.15 and 1.16. At the end of the day, it turns out that getting the path from the toplevel explorer window is fragile. Fortunately, the shell does offer us a way to get that information: the site chain. This pull request replaces GetPathFromExplorer() with an implementation of `IObjectWithSite`, which allows us to use the site chain to look up from whence a context menu request was initiated. It also makes item lookup generally more robust. ✅ Tested on Windows 11 ✅ Desktop ✅ Folder Background ✅ Folder Selected ✅ Quick Access (does not appear) ✅ This PC (does not appear) References #13206 References #13523 Closes #12578 --- .../ShellExtension/OpenTerminalHere.cpp | 169 +++++------------- .../ShellExtension/OpenTerminalHere.h | 13 +- 2 files changed, 59 insertions(+), 123 deletions(-) diff --git a/src/cascadia/ShellExtension/OpenTerminalHere.cpp b/src/cascadia/ShellExtension/OpenTerminalHere.cpp index d9b872136da..ea94206ff9b 100644 --- a/src/cascadia/ShellExtension/OpenTerminalHere.cpp +++ b/src/cascadia/ShellExtension/OpenTerminalHere.cpp @@ -26,29 +26,15 @@ static constexpr std::wstring_view VerbName{ L"WindowsTerminalOpenHere" }; HRESULT OpenTerminalHere::Invoke(IShellItemArray* psiItemArray, IBindCtx* /*pBindContext*/) { - wil::unique_cotaskmem_string pszName; - - if (psiItemArray == nullptr) + winrt::com_ptr psi; + RETURN_IF_FAILED(GetBestLocationFromSelectionOrSite(psiItemArray, psi.put())); + if (!psi) { - // get the current path from explorer.exe - const auto path = this->_GetPathFromExplorer(); - - // no go, unable to get a reasonable path - if (path.empty()) - { - return S_FALSE; - } - pszName = wil::make_cotaskmem_string(path.c_str(), path.length()); + return S_FALSE; } - else - { - DWORD count; - psiItemArray->GetCount(&count); - winrt::com_ptr psi; - RETURN_IF_FAILED(psiItemArray->GetItemAt(0, psi.put())); - RETURN_IF_FAILED(psi->GetDisplayName(SIGDN_FILESYSPATH, &pszName)); - } + wil::unique_cotaskmem_string pszName; + RETURN_IF_FAILED(psi->GetDisplayName(SIGDN_FILESYSPATH, &pszName)); { wil::unique_process_information _piClient; @@ -109,21 +95,14 @@ HRESULT OpenTerminalHere::GetState(IShellItemArray* psiItemArray, // We however don't need to bother with any of that. // If no item was selected when the context menu was opened and Explorer - // is not at a valid path (e.g. This PC or Quick Access), we should hide + // is not at a valid location (e.g. This PC or Quick Access), we should hide // the verb from the context menu. - if (psiItemArray == nullptr) - { - const auto path = this->_GetPathFromExplorer(); - *pCmdState = path.empty() ? ECS_HIDDEN : ECS_ENABLED; - } - else - { - winrt::com_ptr psi; - psiItemArray->GetItemAt(0, psi.put()); - SFGAOF attributes; - const bool isFileSystemItem = (psi->GetAttributes(SFGAO_FILESYSTEM, &attributes) == S_OK); - *pCmdState = isFileSystemItem ? ECS_ENABLED : ECS_HIDDEN; - } + winrt::com_ptr psi; + RETURN_IF_FAILED(GetBestLocationFromSelectionOrSite(psiItemArray, psi.put())); + + SFGAOF attributes; + const bool isFileSystemItem = psi && (psi->GetAttributes(SFGAO_FILESYSTEM, &attributes) == S_OK); + *pCmdState = isFileSystemItem ? ECS_ENABLED : ECS_HIDDEN; return S_OK; } @@ -160,102 +139,52 @@ HRESULT OpenTerminalHere::EnumSubCommands(IEnumExplorerCommand** ppEnum) return E_NOTIMPL; } -std::wstring OpenTerminalHere::_GetPathFromExplorer() const +IFACEMETHODIMP OpenTerminalHere::SetSite(IUnknown* site) noexcept { - using namespace std; - using namespace winrt; - - wstring path; - HRESULT hr = NOERROR; - - auto hwnd = ::GetForegroundWindow(); - if (hwnd == nullptr) - { - return path; - } - - TCHAR szName[MAX_PATH] = { 0 }; - ::GetClassName(hwnd, szName, MAX_PATH); - if (0 == StrCmp(szName, L"WorkerW") || - 0 == StrCmp(szName, L"Progman")) - { - //special folder: desktop - hr = ::SHGetFolderPath(NULL, CSIDL_DESKTOP, NULL, SHGFP_TYPE_CURRENT, szName); - if (FAILED(hr)) - { - return path; - } - - path = szName; - return path; - } - - if (0 != StrCmp(szName, L"CabinetWClass")) - { - return path; - } - - com_ptr shell; - try - { - shell = create_instance(CLSID_ShellWindows, CLSCTX_ALL); - } - catch (...) - { - //look like try_create_instance is not available no more - } + site_.copy_from(site); + return S_OK; +} - if (shell == nullptr) - { - return path; - } +IFACEMETHODIMP OpenTerminalHere::GetSite(REFIID riid, void** site) noexcept +try +{ + site_.as(riid, site); + return S_OK; +} +CATCH_RETURN(); - com_ptr disp; - wil::unique_variant variant; - variant.vt = VT_I4; +HRESULT OpenTerminalHere::GetLocationFromSite(IShellItem** location) const +try +{ + auto serviceProvider = site_.as(); + winrt::com_ptr folderView; + RETURN_IF_FAILED(serviceProvider->QueryService(SID_SFolderView, IID_PPV_ARGS(folderView.put()))); + RETURN_IF_FAILED(folderView->GetFolder(IID_PPV_ARGS(location))); + return S_OK; +} +CATCH_RETURN() - com_ptr browser; - // look for correct explorer window - for (variant.intVal = 0; - shell->Item(variant, disp.put()) == S_OK; - variant.intVal++) +HRESULT OpenTerminalHere::GetBestLocationFromSelectionOrSite(IShellItemArray* psiArray, IShellItem** location) const +try +{ + winrt::com_ptr psi; + if (psiArray) { - com_ptr tmp; - if (FAILED(disp->QueryInterface(tmp.put()))) - { - disp = nullptr; // get rid of DEBUG non-nullptr warning - continue; - } - - HWND tmpHWND = NULL; - hr = tmp->get_HWND(reinterpret_cast(&tmpHWND)); - if (hwnd == tmpHWND) + DWORD count{}; + RETURN_IF_FAILED(psiArray->GetCount(&count)); + if (count) // Sometimes we get an array with a count of 0. Fall back to the site chain. { - browser = tmp; - disp = nullptr; // get rid of DEBUG non-nullptr warning - break; //found + RETURN_IF_FAILED(psiArray->GetItemAt(0, psi.put())); } - - disp = nullptr; // get rid of DEBUG non-nullptr warning } - if (browser != nullptr) + if (!psi) { - wil::unique_bstr url; - hr = browser->get_LocationURL(&url); - if (FAILED(hr)) - { - return path; - } - - wstring sUrl(url.get(), SysStringLen(url.get())); - DWORD size = MAX_PATH; - hr = ::PathCreateFromUrl(sUrl.c_str(), szName, &size, NULL); - if (SUCCEEDED(hr)) - { - path = szName; - } + RETURN_IF_FAILED(GetLocationFromSite(psi.put())); } - return path; + RETURN_HR_IF(S_FALSE, !psi); + psi.copy_to(location); + return S_OK; } +CATCH_RETURN() diff --git a/src/cascadia/ShellExtension/OpenTerminalHere.h b/src/cascadia/ShellExtension/OpenTerminalHere.h index a5af440fb50..0f1a735cfee 100644 --- a/src/cascadia/ShellExtension/OpenTerminalHere.h +++ b/src/cascadia/ShellExtension/OpenTerminalHere.h @@ -22,7 +22,7 @@ Author(s): --*/ #pragma once -#include +#include using namespace Microsoft::WRL; @@ -34,7 +34,7 @@ struct #else // DEV __declspec(uuid("52065414-e077-47ec-a3ac-1cc5455e1b54")) #endif - OpenTerminalHere : public RuntimeClass, IExplorerCommand> + OpenTerminalHere : public RuntimeClass, IExplorerCommand, IObjectWithSite> { #pragma region IExplorerCommand STDMETHODIMP Invoke(IShellItemArray* psiItemArray, @@ -52,9 +52,16 @@ struct STDMETHODIMP GetCanonicalName(GUID* pguidCommandName); STDMETHODIMP EnumSubCommands(IEnumExplorerCommand** ppEnum); #pragma endregion +#pragma region IObjectWithSite + IFACEMETHODIMP SetSite(IUnknown* site) noexcept; + IFACEMETHODIMP GetSite(REFIID riid, void** site) noexcept; +#pragma endregion private: - std::wstring _GetPathFromExplorer() const; + HRESULT GetLocationFromSite(IShellItem** location) const; + HRESULT GetBestLocationFromSelectionOrSite(IShellItemArray* psiArray, IShellItem** location) const; + + winrt::com_ptr site_; }; CoCreatableClass(OpenTerminalHere);