Repository: microsoft/terminal (language: cpp)

Issue:
Search results steal focus and scroll when the buffer changes at all
### Windows Terminal version

1.21.1272.0

### Windows build number

10.0.22631.0

### Other Software

_No response_

### Steps to reproduce

- Fill the buffer with enough text to make it scrollable (for example, using `Get-Random -Count 1000` in PowerShell),
- search something in such a way that the last result is outside the view,
- select a search result which is not the last (optinal),
- type something in the prompt.

### Expected Behavior

While typing in the prompt nothing happen, the select result stays the same, and the view focus the prompt.

### Actual Behavior

When typing, the search appear to retrigger, and the focus goes to the last result, moving the prompt outside view.


Discussion / hints:
Hey, I've been hitting this too!

Marking it up for 1.22 and as P1 - we should fix it and backport to 1.21 before 1.21 goes stable.

Fix patch:
diff --git a/src/buffer/out/search.cpp b/src/buffer/out/search.cpp
index 31c7081b2b9..75143f047ca 100644
--- a/src/buffer/out/search.cpp
+++ b/src/buffer/out/search.cpp
@@ -16,7 +16,7 @@ bool Search::IsStale(const Microsoft::Console::Render::IRenderData& renderData,
            _lastMutationId != renderData.GetTextBuffer().GetLastMutationId();
 }
 
-bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse)
+void Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse)
 {
     const auto& textBuffer = renderData.GetTextBuffer();
 
@@ -30,15 +30,15 @@ bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const st
     _results = std::move(result).value_or(std::vector<til::point_span>{});
     _index = reverse ? gsl::narrow_cast<ptrdiff_t>(_results.size()) - 1 : 0;
     _step = reverse ? -1 : 1;
-    return true;
-}
 
-void Search::MoveToCurrentSelection()
-{
     if (_renderData->IsSelectionActive())
     {
         MoveToPoint(_renderData->GetTextBuffer().ScreenToBufferPosition(_renderData->GetSelectionAnchor()));
     }
+    else if (const auto span = _renderData->GetSearchHighlightFocused())
+    {
+        MoveToPoint(_step > 0 ? span->start : span->end);
+    }
 }
 
 void Search::MoveToPoint(const til::point anchor) noexcept
diff --git a/src/buffer/out/search.h b/src/buffer/out/search.h
index 763b5d40c73..304de1ab889 100644
--- a/src/buffer/out/search.h
+++ b/src/buffer/out/search.h
@@ -36,9 +36,8 @@ class Search final
     Search() = default;
 
     bool IsStale(const Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags) const noexcept;
-    bool Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse);
+    void Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse);
 
-    void MoveToCurrentSelection();
     void MoveToPoint(til::point anchor) noexcept;
     void MovePastPoint(til::point anchor) noexcept;
     void FindNext(bool reverse) noexcept;
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index cd4f5f58b81..5d379ba6c5e 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -1697,38 +1697,41 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     SearchResults ControlCore::Search(SearchRequest request)
     {
         const auto lock = _terminal->LockForWriting();
+
         SearchFlag flags{};
         WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !request.CaseSensitive);
         WI_SetFlagIf(flags, SearchFlag::RegularExpression, request.RegularExpression);
         const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), request.Text, flags);
 
-        if (searchInvalidated || !request.Reset)
+        if (searchInvalidated || !request.ResetOnly)
         {
             std::vector<til::point_span> oldResults;
+            til::point_span oldFocused;
+
+            if (const auto focused = _terminal->GetSearchHighlightFocused())
+            {
+                oldFocused = *focused;
+            }
 
             if (searchInvalidated)
             {
                 oldResults = _searcher.ExtractResults();
                 _searcher.Reset(*_terminal.get(), request.Text, flags, !request.GoForward);
-
-                if (SnapSearchResultToSelection())
-                {
-                    _searcher.MoveToCurrentSelection();
-                    SnapSearchResultToSelection(false);
-                }
-
                 _terminal->SetSearchHighlights(_searcher.Results());
             }
-            else
+
+            if (!request.ResetOnly)
             {
                 _searcher.FindNext(!request.GoForward);
             }
 
-            if (const auto idx = _searcher.CurrentMatch(); idx >= 0)
+            _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(std::max<ptrdiff_t>(0, _searcher.CurrentMatch())));
+            _renderer->TriggerSearchHighlight(oldResults);
+
+            if (const auto focused = _terminal->GetSearchHighlightFocused(); focused && *focused != oldFocused)
             {
-                _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx), request.ScrollOffset);
+                _terminal->ScrollToSearchHighlight(request.ScrollOffset);
             }
-            _renderer->TriggerSearchHighlight(oldResults);
         }
 
         int32_t totalMatches = 0;
@@ -1756,27 +1759,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         const auto lock = _terminal->LockForWriting();
         _terminal->SetSearchHighlights({});
-        _terminal->SetSearchHighlightFocused({}, 0);
+        _terminal->SetSearchHighlightFocused(0);
         _renderer->TriggerSearchHighlight(_searcher.Results());
         _searcher = {};
     }
 
-    // Method Description:
-    // - Tells ControlCore to snap the current search result index to currently
-    //   selected text if the search was started using it.
-    void ControlCore::SnapSearchResultToSelection(bool shouldSnap) noexcept
-    {
-        _snapSearchResultToSelection = shouldSnap;
-    }
-
-    // Method Description:
-    // - Returns true, if we should snap the current search result index to
-    //   the currently selected text after a new search is started, else false.
-    bool ControlCore::SnapSearchResultToSelection() const noexcept
-    {
-        return _snapSearchResultToSelection;
-    }
-
     void ControlCore::Close()
     {
         if (!_IsClosing())
diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h
index 9ac8e5450c3..2e9f2490d9d 100644
--- a/src/cascadia/TerminalControl/ControlCore.h
+++ b/src/cascadia/TerminalControl/ControlCore.h
@@ -228,8 +228,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         SearchResults Search(SearchRequest request);
         const std::vector<til::point_span>& SearchResultRows() const noexcept;
         void ClearSearch();
-        void SnapSearchResultToSelection(bool snap) noexcept;
-        bool SnapSearchResultToSelection() const noexcept;
 
         void LeftClickOnTerminal(const til::point terminalPosition,
                                  const int numberOfClicks,
diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl
index 3be52269a89..ee1a4787e22 100644
--- a/src/cascadia/TerminalControl/ControlCore.idl
+++ b/src/cascadia/TerminalControl/ControlCore.idl
@@ -55,7 +55,7 @@ namespace Microsoft.Terminal.Control
         Boolean GoForward;
         Boolean CaseSensitive;
         Boolean RegularExpression;
-        Boolean Reset;
+        Boolean ResetOnly;
         Int32 ScrollOffset;
     };
 
@@ -148,7 +148,6 @@ namespace Microsoft.Terminal.Control
 
         SearchResults Search(SearchRequest request);
         void ClearSearch();
-        Boolean SnapSearchResultToSelection;
 
         Microsoft.Terminal.Core.Color ForegroundColor { get; };
         Microsoft.Terminal.Core.Color BackgroundColor { get; };
diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
index 565ea02ccf6..5f981f89e65 100644
--- a/src/cascadia/TerminalControl/TermControl.cpp
+++ b/src/cascadia/TerminalControl/TermControl.cpp
@@ -576,7 +576,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     // but since code paths differ, extra work is required to ensure correctness.
                     if (!_core.HasMultiLineSelection())
                     {
-                        _core.SnapSearchResultToSelection(true);
                         const auto selectedLine{ _core.SelectedText(true) };
                         _searchBox->PopulateTextbox(selectedLine);
                     }
@@ -3844,7 +3843,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         const auto goForward = _searchBox->GoForward();
         const auto caseSensitive = _searchBox->CaseSensitive();
         const auto regularExpression = _searchBox->RegularExpression();
-        const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _calculateSearchScrollOffset() };
+        const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _searchScrollOffset };
         _handleSearchResults(_core.Search(request));
     }
 
diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
index c5876e1497c..df24a3e8917 100644
--- a/src/cascadia/TerminalCore/Terminal.cpp
+++ b/src/cascadia/TerminalCore/Terminal.cpp
@@ -1259,15 +1259,17 @@ void Terminal::SetSearchHighlights(const std::vector<til::point_span>& highlight
 // Method Description:
 // - Stores the focused search highlighted region in the terminal
 // - If the region isn't empty, it will be brought into view
-void Terminal::SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset)
+void Terminal::SetSearchHighlightFocused(const size_t focusedIdx) noexcept
 {
     _assertLocked();
     _searchHighlightFocused = focusedIdx;
+}
 
-    // bring the focused region into the view if the index is in valid range
-    if (focusedIdx < _searchHighlights.size())
+void Terminal::ScrollToSearchHighlight(til::CoordType searchScrollOffset)
+{
+    if (_searchHighlightFocused < _searchHighlights.size())
     {
-        const auto focused = til::at(_searchHighlights, focusedIdx);
+        const auto focused = til::at(_searchHighlights, _searchHighlightFocused);
         const auto adjustedStart = til::point{ focused.start.x, std::max(0, focused.start.y - searchScrollOffset) };
         const auto adjustedEnd = til::point{ focused.end.x, std::max(0, focused.end.y - searchScrollOffset) };
         _ScrollToPoints(adjustedStart, adjustedEnd);
diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp
index e9d3f1abd0a..d62a76b3bc8 100644
--- a/src/cascadia/TerminalCore/Terminal.hpp
+++ b/src/cascadia/TerminalCore/Terminal.hpp
@@ -234,7 +234,8 @@ class Microsoft::Terminal::Core::Terminal final :
     void SetClearQuickFixCallback(std::function<void()> pfn) noexcept;
     void SetWindowSizeChangedCallback(std::function<void(int32_t, int32_t)> pfn) noexcept;
     void SetSearchHighlights(const std::vector<til::point_span>& highlights) noexcept;
-    void SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset);
+    void SetSearchHighlightFocused(size_t focusedIdx) noexcept;
+    void ScrollToSearchHighlight(til::CoordType searchScrollOffset);
 
     void BlinkCursor() noexcept;
     void SetCursorOn(const bool isOn) noexcept;
diff --git a/src/interactivity/win32/find.cpp b/src/interactivity/win32/find.cpp
index 033918e3ae5..07996c47767 100644
--- a/src/interactivity/win32/find.cpp
+++ b/src/interactivity/win32/find.cpp
@@ -57,7 +57,6 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l
             if (searcher.IsStale(gci.renderData, lastFindString, flags))
             {
                 searcher.Reset(gci.renderData, lastFindString, flags, reverse);
-                searcher.MoveToCurrentSelection();
             }
             else
             {


Test patch: