Repository: microsoft/terminal (language: cpp)

Issue:
TermControl does not receive enough `_CursorPositionChanged` events
### Windows Terminal version (or Windows build number)

1.12

### Other Software

_No response_

### Steps to reproduce

1. Start Terminal in debug mode
2. Set a breakpoint at `TermControl::_CursorPositionChanged`
3. Enter some input (without any newlines)

### Expected Behavior

The breakpoint at `TermControl::_CursorPositionChanged` should get hit

### Actual Behavior

The breakpoint does not get hit, it only gets hit when the input contains a newline
TermControl does not receive enough `_CursorPositionChanged` events
### Windows Terminal version (or Windows build number)

1.12

### Other Software

_No response_

### Steps to reproduce

1. Start Terminal in debug mode
2. Set a breakpoint at `TermControl::_CursorPositionChanged`
3. Enter some input (without any newlines)

### Expected Behavior

The breakpoint at `TermControl::_CursorPositionChanged` should get hit

### Actual Behavior

The breakpoint does not get hit, it only gets hit when the input contains a newline


Discussion / hints:
I believe @DHowett had a suspicion for when this may have regressed
Indeed! We stopped doing it when we added position deferral. This results in the IME not tracking the cursor position when it changes within a line.
> This results in the IME not tracking the cursor position when it changes within a line

That sounds... pretty bad actually. Should this be fixed for 1.12 Stable?
Yes, we should. Good catch!
I believe @DHowett had a suspicion for when this may have regressed
Indeed! We stopped doing it when we added position deferral. This results in the IME not tracking the cursor position when it changes within a line.
> This results in the IME not tracking the cursor position when it changes within a line

That sounds... pretty bad actually. Should this be fixed for 1.12 Stable?
Yes, we should. Good catch!

Fix patch:
diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
index bcb6fa07b3f..d03bcbb3abb 100644
--- a/src/cascadia/TerminalCore/Terminal.cpp
+++ b/src/cascadia/TerminalCore/Terminal.cpp
@@ -431,7 +431,21 @@ void Terminal::Write(std::wstring_view stringView)
 {
     auto lock = LockForWriting();
 
+    auto& cursor = _buffer->GetCursor();
+    const til::point cursorPosBefore{ cursor.GetPosition() };
+
     _stateMachine->ProcessString(stringView);
+
+    const til::point cursorPosAfter{ cursor.GetPosition() };
+
+    // Firing the CursorPositionChanged event is very expensive so we try not to
+    // do that when the cursor does not need to be redrawn. We don't do this
+    // inside _AdjustCursorPosition, only once we're done writing the whole run
+    // of output.
+    if (cursorPosBefore != cursorPosAfter)
+    {
+        _NotifyTerminalCursorPositionChanged();
+    }
 }
 
 void Terminal::WritePastedText(std::wstring_view stringView)
@@ -1093,13 +1107,6 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
         COORD delta{ 0, gsl::narrow_cast<short>(-rowsPushedOffTopOfBuffer) };
         _buffer->GetRenderTarget().TriggerScroll(&delta);
     }
-
-    // Firing the CursorPositionChanged event is very expensive so we try not to do that when
-    // the cursor does not need to be redrawn.
-    if (!cursor.IsDeferDrawing())
-    {
-        _NotifyTerminalCursorPositionChanged();
-    }
 }
 
 void Terminal::UserScrollViewport(const int viewTop)
diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp
index e32c197d3fe..a5cbeea6c6f 100644
--- a/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp
+++ b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp
@@ -51,6 +51,8 @@ class TerminalCoreUnitTests::TerminalBufferTests final
 
     TEST_METHOD(TestGetReverseTab);
 
+    TEST_METHOD(TestCursorNotifications);
+
     TEST_METHOD_SETUP(MethodSetup)
     {
         // STEP 1: Set up the Terminal
@@ -592,3 +594,43 @@ void TerminalBufferTests::TestGetReverseTab()
                          L"Cursor adjusted to last item in the sample list from position beyond end.");
     }
 }
+
+void TerminalBufferTests::TestCursorNotifications()
+{
+    // Test for GH#11170
+
+    // Suppress test exceptions. If they occur in the lambda, they'll just crash
+    // TAEF, which is annoying.
+    const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope;
+
+    bool callbackWasCalled = false;
+    int expectedCallbacks = 0;
+    auto cb = [&expectedCallbacks, &callbackWasCalled]() mutable {
+        Log::Comment(L"Callback triggered");
+        callbackWasCalled = true;
+        expectedCallbacks--;
+        VERIFY_IS_GREATER_THAN_OR_EQUAL(expectedCallbacks, 0);
+    };
+    term->_pfnCursorPositionChanged = cb;
+
+    // The exact number of callbacks here is fungible, if need be.
+
+    expectedCallbacks = 1;
+    callbackWasCalled = false;
+    term->Write(L"Foo");
+    VERIFY_ARE_EQUAL(0, expectedCallbacks);
+    VERIFY_IS_TRUE(callbackWasCalled);
+
+    expectedCallbacks = 1;
+    callbackWasCalled = false;
+    term->Write(L"Foo\r\nBar");
+    VERIFY_ARE_EQUAL(0, expectedCallbacks);
+    VERIFY_IS_TRUE(callbackWasCalled);
+
+    expectedCallbacks = 2; // One for each Write
+    callbackWasCalled = false;
+    term->Write(L"Foo\r\nBar");
+    term->Write(L"Foo\r\nBar");
+    VERIFY_ARE_EQUAL(0, expectedCallbacks);
+    VERIFY_IS_TRUE(callbackWasCalled);
+}


Test patch: