From faa4c1ac8b33a6288be281292fc2ad6ca1fa831d Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Wed, 21 Sep 2022 12:41:37 -0500 Subject: [PATCH] OpenHere: Replace explorer window lookup code w/ site lookup (#14048) 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) * ✅ Tested on Windows 10 * ✅ Desktop * ✅ Folder Background * ✅ Folder Selected * ✅ Quick Access (does not appear) * ✅ This PC (does not appear) References #13206 References #13523 Closes #12578 Co-authored-by: John Lueders (cherry picked from commit 5027c8031d009ff5d26032134ab1e70b0c91f587) Service-Card-Id: 85788408 Service-Version: 1.15 --- .../ShellExtension/OpenTerminalHere.cpp | 163 ++++++------------ .../ShellExtension/OpenTerminalHere.h | 13 +- 2 files changed, 61 insertions(+), 115 deletions(-) diff --git a/src/cascadia/ShellExtension/OpenTerminalHere.cpp b/src/cascadia/ShellExtension/OpenTerminalHere.cpp index 5f472d72f4b..4d9f44f7212 100644 --- a/src/cascadia/ShellExtension/OpenTerminalHere.cpp +++ b/src/cascadia/ShellExtension/OpenTerminalHere.cpp @@ -25,37 +25,25 @@ static constexpr std::wstring_view VerbName{ L"WindowsTerminalOpenHere" }; // failure from an earlier HRESULT. HRESULT OpenTerminalHere::Invoke(IShellItemArray* psiItemArray, IBindCtx* /*pBindContext*/) +try { - wil::unique_cotaskmem_string pszName; - - if (psiItemArray == nullptr) + wil::com_ptr_nothrow 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; STARTUPINFOEX siEx{ 0 }; siEx.StartupInfo.cb = sizeof(STARTUPINFOEX); - auto cmdline{ wil::str_printf(LR"-("%s" -d %s)-", GetWtExePath().c_str(), QuoteAndEscapeCommandlineArg(pszName.get()).c_str()) }; + std::wstring cmdline; + RETURN_IF_FAILED(wil::str_printf_nothrow(cmdline, LR"-("%s" -d %s)-", GetWtExePath().c_str(), QuoteAndEscapeCommandlineArg(pszName.get()).c_str())); RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW( nullptr, // lpApplicationName cmdline.data(), @@ -72,6 +60,7 @@ HRESULT OpenTerminalHere::Invoke(IShellItemArray* psiItemArray, return S_OK; } +CATCH_RETURN() HRESULT OpenTerminalHere::GetToolTip(IShellItemArray* /*psiItemArray*/, LPWSTR* ppszInfoTip) @@ -97,7 +86,7 @@ HRESULT OpenTerminalHere::GetTitle(IShellItemArray* /*psiItemArray*/, return SHStrDup(resource.data(), ppszName); } -HRESULT OpenTerminalHere::GetState(IShellItemArray* /*psiItemArray*/, +HRESULT OpenTerminalHere::GetState(IShellItemArray* psiItemArray, BOOL /*fOkToBeSlow*/, EXPCMDSTATE* pCmdState) { @@ -106,10 +95,17 @@ HRESULT OpenTerminalHere::GetState(IShellItemArray* /*psiItemArray*/, // E_PENDING and this object will be called back on a background thread with // fOkToBeSlow == TRUE - // We however don't need to bother with any of that, so we'll just return - // ECS_ENABLED. + // 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 location (e.g. This PC or Quick Access), we should hide + // the verb from the context menu. + wil::com_ptr_nothrow psi; + RETURN_IF_FAILED(GetBestLocationFromSelectionOrSite(psiItemArray, psi.put())); - *pCmdState = ECS_ENABLED; + SFGAOF attributes; + const bool isFileSystemItem = psi && (psi->GetAttributes(SFGAO_FILESYSTEM, &attributes) == S_OK); + *pCmdState = isFileSystemItem ? ECS_ENABLED : ECS_HIDDEN; return S_OK; } @@ -145,102 +141,47 @@ 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_ = site; + return S_OK; +} - if (shell == nullptr) - { - return path; - } +IFACEMETHODIMP OpenTerminalHere::GetSite(REFIID riid, void** site) noexcept +{ + RETURN_IF_FAILED(site_.query_to(riid, site)); + return S_OK; +} - com_ptr disp; - wil::unique_variant variant; - variant.vt = VT_I4; +HRESULT OpenTerminalHere::GetLocationFromSite(IShellItem** location) const noexcept +{ + wil::com_ptr_nothrow serviceProvider; + RETURN_IF_FAILED(site_.query_to(serviceProvider.put())); + wil::com_ptr_nothrow 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; +} - 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 noexcept +{ + wil::com_ptr_nothrow 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); + RETURN_IF_FAILED(psi.copy_to(location)); + return S_OK; } diff --git a/src/cascadia/ShellExtension/OpenTerminalHere.h b/src/cascadia/ShellExtension/OpenTerminalHere.h index a5af440fb50..bfcf660faee 100644 --- a/src/cascadia/ShellExtension/OpenTerminalHere.h +++ b/src/cascadia/ShellExtension/OpenTerminalHere.h @@ -22,8 +22,6 @@ Author(s): --*/ #pragma once -#include - using namespace Microsoft::WRL; struct @@ -34,7 +32,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 +50,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 noexcept; + HRESULT GetBestLocationFromSelectionOrSite(IShellItemArray* psiArray, IShellItem** location) const noexcept; + + wil::com_ptr_nothrow site_; }; CoCreatableClass(OpenTerminalHere);