From ee6060b3a4106cac0d9b220023fa002a68a8fc41 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <carlos.zamora@microsoft.com>
Date: Mon, 24 Nov 2025 09:35:52 -0800
Subject: [PATCH] Fix search not scrolling to result past view (#19571)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary of the Pull Request
Fixes a bug where search would not scroll to results just below the
viewport.

This was caused by code intended to scroll the search result in such a
way that it isn't covered by the search box. The scroll offset is
calculated in `TermControl::_calculateSearchScrollOffset()` then handed
down in the `SearchRequest` when conducting a search. This would get to
`Terminal::ScrollToSearchHighlight()` where the offset is applied to the
search result's position so that we would scroll to the adjusted
position.

The adjustment was overly aggressive in that it would apply it to both
"start" and "end". In reality, we don't need to apply it to "end"
because it wouldn't be covered by the search box (we only scroll to end
if it's past the end of the current view anyways).

The fix applies the adjustment only to "start" and only does so if it's
actually in the first few rows that would be covered by the search box.

That unveiled another bug where `Terminal::_ScrollToPoints()` would also
be too aggressive about scrolling the "end" into view. In some testing,
it would generally end up scrolling to the end of the buffer. To fix
this cascading bug, I just had `_ScrollToPoints()` just call
`Terminal::_ScrollToPoint()` (singular, not plural) which is
consistently used throughout the Terminal code for selection (so it's
battle tested).

`_ScrollToPoints()` was kept since it's still used for accessibility
when selecting a new region to keep the new selection in view. It's also
just a nice wrapper that ensures a range is visible (or at least as much
as it could be).

## References and Relevant Issues
Scroll offset was added in #17516

## Validation Steps Performed
✅ search results that would be covered by the search box are still
adjusted
✅ search results that are past the end of the view become visible
✅ UIA still selects properly and brings the selection into view

## PR Checklist
Duncan reported this bug internally, but there doesn't seem to be one on
the repo.
---
 src/cascadia/TerminalCore/Terminal.cpp        | 12 +++++++++---
 .../TerminalCore/terminalrenderdata.cpp       | 19 ++-----------------
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
index b6965e3037..aec06719d7 100644
--- a/src/cascadia/TerminalCore/Terminal.cpp
+++ b/src/cascadia/TerminalCore/Terminal.cpp
@@ -1297,9 +1297,15 @@ void Terminal::ScrollToSearchHighlight(til::CoordType searchScrollOffset)
     if (_searchHighlightFocused < _searchHighlights.size())
     {
         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);
+
+        // Only adjust the y coordinates if "start" is in a row that would be covered by the search box
+        auto adjustedStart = focused.start;
+        const auto firstVisibleRow = _VisibleStartIndex();
+        if (focused.start.y > firstVisibleRow && focused.start.y < firstVisibleRow + searchScrollOffset)
+        {
+            adjustedStart.y = std::max(0, focused.start.y - searchScrollOffset);
+        }
+        _ScrollToPoints(adjustedStart, focused.end);
     }
 }
 
diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp
index f2a9087a90..82f539f18b 100644
--- a/src/cascadia/TerminalCore/terminalrenderdata.cpp
+++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp
@@ -152,29 +152,14 @@ const til::point_span* Terminal::GetSearchHighlightFocused() const noexcept
 // - The updated scroll offset
 til::CoordType Terminal::_ScrollToPoints(const til::point coordStart, const til::point coordEnd)
 {
-    auto notifyScrollChange = false;
     if (coordStart.y < _VisibleStartIndex())
     {
-        // recalculate the scrollOffset
-        _scrollOffset = ViewStartIndex() - coordStart.y;
-        notifyScrollChange = true;
+        _ScrollToPoint(coordStart);
     }
     else if (coordEnd.y > _VisibleEndIndex())
     {
-        // recalculate the scrollOffset, note that if the found text is
-        // beneath the current visible viewport, it may be within the
-        // current mutableViewport and the scrollOffset will be smaller
-        // than 0
-        _scrollOffset = std::max(0, ViewStartIndex() - coordStart.y);
-        notifyScrollChange = true;
+        _ScrollToPoint(coordEnd);
     }
-
-    if (notifyScrollChange)
-    {
-        _activeBuffer().TriggerScroll();
-        _NotifyScrollEvent();
-    }
-
     return _VisibleStartIndex();
 }