Repository: microsoft/terminal (language: cpp)

Issue:
GUI apps still don't open in the foreground, from a default terminal launch
A TCC user said that GUI apps started by TCC in WT don't get focus (unlike other shells in WT and unlike TCC in a console).  I said I didn't think I could get TCC and/or WT to act that way if I tried.  Have I overlooked anything ... any ideas?  Thanks!


Discussion / hints:
This is not actionable without knowing the version of Windows Terminal the TCC user has installed -- we made some significant windowing changes in Preview 1.14.
His setup is the same as mine.  I have no such problem.

v:\> ver
TCC 28.02.18 x64
Microsoft Windows 10 Pro for Workstations
10.0.19044.1706 (2009, 21H2)
WindowsTerminalPreview 1.14.1432.0
The user in question is using WindowsTerminalPreview 1.14.2205.25002.  I am using WindowsTerminalPreview 1.14.2205.23002.

Is that newer build available on GitHub?
I don't think the revision numbers make a difference here. 

Do you have a link to the original bug report?

There was a LOT of work in this area in 1.14, in #2988, #12515, and every thread linked to those. How are the GUI apps getting spawned by TCC? Via `ShellExecute`?
Here's a [link](https://jpsoft.com/forums/threads/tcc-does-not-pass-focus-to-applications-launched-by-tcc-from-windows-terminal.11203/#post-63560) to the original report.

I suspect with CreateProcessW.  WinDbg isn't being very cooperative:

0:001> bp shell32!ShellExecuteW
0:001> bp shell32!ShellExecuteExW
0:001> bp kernel32!CreateProcessW
Couldn't resolve error at 'kernel32!CreateProcessW'

It does not break on ShellEXecute[Ex] when I issue "notepad".

I tried again.  WinDbg does break at CreateProcessW when I issue "notepad".
Mike, if you're following, you'll be  interested in [#18](https://jpsoft.com/forums/threads/tcc-does-not-pass-focus-to-applications-launched-by-tcc-from-windows-terminal.11203/post-63576).  The user reports that whatever shell is in WT's FIRST tab will exhibit this problem.

In a nutshell ... it's not only TCC.
And, if I read his comments accurately, he has a setup in which WT is the default handler for console apps (as opposed to conhost).  Musy be (?) he has some sort of insider status.
I was the originator of this bug report, over on the JPSoft forums.  I mistakenly thought the problem was limited to TCC, but that is not the case.

I am not an “Insider”, this is a public build of Windows Terminal Preview, and a public build of Windows.

Microsoft Windows
Version 21H2 (OS Build 22000.708)


Windows Terminal Preview
Version 1.14.1452.0

I obtained Windows Terminal Preview from the Microsoft Store.  It offered to make itself the default handler for shells, and I accepted the offer.  I have shortcuts on the desktop for cmd.exe and tcc.exe (Jpsoft).  If I click on either of those shortcuts, they start in a new Windows Terminal Preview tabbed window.  And from there, if I start an application (notepad, for instance), the focus stays in the Windows Terminal Preview window, it does not shift to the newly launched application.  The same behavior for tcc and for cmd.  Doesn’t matter if I type in “notepad” or “start notepad”.

Also, if I launch Powershell Preview from the Start Menu, it starts up in a Windows Terminal tabbed window, and exhibits the same behavior.

Now once I have a window up for Windows Terminal Preview, if I start another tab, the problem does not exist in the secondary tab.  And it doesn’t matter what’s running in the secondary tab, cmd, tcc, pwsh, all behave normally.  Launch an application, that new application gets the focus.

And if I kill the primary tab, so the secondary tab becomes the primary tab, there is now no problem in the new primary tab.  

I also have Windows Terminal Version: 1.12.10983.0
Installed on this system.  I have placed a shortcut on the desktop for
C:\Users\Henry\AppData\Local\Microsoft\WindowsApps\Microsoft.WindowsTerminal_8wekyb3d8bbwe\wt.exe c:\tcmd\tcc.exe

That launches tcc under the older version of WT, and the problem does not then exist.

Thanks for all the background guys. I had a feeling this was related to defterm, and everything here confirms that suspicion. I bet the focus/foreground handler isn't triggered for defterm connections. I'll take a look. 
Mike, can defterm be configured manually on Windows 10?  I have onle unzipped installations of WT?  Once installed, by any means, is it easy to switch it on/off?  Thanks!
> can defterm be configured manually on Windows 10?

Nope.

> is it easy to switch it on/off? Thanks!

Yep, it's a dropdown in the Settings UI, in the Settings app, and in the Console properties sheet. 

Fix patch:
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 46889b1db99..ea1bb14a8fb 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -69,6 +69,29 @@ namespace winrt::TerminalApp::implementation
     //   - see GH#2988
     HRESULT TerminalPage::Initialize(HWND hwnd)
     {
+        if (!_hostingHwnd.has_value())
+        {
+            // GH#13211 - if we haven't yet set the owning hwnd, reparent all the controls now.
+            for (const auto& tab : _tabs)
+            {
+                if (auto terminalTab{ _GetTerminalTabImpl(tab) })
+                {
+                    terminalTab->GetRootPane()->WalkTree([&](auto&& pane) {
+                        if (const auto& term{ pane->GetTerminalControl() })
+                        {
+                            term.OwningHwnd(reinterpret_cast<uint64_t>(hwnd));
+                        }
+                    });
+                }
+                // We don't need to worry about resetting the owning hwnd for the
+                // SUI here. GH#13211 only repros for a defterm connection, where
+                // the tab is spawned before the window is created. It's not
+                // possible to make a SUI tab like that, before the window is
+                // created. The SUI could be spawned as a part of a window restore,
+                // but that would still work fine. The window would be created
+                // before restoring previous tabs in that scenario.
+            }
+        }
         _hostingHwnd = hwnd;
         return S_OK;
     }
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index ec470a31dd6..5635f4603d9 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -289,11 +289,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             const auto height = vp.Height();
             _connection.Resize(height, width);
 
-            if (_OwningHwnd != 0)
+            if (_owningHwnd != 0)
             {
                 if (auto conpty{ _connection.try_as<TerminalConnection::ConptyConnection>() })
                 {
-                    conpty.ReparentWindow(_OwningHwnd);
+                    conpty.ReparentWindow(_owningHwnd);
                 }
             }
 
@@ -1881,6 +1881,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         return Opacity() < 1.0f || UseAcrylic() || !_settings->BackgroundImage().empty() || _settings->UseBackgroundImageForWindow();
     }
 
+    uint64_t ControlCore::OwningHwnd()
+    {
+        return _owningHwnd;
+    }
+
+    void ControlCore::OwningHwnd(uint64_t owner)
+    {
+        if (owner != _owningHwnd && _connection)
+        {
+            if (auto conpty{ _connection.try_as<TerminalConnection::ConptyConnection>() })
+            {
+                conpty.ReparentWindow(owner);
+            }
+        }
+        _owningHwnd = owner;
+    }
+
     Windows::Foundation::Collections::IVector<Control::ScrollMark> ControlCore::ScrollMarks() const
     {
         auto internalMarks{ _terminal->GetScrollMarks() };
diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h
index 5f20493db17..0dcfde33359 100644
--- a/src/cascadia/TerminalControl/ControlCore.h
+++ b/src/cascadia/TerminalControl/ControlCore.h
@@ -186,10 +186,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         void WindowVisibilityChanged(const bool showOrHide);
 
-        // TODO:GH#1256 - When a tab can be torn out or otherwise reparented to
-        // another window, this value will need a custom setter, so that we can
-        // also update the connection.
-        WINRT_PROPERTY(uint64_t, OwningHwnd, 0);
+        uint64_t OwningHwnd();
+        void OwningHwnd(uint64_t owner);
 
         RUNTIME_SETTING(double, Opacity, _settings->Opacity());
         RUNTIME_SETTING(bool, UseAcrylic, _settings->UseAcrylic());
@@ -259,6 +257,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         double _panelHeight{ 0 };
         double _compositionScale{ 0 };
 
+        uint64_t _owningHwnd{ 0 };
+
         winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
         std::shared_ptr<ThrottledFuncTrailing<>> _tsfTryRedrawCanvas;
         std::shared_ptr<ThrottledFuncTrailing<>> _updatePatternLocations;
diff --git a/src/host/ConsoleArguments.cpp b/src/host/ConsoleArguments.cpp
index f89bf249732..2541cc8ab82 100644
--- a/src/host/ConsoleArguments.cpp
+++ b/src/host/ConsoleArguments.cpp
@@ -25,6 +25,12 @@ const std::wstring_view ConsoleArguments::FEATURE_ARG = L"--feature";
 const std::wstring_view ConsoleArguments::FEATURE_PTY_ARG = L"pty";
 const std::wstring_view ConsoleArguments::COM_SERVER_ARG = L"-Embedding";
 const std::wstring_view ConsoleArguments::PASSTHROUGH_ARG = L"--passthrough";
+// NOTE: Thinking about adding more commandline args that control conpty, for
+// the Terminal? Make sure you add them to the commandline in
+// ConsoleEstablishHandoff. We use that to initialize the ConsoleArguments for a
+// defterm handoff s.t. they behave like a conpty connection that was started by
+// the terminal. If you forget them there, the conpty won't obey them, only for
+// defterm.
 
 std::wstring EscapeArgument(std::wstring_view ac)
 {
diff --git a/src/host/input.cpp b/src/host/input.cpp
index a689a9454c7..e49dee1a8be 100644
--- a/src/host/input.cpp
+++ b/src/host/input.cpp
@@ -326,7 +326,7 @@ void ProcessCtrlEvents()
         if (NT_SUCCESS(Status))
         {
             Status = ServiceLocator::LocateConsoleControl()
-                         ->EndTask((HANDLE)rgProcessHandleList[i].dwProcessID,
+                         ->EndTask(rgProcessHandleList[i].dwProcessID,
                                    EventType,
                                    CtrlFlags);
             if (rgProcessHandleList[i].hProcess == nullptr)
diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp
index d84f503e29f..285809d7002 100644
--- a/src/host/srvinit.cpp
+++ b/src/host/srvinit.cpp
@@ -525,7 +525,12 @@ try
     outPipeTheirSide.reset();
     signalPipeTheirSide.reset();
 
-    const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --signal {:#x}"), (int64_t)signalPipeOurSide.release());
+    // GH#13211 - Make sure we request win32input mode and that the terminal
+    // obeys the resizing quirk. Otherwise, defterm connections to the Terminal
+    // are going to have weird resizing, and aren't going to send full fidelity
+    // input messages.
+    const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --resizeQuirk --win32input --signal {:#x}"),
+                                         (int64_t)signalPipeOurSide.release());
 
     ConsoleArguments consoleArgs(commandLine, inPipeOurSide.release(), outPipeOurSide.release());
     RETURN_IF_FAILED(consoleArgs.ParseCommandline());
diff --git a/src/inc/HostSignals.hpp b/src/inc/HostSignals.hpp
index 05e1d0f3f72..ec29e540646 100644
--- a/src/inc/HostSignals.hpp
+++ b/src/inc/HostSignals.hpp
@@ -13,21 +13,21 @@ namespace Microsoft::Console
     struct HostSignalNotifyAppData
     {
         uint32_t sizeInBytes;
-        uint32_t processId;
+        uint32_t processId; // THIS IS A PID
     };
 
     struct HostSignalSetForegroundData
     {
         uint32_t sizeInBytes;
-        uint32_t processId;
+        uint32_t processId; // THIS IS A HANDLE, NOT A PID
         bool isForeground;
     };
 
     struct HostSignalEndTaskData
     {
         uint32_t sizeInBytes;
-        uint32_t processId;
+        uint32_t processId; // THIS IS A PID
         uint32_t eventType;
         uint32_t ctrlFlags;
     };
-};
\ No newline at end of file
+};
diff --git a/src/interactivity/base/HostSignalInputThread.cpp b/src/interactivity/base/HostSignalInputThread.cpp
index 397131b08e4..bed01bbd013 100644
--- a/src/interactivity/base/HostSignalInputThread.cpp
+++ b/src/interactivity/base/HostSignalInputThread.cpp
@@ -94,9 +94,11 @@ T HostSignalInputThread::_ReceiveTypedPacket()
         }
         case HostSignals::SetForeground:
         {
-            auto msg = _ReceiveTypedPacket<HostSignalSetForegroundData>();
+            _ReceiveTypedPacket<HostSignalSetForegroundData>();
 
-            LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->SetForeground(ULongToHandle(msg.processId), msg.isForeground));
+            // GH#13211 - This honestly shouldn't be called by OpenConsole
+            // anymore, but it's possible that a much older version of
+            // OpenConsole is still calling this. Just do nothing.
 
             break;
         }
@@ -104,7 +106,7 @@ T HostSignalInputThread::_ReceiveTypedPacket()
         {
             auto msg = _ReceiveTypedPacket<HostSignalEndTaskData>();
 
-            LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->EndTask(ULongToHandle(msg.processId), msg.eventType, msg.ctrlFlags));
+            LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->EndTask(msg.processId, msg.eventType, msg.ctrlFlags));
 
             break;
         }
diff --git a/src/interactivity/base/RemoteConsoleControl.cpp b/src/interactivity/base/RemoteConsoleControl.cpp
index 21b8155a79f..7cd03e36317 100644
--- a/src/interactivity/base/RemoteConsoleControl.cpp
+++ b/src/interactivity/base/RemoteConsoleControl.cpp
@@ -57,19 +57,17 @@ template<typename T>
 
 [[nodiscard]] NTSTATUS RemoteConsoleControl::SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground)
 {
-    HostSignalSetForegroundData data{};
-    data.sizeInBytes = sizeof(data);
-    data.processId = HandleToULong(hProcess);
-    data.isForeground = fForeground;
-
-    return _SendTypedPacket(_pipe.get(), HostSignals::SetForeground, data);
+    // GH#13211 - Apparently this API doesn't need to be forwarded to conhost at
+    // all. Instead, just perform the ConsoleControl operation here, in proc.
+    // This lets us avoid all sorts of strange handle duplicating weirdness.
+    return _control.SetForeground(hProcess, fForeground);
 }
 
-[[nodiscard]] NTSTATUS RemoteConsoleControl::EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags)
+[[nodiscard]] NTSTATUS RemoteConsoleControl::EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags)
 {
     HostSignalEndTaskData data{};
     data.sizeInBytes = sizeof(data);
-    data.processId = HandleToULong(hProcessId);
+    data.processId = dwProcessId;
     data.eventType = dwEventType;
     data.ctrlFlags = ulCtrlFlags;
 
diff --git a/src/interactivity/base/RemoteConsoleControl.hpp b/src/interactivity/base/RemoteConsoleControl.hpp
index 52de8db1eb3..b922b9beaa6 100644
--- a/src/interactivity/base/RemoteConsoleControl.hpp
+++ b/src/interactivity/base/RemoteConsoleControl.hpp
@@ -14,6 +14,7 @@ Author(s):
 #pragma once
 
 #include "../inc/IConsoleControl.hpp"
+#include "../win32/ConsoleControl.hpp"
 
 namespace Microsoft::Console::Interactivity
 {
@@ -25,9 +26,11 @@ namespace Microsoft::Console::Interactivity
         // IConsoleControl Members
         [[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId);
         [[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground);
-        [[nodiscard]] NTSTATUS EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags);
+        [[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags);
 
     private:
         wil::unique_handle _pipe;
+
+        Win32::ConsoleControl _control;
     };
 }
diff --git a/src/interactivity/inc/IConsoleControl.hpp b/src/interactivity/inc/IConsoleControl.hpp
index 02d83e1b97e..7129cff68ff 100644
--- a/src/interactivity/inc/IConsoleControl.hpp
+++ b/src/interactivity/inc/IConsoleControl.hpp
@@ -23,6 +23,6 @@ namespace Microsoft::Console::Interactivity
         virtual ~IConsoleControl() = default;
         [[nodiscard]] virtual NTSTATUS NotifyConsoleApplication(DWORD dwProcessId) = 0;
         [[nodiscard]] virtual NTSTATUS SetForeground(HANDLE hProcess, BOOL fForeground) = 0;
-        [[nodiscard]] virtual NTSTATUS EndTask(HANDLE hProcessId, DWORD dwEventType, ULONG ulCtrlFlags) = 0;
+        [[nodiscard]] virtual NTSTATUS EndTask(DWORD dwProcessId, DWORD dwEventType, ULONG ulCtrlFlags) = 0;
     };
 }
diff --git a/src/interactivity/onecore/ConsoleControl.cpp b/src/interactivity/onecore/ConsoleControl.cpp
index 662a2b6b1e9..248d02b3ac8 100644
--- a/src/interactivity/onecore/ConsoleControl.cpp
+++ b/src/interactivity/onecore/ConsoleControl.cpp
@@ -22,13 +22,13 @@ using namespace Microsoft::Console::Interactivity::OneCore;
     return STATUS_SUCCESS;
 }
 
-[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags)
+[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags)
 {
     USER_API_MSG m{};
     const auto a = &m.u.EndTask;
 
     RtlZeroMemory(a, sizeof(*a));
-    a->ProcessId = hProcessId;
+    a->ProcessId = ULongToHandle(dwProcessId); // This is actually a PID, even though the struct expects a HANDLE.
     a->ConsoleEventCode = dwEventType;
     a->ConsoleFlags = ulCtrlFlags;
 
diff --git a/src/interactivity/onecore/ConsoleControl.hpp b/src/interactivity/onecore/ConsoleControl.hpp
index 9b406a475f4..4771c6c2220 100644
--- a/src/interactivity/onecore/ConsoleControl.hpp
+++ b/src/interactivity/onecore/ConsoleControl.hpp
@@ -26,6 +26,6 @@ namespace Microsoft::Console::Interactivity::OneCore
         // IConsoleControl Members
         [[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId) noexcept override;
         [[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground) noexcept override;
-        [[nodiscard]] NTSTATUS EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) override;
+        [[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) override;
     };
 }
diff --git a/src/interactivity/win32/ConsoleControl.cpp b/src/interactivity/win32/ConsoleControl.cpp
index 4cea4e3857e..78e66110df2 100644
--- a/src/interactivity/win32/ConsoleControl.cpp
+++ b/src/interactivity/win32/ConsoleControl.cpp
@@ -37,12 +37,12 @@ using namespace Microsoft::Console::Interactivity::Win32;
                    sizeof(Flags));
 }
 
