Fix detecting partially visible URLs

src/buffer/out/UTextAdapter.cpp

@@ -390,6 +390,9 @@ Microsoft::Console::ICU::unique_utext Microsoft::Console::ICU::UTextFromTextBuff
     utext_setup(&ut, 0, &status);
     FAIL_FAST_IF(status > U_ZERO_ERROR);
 
+    rowBeg = std::max<til::CoordType>(0, rowBeg);
+    rowEnd = std::min(textBuffer.GetSize().BottomExclusive(), rowEnd);
+
     ut.providerProperties = (1 << UTEXT_PROVIDER_LENGTH_IS_EXPENSIVE) | (1 << UTEXT_PROVIDER_STABLE_CHUNKS);
     ut.pFuncs = &utextFuncs;
     ut.context = &textBuffer;

src/cascadia/TerminalCore/Terminal.cpp

@@ -513,52 +513,27 @@ std::wstring Terminal::GetHyperlinkAtViewportPosition(const til::point viewportP
 
 std::wstring Terminal::GetHyperlinkAtBufferPosition(const til::point bufferPos)
 {
+    const auto& buffer = _activeBuffer();
+
     // Case 1: buffer position has a hyperlink stored in the buffer
-    const auto attr = _activeBuffer().GetCellDataAt(bufferPos)->TextAttr();
+    const auto attr = buffer.GetCellDataAt(bufferPos)->TextAttr();
     if (attr.IsHyperlink())
     {
-        return _activeBuffer().GetHyperlinkUriFromId(attr.GetHyperlinkId());
+        return buffer.GetHyperlinkUriFromId(attr.GetHyperlinkId());
     }
 
     // Case 2: buffer position may point to an auto-detected hyperlink
-    // Case 2 - Step 1: get the auto-detected hyperlink
-    std::optional<interval_tree::Interval<til::point, size_t>> result;
-    const auto visibleViewport = _GetVisibleViewport();
-    if (visibleViewport.IsInBounds(bufferPos))
+    // Check cached interval tree (covers visible viewport +/- viewport height)
+    if (const auto results = _patternIntervalTree.findOverlapping({ bufferPos.x + 1, bufferPos.y }, bufferPos); !results.empty())
     {
-        // Hyperlink is in the current view, so let's just get it
-        auto viewportPos = bufferPos;
-        visibleViewport.ConvertToOrigin(&viewportPos);
-        result = GetHyperlinkIntervalFromViewportPosition(viewportPos);
-        if (result.has_value())
-        {
-            result->start = _ConvertToBufferCell(result->start, false);
-            result->stop = _ConvertToBufferCell(result->stop, true);
-        }
-    }
-    else
-    {
-        // Hyperlink is outside of the current view.
-        // We need to find if there's a pattern at that location.
-        const auto patterns = _getPatterns(bufferPos.y, bufferPos.y);
-
-        // NOTE: patterns is stored with top y-position being 0,
-        //       so we need to cleverly set the y-pos to 0.
-        const til::point viewportPos{ bufferPos.x, 0 };
-        const auto results = patterns.findOverlapping(viewportPos, viewportPos);
-        if (!results.empty())
+        for (const auto& result : results)
         {
-            result = results.front();
-            result->start.y += bufferPos.y;
-            result->stop.y += bufferPos.y;
+            if (result.value == _hyperlinkPatternId)
+            {
+                return buffer.GetPlainText(result.start, result.stop);
+            }
         }
     }
-
-    // Case 2 - Step 2: get the auto-detected hyperlink
-    if (result.has_value() && result->value == _hyperlinkPatternId)
-    {
-        return _activeBuffer().GetPlainText(result->start, result->stop);
-    }
     return {};
 }
 
@@ -578,17 +553,25 @@ uint16_t Terminal::GetHyperlinkIdAtViewportPosition(const til::point viewportPos
 // Arguments:
 // - The position relative to the viewport
 // Return value:
-// - The interval representing the start and end coordinates
+// - The interval representing the start and end coordinates (viewport-relative)
 std::optional<PointTree::interval> Terminal::GetHyperlinkIntervalFromViewportPosition(const til::point viewportPos)
 {
-    const auto results = _patternIntervalTree.findOverlapping({ viewportPos.x + 1, viewportPos.y }, viewportPos);
+    // GH#18177: The tree stores buffer-absolute coordinates
+    // Convert viewport-relative (y=0 at visible start) to buffer-absolute
+    const auto visStart = _VisibleStartIndex();
+    const til::point bufferPos{ viewportPos.x, viewportPos.y + visStart };
+    const auto results = _patternIntervalTree.findOverlapping({ bufferPos.x + 1, bufferPos.y }, bufferPos);
     if (results.size() > 0)
     {
         for (const auto& result : results)
         {
             if (result.value == _hyperlinkPatternId)
             {
-                return result;
+                // Convert back to viewport-relative coordinates
+                auto interval = result;
+                interval.start.y -= visStart;
+                interval.stop.y -= visStart;
+                return interval;
             }
         }
     }
@@ -807,11 +790,8 @@ TerminalInput::OutputType Terminal::FocusChanged(const bool focused)
 // - The interval tree containing regions that need to be invalidated
 void Terminal::_InvalidatePatternTree()
 {
-    const auto vis = _VisibleStartIndex();
     _patternIntervalTree.visit_all([&](const PointTree::interval& interval) {
-        const til::point startCoord{ interval.start.x, interval.start.y + vis };
-        const til::point endCoord{ interval.stop.x, interval.stop.y + vis };
-        _InvalidateFromCoords(startCoord, endCoord);
+        _InvalidateFromCoords(interval.start, interval.stop);
     });
 }
 
@@ -1209,7 +1189,16 @@ void Terminal::SetPlayMidiNoteCallback(std::function<void(const int, const int,
 void Terminal::UpdatePatternsUnderLock()
 {
     _InvalidatePatternTree();
-    _patternIntervalTree = _getPatterns(_VisibleStartIndex(), _VisibleEndIndex());
+    const auto visStart = _VisibleStartIndex();
+    const auto visEnd = _VisibleEndIndex();
+    const auto viewportHeight = visEnd - visStart;
+
+    // GH#18177: Scan extra rows beyond the viewport so that URLs
+    // wrapping across the viewport boundary are matched in full
+    const auto bufferSize = _activeBuffer().GetSize();
+    const auto beg = std::max<til::CoordType>(0, visStart - viewportHeight);
+    const auto end = std::min(bufferSize.BottomInclusive(), visEnd + viewportHeight);
+    _patternIntervalTree = _getPatterns(beg, end);
     _InvalidatePatternTree();
 }
 
@@ -1424,10 +1413,8 @@ PointTree Terminal::_getPatterns(til::CoordType beg, til::CoordType end) const
         {
             do
             {
-                auto range = ICU::BufferRangeFromMatch(&text, re.get());
-                // PointTree uses half-open ranges and viewport-relative coordinates.
-                range.start.y -= beg;
-                range.end.y -= beg;
+                // PointTree uses half-open ranges and buffer-absolute coordinates.
+                const auto range = ICU::BufferRangeFromMatch(&text, re.get());
                 intervals.push_back(PointTree::interval(range.start, range.end, 0));
             } while (uregex_findNext(re.get(), &status));
         }

src/cascadia/TerminalCore/Terminal.hpp

@@ -204,7 +204,7 @@ class Microsoft::Terminal::Core::Terminal final :
     bool IsGridLineDrawingAllowed() noexcept override;
     std::wstring GetHyperlinkUri(uint16_t id) const override;
     std::wstring GetHyperlinkCustomId(uint16_t id) const override;
-    std::vector<size_t> GetPatternId(const til::point location) const override;
+    std::vector<size_t> GetPatternId(const til::point viewportPos) const override;
 
     std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
     std::span<const til::point_span> GetSelectionSpans() const noexcept override;
@@ -397,7 +397,6 @@ class Microsoft::Terminal::Core::Terminal final :
     std::wstring _wordDelimiters;
     SelectionExpansion _multiClickSelectionMode = SelectionExpansion::Char;
     SelectionInteractionMode _selectionMode = SelectionInteractionMode::None;
-    bool _selectionIsTargetingUrl = false;
     SelectionEndpoint _selectionEndpoint = SelectionEndpoint::None;
     bool _anchorInactiveSelectionEndpoint = false;
 #pragma endregion

src/cascadia/TerminalCore/TerminalSelection.cpp

@@ -254,7 +254,6 @@ void Terminal::_SetSelectionEnd(SelectionInfo* selection, const til::point viewp
         std::tie(selection->start, selection->end) = expandedAnchors;
     }
     _selectionMode = SelectionInteractionMode::Mouse;
-    _selectionIsTargetingUrl = false;
 }
 
 // Method Description:
@@ -399,7 +398,6 @@ void Terminal::ToggleMarkMode()
         }
         _ScrollToPoint(_selection->start);
         _selectionMode = SelectionInteractionMode::Mark;
-        _selectionIsTargetingUrl = false;
     }
 }
 
@@ -460,7 +458,6 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
     if (_selectionMode != SelectionInteractionMode::Mark)
     {
         // This feature only works in mark mode
-        _selectionIsTargetingUrl = false;
         return;
     }
 
@@ -469,19 +466,10 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
     const auto bufferSize = buffer.GetSize();
     const auto viewportHeight = _GetMutableViewport().Height();
 
-    // The patterns are stored relative to the "search area". Initially, this search area will be the viewport,
-    // but as we progressively search through more of the buffer, this will change.
-    // Keep track of the search area here, and use the lambda below to convert points to the search area coordinate space.
-    auto searchArea = _GetVisibleViewport();
-    auto convertToSearchArea = [&searchArea](const til::point pt) noexcept {
-        auto copy = pt;
-        searchArea.ConvertToOrigin(&copy);
-        return copy;
-    };
-
     // extracts the next/previous hyperlink from the list of hyperlink ranges provided
     auto extractResultFromList = [&](std::vector<interval_tree::Interval<til::point, size_t>>& list) noexcept {
-        const auto selectionStartInSearchArea = convertToSearchArea(_selection->start);
+        const auto selectionStart = _selection->start;
+        const auto selectionEnd = _selection->end;
 
         std::optional<std::pair<til::point, til::point>> resultFromList;
         if (!list.empty())
@@ -491,12 +479,11 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
                 // pattern tree includes the currently selected range when going forward,
                 // so we need to check if we're pointing to that one before returning it.
                 auto range = list.front();
-                if (_selectionIsTargetingUrl && range.start == selectionStartInSearchArea)
+                if (range.start == selectionStart && range.stop == selectionEnd)
                 {
                     if (list.size() > 1)
                     {
-                        // if we're pointing to the currently selected URL,
-                        // pick the next one.
+                        // Selection matches the current URL, pick the next one
                         range = til::at(list, 1);
                         resultFromList = { range.start, range.stop };
                     }
@@ -521,14 +508,6 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
                 resultFromList = { range.start, range.stop };
             }
         }
-
-        // pattern tree stores everything as viewport coords,
-        // so we need to convert them on the way out
-        if (resultFromList)
-        {
-            searchArea.ConvertFromOrigin(&resultFromList->first);
-            searchArea.ConvertFromOrigin(&resultFromList->second);
-        }
         return resultFromList;
     };
 
@@ -546,8 +525,8 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
         searchEnd = _selection->start;
     }
 
-    // 1.A) Try searching the current viewport (no scrolling required)
-    auto resultList = _patternIntervalTree.findContained(convertToSearchArea(searchStart), convertToSearchArea(searchEnd));
+    // 1.A) Try searching the cached pattern tree (no scanning required)
+    auto resultList = _patternIntervalTree.findContained(searchStart, searchEnd);
     std::optional<std::pair<til::point, til::point>> result = extractResultFromList(resultList);
     if (!result)
     {
@@ -562,14 +541,12 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
             searchEnd = { bufferSize.RightInclusive(), searchStart.y - 1 };
             searchStart = { bufferSize.Left(), std::max(searchStart.y - viewportHeight, bufferSize.Top()) };
         }
-        searchArea = Viewport::FromDimensions(searchStart, { searchEnd.x + 1, searchEnd.y + 1 });
-
         const til::point bufferStart{ bufferSize.Origin() };
         const til::point bufferEnd{ bufferSize.RightInclusive(), ViewEndIndex() };
         while (!result && bufferSize.IsInBounds(searchStart) && bufferSize.IsInBounds(searchEnd) && searchStart <= searchEnd && bufferStart <= searchStart && searchEnd <= bufferEnd)
         {
             auto patterns = _getPatterns(searchStart.y, searchEnd.y);
-            resultList = patterns.findContained(convertToSearchArea(searchStart), convertToSearchArea(searchEnd));
+            resultList = patterns.findContained(searchStart, searchEnd);
             result = extractResultFromList(resultList);
             if (!result)
             {
@@ -583,7 +560,6 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
                     searchEnd.y -= 1;
                     searchStart.y = std::max(searchEnd.y - viewportHeight, bufferSize.Top());
                 }
-                searchArea = Viewport::FromDimensions(searchStart, { searchEnd.x + 1, searchEnd.y + 1 });
             }
         }
     }
@@ -656,11 +632,10 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
     selection->start = result->first;
     selection->pivot = result->first;
     selection->end = result->second;
-    _selectionIsTargetingUrl = true;
     _selectionEndpoint = SelectionEndpoint::End;
 
     // 4. Scroll to the selected area (if necessary)
-    _ScrollToPoint(selection->end);
+    _ScrollToPoint(dir == SearchDirection::Forward ? selection->end : selection->start);
 }
 
 Terminal::UpdateSelectionParams Terminal::ConvertKeyEventToUpdateSelectionParams(const ControlKeyStates mods, const WORD vkey) const noexcept
@@ -768,7 +743,6 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion
     }
 
     // 3. Actually modify the selection state
-    _selectionIsTargetingUrl = false;
     _selectionMode = std::max(_selectionMode, SelectionInteractionMode::Keyboard);
 
     auto selection{ _selection.write() };
@@ -811,7 +785,6 @@ void Terminal::SelectAll()
     };
 
     _selectionMode = SelectionInteractionMode::Keyboard;
-    _selectionIsTargetingUrl = false;
     _ScrollToPoint(_selection->start);
 }
 
@@ -943,7 +916,6 @@ void Terminal::ClearSelection()
     _assertLocked();
     _selection.write()->active = false;
     _selectionMode = SelectionInteractionMode::None;
-    _selectionIsTargetingUrl = false;
     _selectionEndpoint = static_cast<SelectionEndpoint>(0);
     _anchorInactiveSelectionEndpoint = false;
 }
@@ -1045,5 +1017,9 @@ void Terminal::_ScrollToPoint(const til::point pos)
         }
         _NotifyScrollEvent();
         _activeBuffer().TriggerScroll();
+
+        // Rebuild the pattern tree for the new viewport position
+        // so that callers always find a fresh cache after scrolling
+        _updateUrlDetection();
     }
 }

src/cascadia/TerminalCore/terminalrenderdata.cpp

@@ -74,15 +74,16 @@ std::wstring Microsoft::Terminal::Core::Terminal::GetHyperlinkCustomId(uint16_t
 // Method Description:
 // - Gets the regex pattern ids of a location
 // Arguments:
-// - The location
+// - The viewport-relative location
 // Return value:
 // - The pattern IDs of the location
-std::vector<size_t> Terminal::GetPatternId(const til::point location) const
+std::vector<size_t> Terminal::GetPatternId(const til::point viewportPos) const
 {
     _assertLocked();
 
-    // Look through our interval tree for this location
-    const auto intervals = _patternIntervalTree.findOverlapping({ location.x + 1, location.y }, location);
+    // Convert viewport-relative (y=0 at visible start) to buffer-absolute
+    const til::point bufferPos{ viewportPos.x, viewportPos.y + _VisibleStartIndex() };
+    const auto intervals = _patternIntervalTree.findOverlapping({ bufferPos.x + 1, bufferPos.y }, bufferPos);
     if (intervals.size() == 0)
     {
         return {};