Properly filter focus events from the console API

src/host/input.cpp

@@ -195,8 +195,7 @@ void HandleFocusEvent(const BOOL fSetFocus)
 
     try
     {
-        const auto EventsWritten = gci.pInputBuffer->Write(std::make_unique<FocusEvent>(!!fSetFocus));
-        FAIL_FAST_IF(EventsWritten != 1);
+        gci.pInputBuffer->WriteFocusEvent(fSetFocus);
     }
     catch (...)
     {

src/host/inputBuffer.cpp

@@ -682,6 +682,56 @@ size_t InputBuffer::Write(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& inEv
     }
 }
 
+// This can be considered a "privileged" variant of Write() which allows FOCUS_EVENTs to generate focus VT sequences.
+// If we didn't do this, someone could write a FOCUS_EVENT_RECORD with WriteConsoleInput, exit without flushing the
+// input buffer and the next application will suddenly get a "\x1b[I" sequence in their input. See GH#13238.
+void InputBuffer::WriteFocusEvent(bool focused) noexcept
+{
+    if (IsInVirtualTerminalInputMode())
+    {
+        _termInput.HandleFocus(focused);
+    }
+    else
+    {
+        // This is a mini-version of Write().
+        const auto wasEmpty = _storage.empty();
+        _storage.push_back(std::make_unique<FocusEvent>(focused));
+        if (wasEmpty)
+        {
+            ServiceLocator::LocateGlobals().hInputEvent.SetEvent();
+        }
+        WakeUpReadersWaitingForData();
+    }
+}
+
+// Returns true when mouse input started. You should then capture the mouse and produce further events.
+bool InputBuffer::WriteMouseEvent(til::point position, const unsigned int button, const short keyState, const short wheelDelta)
+{
+    if (IsInVirtualTerminalInputMode())
+    {
+        // This magic flag is "documented" at https://msdn.microsoft.com/en-us/library/windows/desktop/ms646301(v=vs.85).aspx
+        // "If the high-order bit is 1, the key is down; otherwise, it is up."
+        static constexpr short KeyPressed{ gsl::narrow_cast<short>(0x8000) };
+
+        const TerminalInput::MouseButtonState state{
+            WI_IsFlagSet(OneCoreSafeGetKeyState(VK_LBUTTON), KeyPressed),
+            WI_IsFlagSet(OneCoreSafeGetKeyState(VK_MBUTTON), KeyPressed),
+            WI_IsFlagSet(OneCoreSafeGetKeyState(VK_RBUTTON), KeyPressed)
+        };
+
+        // GH#6401: VT applications should be able to receive mouse events from outside the
+        // terminal buffer. This is likely to happen when the user drags the cursor offscreen.
+        // We shouldn't throw away perfectly good events when they're offscreen, so we just
+        // clamp them to be within the range [(0, 0), (W, H)].
+        const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+        gci.GetActiveOutputBuffer().GetViewport().ToOrigin().Clamp(position);
+
+        return _termInput.HandleMouse(position, button, keyState, wheelDelta, state);
+    }
+
+    return false;
+}
+
 // Routine Description:
 // - Coalesces input events and transfers them to storage queue.
 // Arguments:

src/host/inputBuffer.hpp

@@ -81,6 +81,9 @@ class InputBuffer final : public ConsoleObjectHeader
     size_t Write(_Inout_ std::unique_ptr<IInputEvent> inEvent);
     size_t Write(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& inEvents);
 
+    void WriteFocusEvent(bool focused) noexcept;
+    bool WriteMouseEvent(til::point position, unsigned int button, short keyState, short wheelDelta);
+
     bool IsInVirtualTerminalInputMode() const;
     Microsoft::Console::VirtualTerminal::TerminalInput& GetTerminalInput();
     void SetTerminalConnection(_In_ Microsoft::Console::Render::VtEngine* const pTtyConnection);

src/interactivity/win32/windowio.cpp

@@ -30,10 +30,6 @@ using Microsoft::Console::Interactivity::ServiceLocator;
 // Bit 29 is whether ALT was held when the message was posted.
 #define WM_SYSKEYDOWN_ALT_PRESSED (0x20000000)
 
-// This magic flag is "documented" at https://msdn.microsoft.com/en-us/library/windows/desktop/ms646301(v=vs.85).aspx
-// "If the high-order bit is 1, the key is down; otherwise, it is up."
-static constexpr short KeyPressed{ gsl::narrow_cast<short>(0x8000) };
-
 // ----------------------------
 // Helpers
 // ----------------------------
@@ -119,30 +115,8 @@ bool HandleTerminalMouseEvent(const til::point cMousePosition,
                               const short sModifierKeystate,
                               const short sWheelDelta)
 {
-    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
-    // If the modes don't align, this is unhandled by default.
-    auto fWasHandled = false;
-
-    // Virtual terminal input mode
-    if (IsInVirtualTerminalInputMode())
-    {
-        const TerminalInput::MouseButtonState state{
-            WI_IsFlagSet(OneCoreSafeGetKeyState(VK_LBUTTON), KeyPressed),
-            WI_IsFlagSet(OneCoreSafeGetKeyState(VK_MBUTTON), KeyPressed),
-            WI_IsFlagSet(OneCoreSafeGetKeyState(VK_RBUTTON), KeyPressed)
-        };
-
-        // GH#6401: VT applications should be able to receive mouse events from outside the
-        // terminal buffer. This is likely to happen when the user drags the cursor offscreen.
-        // We shouldn't throw away perfectly good events when they're offscreen, so we just
-        // clamp them to be within the range [(0, 0), (W, H)].
-        auto clampedPosition{ cMousePosition };
-        const auto clampViewport{ gci.GetActiveOutputBuffer().GetViewport().ToOrigin() };
-        clampViewport.Clamp(clampedPosition);
-        fWasHandled = gci.GetActiveInputBuffer()->GetTerminalInput().HandleMouse(clampedPosition, uiButton, sModifierKeystate, sWheelDelta, state);
-    }
-
-    return fWasHandled;
+    const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    return gci.pInputBuffer->WriteMouseEvent(cMousePosition, uiButton, sModifierKeystate, sWheelDelta);
 }
 
 void HandleKeyEvent(const HWND hWnd,

src/terminal/adapter/InteractDispatch.cpp

@@ -240,7 +240,7 @@ bool InteractDispatch::FocusChanged(const bool focused) const
 
         WI_UpdateFlag(gci.Flags, CONSOLE_HAS_FOCUS, shouldActuallyFocus);
         gci.ProcessHandleList.ModifyConsoleProcessFocus(shouldActuallyFocus);
-        gci.pInputBuffer->Write(std::make_unique<FocusEvent>(focused));
+        gci.pInputBuffer->WriteFocusEvent(focused);
     }
     // Does nothing outside of ConPTY. If there's a real HWND, then the HWND is solely in charge.
 

src/terminal/adapter/ut_adapter/inputTest.cpp

@@ -328,14 +328,9 @@ void InputTest::TestFocusEvents()
         auto inputEvent = IInputEvent::Create(irTest);
         VERIFY_ARE_EQUAL(false, pInput->HandleKey(inputEvent.get()), L"Verify FOCUS_EVENT from API was NOT handled.");
     }
-    {
-        auto inputEvent = std::make_unique<FocusEvent>(false);
-        VERIFY_ARE_EQUAL(false, pInput->HandleKey(inputEvent.get()), L"Verify FocusEvent from any other source was NOT handled.");
-    }
-    {
-        auto inputEvent = std::make_unique<FocusEvent>(true);
-        VERIFY_ARE_EQUAL(false, pInput->HandleKey(inputEvent.get()), L"Verify FocusEvent from any other source was NOT handled.");
-    }
+
+    VERIFY_ARE_EQUAL(false, pInput->HandleFocus(false), L"Verify FocusEvent from any other source was NOT handled.");
+    VERIFY_ARE_EQUAL(false, pInput->HandleFocus(true), L"Verify FocusEvent from any other source was NOT handled.");
 
     Log::Comment(L"Enable focus event handling");
 
@@ -351,16 +346,11 @@ void InputTest::TestFocusEvents()
         auto inputEvent = IInputEvent::Create(irTest);
         VERIFY_ARE_EQUAL(false, pInput->HandleKey(inputEvent.get()), L"Verify FOCUS_EVENT from API was NOT handled.");
     }
-    {
-        s_expectedInput = L"\x1b[O";
-        auto inputEvent = std::make_unique<FocusEvent>(false);
-        VERIFY_ARE_EQUAL(true, pInput->HandleKey(inputEvent.get()), L"Verify FocusEvent from any other source was handled.");
-    }
-    {
-        s_expectedInput = L"\x1b[I";
-        auto inputEvent = std::make_unique<FocusEvent>(true);
-        VERIFY_ARE_EQUAL(true, pInput->HandleKey(inputEvent.get()), L"Verify FocusEvent from any other source was handled.");
-    }
+
+    s_expectedInput = L"\x1b[O";
+    VERIFY_ARE_EQUAL(true, pInput->HandleFocus(false), L"Verify FocusEvent from any other source was handled.");
+    s_expectedInput = L"\x1b[I";
+    VERIFY_ARE_EQUAL(true, pInput->HandleFocus(true), L"Verify FocusEvent from any other source was handled.");
 }
 
 void InputTest::TerminalInputModifierKeyTests()

src/terminal/input/terminalInput.cpp

@@ -523,22 +523,6 @@ bool TerminalInput::HandleKey(const IInputEvent* const pInEvent)
         return false;
     }
 
-    // GH#11682: If this was a focus event, we can handle this. Steal the
-    // focused state, and return true if we're actually in focus event mode.
-    if (pInEvent->EventType() == InputEventType::FocusEvent)
-    {
-        const auto& focusEvent = *static_cast<const FocusEvent* const>(pInEvent);
-
-        // BODGY
-        // GH#13238 - Filter out focus events that came from the API.
-        if (focusEvent.CameFromApi())
-        {
-            return false;
-        }
-
-        return HandleFocus(focusEvent.GetFocus());
-    }
-
     // On key presses, prepare to translate to VT compatible sequences
     if (pInEvent->EventType() != InputEventType::KeyEvent)
     {

src/types/inc/IInputEvent.hpp

@@ -515,14 +515,12 @@ class FocusEvent : public IInputEvent
 {
 public:
     constexpr FocusEvent(const FOCUS_EVENT_RECORD& record) :
-        _focus{ !!record.bSetFocus },
-        _cameFromApi{ true }
+        _focus{ !!record.bSetFocus }
     {
     }
 
     constexpr FocusEvent(const bool focus) :
-        _focus{ focus },
-        _cameFromApi{ false }
+        _focus{ focus }
     {
     }
 
@@ -542,15 +540,8 @@ class FocusEvent : public IInputEvent
 
     void SetFocus(const bool focus) noexcept;
 
-    // BODGY - see FocusEvent.cpp for details.
-    constexpr bool CameFromApi() const noexcept
-    {
-        return _cameFromApi;
-    }
-
 private:
     bool _focus;
-    bool _cameFromApi;
 
 #ifdef UNIT_TESTING
     friend std::wostream& operator<<(std::wostream& stream, const FocusEvent* const pFocusEvent);