-[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags)
+[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags)
 {
     auto pConsoleWindow = ServiceLocator::LocateConsoleWindow();
 
     CONSOLEENDTASK ConsoleEndTaskParams;
-    ConsoleEndTaskParams.ProcessId = hProcessId;
+    ConsoleEndTaskParams.ProcessId = ULongToHandle(dwProcessId); // This is actually a PID, even though the struct expects a HANDLE.
     ConsoleEndTaskParams.ConsoleEventCode = dwEventType;
     ConsoleEndTaskParams.ConsoleFlags = ulCtrlFlags;
     ConsoleEndTaskParams.hwnd = pConsoleWindow == nullptr ? nullptr : pConsoleWindow->GetWindowHandle();
diff --git a/src/interactivity/win32/ConsoleControl.hpp b/src/interactivity/win32/ConsoleControl.hpp
index 9a8ab927fc0..fdd490a528a 100644
--- a/src/interactivity/win32/ConsoleControl.hpp
+++ b/src/interactivity/win32/ConsoleControl.hpp
@@ -47,7 +47,7 @@ namespace Microsoft::Console::Interactivity::Win32
         // IConsoleControl Members
         [[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId);
         [[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground);
-        [[nodiscard]] NTSTATUS EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags);
+        [[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags);
 
         // Public Members
         [[nodiscard]] NTSTATUS Control(_In_ ConsoleControl::ControlType ConsoleCommand,
diff --git a/src/server/ProcessList.cpp b/src/server/ProcessList.cpp
index f852db7728e..ab9c529a76d 100644
--- a/src/server/ProcessList.cpp
+++ b/src/server/ProcessList.cpp
@@ -305,7 +305,7 @@ void ConsoleProcessList::ModifyConsoleProcessFocus(const bool fForeground)
     {
         const auto pProcessHandle = *it;
 
-        if (pProcessHandle->_hProcess != nullptr)
+        if (pProcessHandle->_hProcess)
         {
             _ModifyProcessForegroundRights(pProcessHandle->_hProcess.get(), fForeground);
         }


Test patch: