Skip to content

Commit

Permalink
OpenHere: Replace explorer window lookup code w/ site lookup (#14048)
Browse files Browse the repository at this point in the history
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 <johnlue@microsoft.com>
  • Loading branch information
DHowett and luedersj authored Sep 21, 2022
1 parent b3c9f01 commit 5027c80
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 125 deletions.
169 changes: 48 additions & 121 deletions src/cascadia/ShellExtension/OpenTerminalHere.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<IShellItem> 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<IShellItem> 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<std::wstring>(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(),
Expand All @@ -72,6 +60,7 @@ HRESULT OpenTerminalHere::Invoke(IShellItemArray* psiItemArray,

return S_OK;
}
CATCH_RETURN()

HRESULT OpenTerminalHere::GetToolTip(IShellItemArray* /*psiItemArray*/,
LPWSTR* ppszInfoTip)
Expand Down Expand Up @@ -109,21 +98,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<IShellItem> psi;
psiItemArray->GetItemAt(0, psi.put());
SFGAOF attributes;
const bool isFileSystemItem = (psi->GetAttributes(SFGAO_FILESYSTEM, &attributes) == S_OK);
*pCmdState = isFileSystemItem ? ECS_ENABLED : ECS_HIDDEN;
}
wil::com_ptr_nothrow<IShellItem> 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;
}
Expand Down Expand Up @@ -160,102 +142,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<IShellWindows> shell;
try
{
shell = create_instance<IShellWindows>(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<IDispatch> disp;
wil::unique_variant variant;
variant.vt = VT_I4;
HRESULT OpenTerminalHere::GetLocationFromSite(IShellItem** location) const noexcept
{
wil::com_ptr_nothrow<IServiceProvider> serviceProvider;
RETURN_IF_FAILED(site_.query_to(serviceProvider.put()));
wil::com_ptr_nothrow<IFolderView> 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<IWebBrowserApp> 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<IShellItem> psi;
if (psiArray)
{
com_ptr<IWebBrowserApp> tmp;
if (FAILED(disp->QueryInterface(tmp.put())))
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.
{
disp = nullptr; // get rid of DEBUG non-nullptr warning
continue;
RETURN_IF_FAILED(psiArray->GetItemAt(0, psi.put()));
}

HWND tmpHWND = NULL;
hr = tmp->get_HWND(reinterpret_cast<SHANDLE_PTR*>(&tmpHWND));
if (hwnd == tmpHWND)
{
browser = tmp;
disp = nullptr; // get rid of DEBUG non-nullptr warning
break; //found
}

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;
}
13 changes: 9 additions & 4 deletions src/cascadia/ShellExtension/OpenTerminalHere.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ Author(s):
--*/
#pragma once

#include <conattrs.hpp>

using namespace Microsoft::WRL;

struct
Expand All @@ -34,7 +32,7 @@ struct
#else // DEV
__declspec(uuid("52065414-e077-47ec-a3ac-1cc5455e1b54"))
#endif
OpenTerminalHere : public RuntimeClass<RuntimeClassFlags<ClassicCom | InhibitFtmBase>, IExplorerCommand>
OpenTerminalHere : public RuntimeClass<RuntimeClassFlags<ClassicCom | InhibitFtmBase>, IExplorerCommand, IObjectWithSite>
{
#pragma region IExplorerCommand
STDMETHODIMP Invoke(IShellItemArray* psiItemArray,
Expand All @@ -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<IUnknown> site_;
};

CoCreatableClass(OpenTerminalHere);

0 comments on commit 5027c80

Please sign in to comment.