Repository: microsoft/terminal (language: cpp)

Issue:
Restoring a session as tall as the buffer breaks *everything*
### Windows Terminal version

1.22

### Steps to reproduce

Somehow, it puts the viewport below the buffer. Permanently!
CNR on 1.21 Preview.


Discussion / hints:

Fix patch:
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index 47ed10b0507..1b6f5ff7e7f 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -1850,33 +1850,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
             if (read < sizeof(buffer))
             {
+                // Normally the cursor should already be at the start of the line, but let's be absolutely sure it is.
+                if (_terminal->GetCursorPosition().x != 0)
+                {
+                    _terminal->Write(L"\r\n");
+                }
+                _terminal->Write(message);
                 break;
             }
         }
-
-        {
-            const auto lock = _terminal->LockForWriting();
-
-            // Normally the cursor should already be at the start of the line, but let's be absolutely sure it is.
-            if (_terminal->GetCursorPosition().x != 0)
-            {
-                _terminal->Write(L"\r\n");
-            }
-
-            _terminal->Write(message);
-
-            // Show 3 lines of scrollback to the user, so they know it's there. Otherwise, in particular with the well
-            // hidden touch scrollbars in WinUI 2 and later, there's no indication that there's something to scroll up to.
-            //
-            // We only show 3 lines because ConPTY doesn't know about our restored buffer contents initially,
-            // and so ReadConsole calls will return whitespace.
-            //
-            // We also avoid using actual newlines or similar here, because if we ever change our text buffer implementation
-            // to actually track the written contents, we don't want this to be part of the next buffer snapshot.
-            const auto cursorPosition = _terminal->GetCursorPosition();
-            const auto y = std::max(0, cursorPosition.y - 4);
-            _terminal->SetViewportPosition({ 0, y });
-        }
     }
 
     void ControlCore::_rendererWarning(const HRESULT hr, wil::zwstring_view parameter)
diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp
index f10bc9c1a17..3348427e0a6 100644
--- a/src/cascadia/TerminalCore/TerminalApi.cpp
+++ b/src/cascadia/TerminalCore/TerminalApi.cpp
@@ -39,15 +39,21 @@ ITerminalApi::BufferState Terminal::GetBufferAndViewport() noexcept
     return { _activeBuffer(), til::rect{ _GetMutableViewport().ToInclusive() }, !_inAltBuffer() };
 }
 
-void Terminal::SetViewportPosition(const til::point position) noexcept
+void Terminal::SetViewportPosition(til::point position) noexcept
 try
 {
     // The viewport is fixed at 0,0 for the alt buffer, so this is a no-op.
     if (!_inAltBuffer())
     {
+        const auto bufferSize = _mainBuffer->GetSize().Dimensions();
+        const auto viewSize = _GetMutableViewport().Dimensions();
+
+        // Ensure the given position is in bounds.
+        position.x = std::clamp(position.x, 0, bufferSize.width - viewSize.width);
+        position.y = std::clamp(position.y, 0, bufferSize.height - viewSize.height);
+
         const auto viewportDelta = position.y - _GetMutableViewport().Origin().y;
-        const auto dimensions = _GetMutableViewport().Dimensions();
-        _mutableViewport = Viewport::FromDimensions(position, dimensions);
+        _mutableViewport = Viewport::FromDimensions(position, viewSize);
         _PreserveUserScrollOffset(viewportDelta);
         _NotifyScrollEvent();
     }


Test patch: