Repository: microsoft/terminal (language: cpp)

Issue:
Terminal shouldn't be offered as an option in the context menu for Quick Access background
From MSFT:37831236

> When I right-click on the background of File Explorer "Quick Access," Terminal shouldn't be offered as an option because it doesn't do anything there

Presumably we can filter if we're visible based on the path that was provided to us. 




Discussion / hints:
From internal mail:


> So, in their `IExplorerCommand::GetState`, I think they could have a check for that against the `IShellItemArray` that gets passed in.

```c++
bool IsFileSystemItem(IShellItem* shellItem)
{
    SFGAOF attributes;
    return (shellItem->GetAttributes(SFGAO_FILESYSTEM, &attributes) == S_OK);
}
```

We may also want to compare notes with MSFT:36909951
Open with... dialog cannot choose the All the Apps from Microsoft Store
:tada:This issue was addressed in #13206, which has now been successfully released as `Windows Terminal v1.14.186`.:tada:

Handy links:
* [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.14.186)
* [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
:tada:This issue was addressed in #13206, which has now been successfully released as `Windows Terminal Preview v1.15.186`.:tada:

Handy links:
* [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.15.186)
* [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
:tada:This issue was addressed in #13206, which has now been successfully released as `Windows Terminal v1.14.228`.:tada:

Handy links:
* [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.14.228)
* [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
:tada:This issue was addressed in #13206, which has now been successfully released as `Windows Terminal Preview v1.15.228`.:tada:

Handy links:
* [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.15.228)
* [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
Lol the bot thought that it should comment here because of the revert 😅 

I'm reopening this cause we reverted this to fix #13523. I think there's an OS bug, at least on Windows 11, that doesn't let us know the difference between "My PC" and the desktop background. Both show up with an empty array and a null path from explorer. So that's useless. 
Upstream issue is now MSFT:41029797
Regarding https://github.com/microsoft/terminal/issues/13523 

The reason that "Open in Terminal" isn't shown consistently when desktop is right clicked is that on Windows 11, the default context menu is made of fancy XAML controls, and the Window Class name of the foreground window is sometimes reported as `XamlExplorerHostIslandWindow`, not `Progman` or `WorkerW`.

In `OpenTerminalHere::_GetPathFromExplorer()`, I think we can add an additional check to work around this issue.

```
if (0 == StrCmp(szName, L"WorkerW") ||
    0 == StrCmp(szName, L"Progman") ||
    0 == StrCmp(szName, L"XamlExplorerHostIslandWindow"))
```


From https://github.com/microsoft/terminal/pull/13206#issuecomment-1217094214
> Okay I'm pretty sure this caused #13523. Turns out that there's a non-zero rate of the shell giving us 0 items in the array. I can check for that easy enough, but it seems like still returning ECS_HIDDEN for that results in the menu entry not always appearing for the desktop.
> 
> I'll need to noodle more

Is there a way to reproduce the case where `psiItemArray` is not `nullptr` but `psiItemArray->GetCount` returns 0? (I assume this is why `OpenTerminalHere::GetState()` was crashing often). I've never succeeded in observing this behavior.


Also, the [PR](https://github.com/microsoft/terminal/pull/13206) that originally fixed this issue was reverted in the internal repo, right? Will the revert make its way to this public repo?
> Also, the https://github.com/microsoft/terminal/pull/13206 that originally fixed this issue was reverted in the internal repo, right? Will the revert make its way to this public repo?

~~This PR~~ The revert was made against and merged into the public repository. There is no private/internal copy of the Windows Terminal source; we do all development right out here in the open! :smile:

Edited to correct for me having misread your question. The commit that reverts it is 2d00432da on `release-1.14` and 76daf71cf on `release-1.15`. We kept it on in `main` so we could have builds to repro the issue with.
Good find on the `Xaml...Window` thing! Do you think that the rest of the explorer lookup logic will work with that HWND as well?
> Edited to correct for me having misread your question. The commit that reverts it is https://github.com/microsoft/terminal/commit/2d00432dab82f1d46d8cdc9176d5e3e6f34f753a on release-1.14 and https://github.com/microsoft/terminal/commit/76daf71cf96f81a6e0589bdef4f01d325c7e7847 on release-1.15. We kept it on in main so we could have builds to repro the issue with.

Thanks for the clarification!

> Good find on the `Xaml...Window` thing! Do you think that the rest of the explorer lookup logic will work with that HWND as well?

I think so. Even when the context menu is open in Explorer, the foreground window's class name is always reported as `CabinetWClass`.

Fix patch:
diff --git a/src/cascadia/ShellExtension/OpenTerminalHere.cpp b/src/cascadia/ShellExtension/OpenTerminalHere.cpp
index d9b872136da..d2320d24759 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<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(),
@@ -72,6 +60,7 @@ HRESULT OpenTerminalHere::Invoke(IShellItemArray* psiItemArray,
 
     return S_OK;
 }
+CATCH_RETURN()
 
 HRESULT OpenTerminalHere::GetToolTip(IShellItemArray* /*psiItemArray*/,
                                      LPWSTR* ppszInfoTip)
@@ -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;
 }
@@ -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;
 }
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 <conattrs.hpp>
-
 using namespace Microsoft::WRL;
 
 struct
@@ -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,
@@ -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);


Test patch: