Skip to content

TerminalControl fix: Invalidate hovered hyperlink on scroll#20239

Closed
VasuBhakt wants to merge 2 commits into
microsoft:mainfrom
VasuBhakt:fix/stale-hyperlink-hover-on-scroll
Closed

TerminalControl fix: Invalidate hovered hyperlink on scroll#20239
VasuBhakt wants to merge 2 commits into
microsoft:mainfrom
VasuBhakt:fix/stale-hyperlink-hover-on-scroll

Conversation

@VasuBhakt

@VasuBhakt VasuBhakt commented May 17, 2026

Copy link
Copy Markdown

Summary of the Pull Request

Invalidate the cached hovered hyperlink state after scrolling, so the hover underline is re-evaluated when the viewport shifts under a stationary cursor.

Detailed Description of the Pull Request / Additional comments

When the user scrolls with the mouse wheel without moving the cursor, _mouseScrollHandler calls UpdateScrollbar which shifts the viewport, but _lastHoveredCell was never invalidated. Since the cached viewport coordinates hadn't changed, _updateHoveredCell hit its early-return guard and skipped re-querying the buffer entirely. This left the hover underline stuck on a hyperlink that had scrolled away, even when plain text was now under the cursor.

The fix adds ControlCore::_refreshHoveredCell() and calls it in updateScrollBar for non-user-initiated scrolling and UserScrollViewport for user-initiated scrolling. This ensures the hover state always reflects the actual buffer cell under the cursor after a scroll.

Validation Steps Performed

Manually reproduced the bug as described in #20219, hovered an OSC 8 hyperlink in Windows Terminal, kept the mouse stationary, and scrolled with the mouse wheel. The hover underline visibly persisted on the scrolled-away link.

PR Checklist

Closes #20219

When the user scrolls with the mouse wheel while the cursor is
stationary, the viewport shifts but _lastHoveredCell was never
invalidated. This caused stale hyperlink hover highlights to persist
even when plain text was now under the cursor.

Fix by clearing and re-evaluating the hovered cell immediately after
UpdateScrollbar in _mouseScrollHandler.

Closes microsoft#20219

@carlos-zamora carlos-zamora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Sorry for the delay.

Looking through the code, there's a few other places that the same bug would still occur:

  • keyboard scrolling
  • new-output auto-scroll

I'm going ahead and pushing new commits to this PR with an implementation that addresses those other scenarios too. That way, you still get credit for your work.

Test script:
Test-HyperlinkHover.zip

Thanks again!

@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 23, 2026
@carlos-zamora

Copy link
Copy Markdown
Member

Oh! Actually, since the CLA hasn't been accepted, I'll just open a new PR. Sorry for the confusion!

@VasuBhakt

Copy link
Copy Markdown
Author

Hi @carlos-zamora, thanks for the review! Will make sure to sign the CLA for future contributions. Just wanted to clarify: will I be considered a contributor on this issue, since you opened a new PR? If yes it would be a great addition to my open source contributions!

@microsoft-github-policy-service microsoft-github-policy-service Bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jun 24, 2026
carlos-zamora added a commit that referenced this pull request Jul 10, 2026
## Summary of the Pull Request
Updates the cached hovered hyperlink whenever a scroll occurs. All
changes were made in `ControlCore`:
- Add `_refreshHoveredCell()` which simply sets the hovered cell again
- Calls the new function in `UserScrollViewport()` for user-initiated
scrolling and `updateScrollBar` for non-user-initiated scrolling

## Validation Steps Performed
With multiple hyperlinks in the text buffer, the following scenarios
resulted in the hovered hyperlink being updated properly:
✅ mouse wheel scrolling
✅ keyboard scrolling
✅ scroll to mark

## PR Checklist
Closes #20219

Based on #20239
DHowett pushed a commit that referenced this pull request Jul 10, 2026
## Summary of the Pull Request
Updates the cached hovered hyperlink whenever a scroll occurs. All
changes were made in `ControlCore`:
- Add `_refreshHoveredCell()` which simply sets the hovered cell again
- Calls the new function in `UserScrollViewport()` for user-initiated
scrolling and `updateScrollBar` for non-user-initiated scrolling

## Validation Steps Performed
With multiple hyperlinks in the text buffer, the following scenarios
resulted in the hovered hyperlink being updated properly:
✅ mouse wheel scrolling
✅ keyboard scrolling
✅ scroll to mark

## PR Checklist
Closes #20219

Based on #20239

(cherry picked from commit 20f5861)
Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgwo_rU
Service-Version: 1.24
DHowett pushed a commit that referenced this pull request Jul 10, 2026
## Summary of the Pull Request
Updates the cached hovered hyperlink whenever a scroll occurs. All
changes were made in `ControlCore`:
- Add `_refreshHoveredCell()` which simply sets the hovered cell again
- Calls the new function in `UserScrollViewport()` for user-initiated
scrolling and `updateScrollBar` for non-user-initiated scrolling

## Validation Steps Performed
With multiple hyperlinks in the text buffer, the following scenarios
resulted in the hovered hyperlink being updated properly:
✅ mouse wheel scrolling
✅ keyboard scrolling
✅ scroll to mark

## PR Checklist
Closes #20219

Based on #20239

(cherry picked from commit 20f5861)
Service-Card-Id: PVTI_lADOAF3p4s4BQX0-zgwo_rQ
Service-Version: 1.25
@VasuBhakt VasuBhakt deleted the fix/stale-hyperlink-hover-on-scroll branch July 11, 2026 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs-Attention The core contributors need to come back around and look at this ASAP.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hovered hyperlink state is not invalidated when buffer content or viewport mapping changes under a stationary cursor

2 participants