TerminalControl fix: Invalidate hovered hyperlink on scroll#20239
Closed
VasuBhakt wants to merge 2 commits into
Closed
TerminalControl fix: Invalidate hovered hyperlink on scroll#20239VasuBhakt wants to merge 2 commits into
VasuBhakt wants to merge 2 commits into
Conversation
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
requested changes
Jun 23, 2026
carlos-zamora
left a comment
Member
There was a problem hiding this comment.
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!
Member
|
Oh! Actually, since the CLA hasn't been accepted, I'll just open a new PR. Sorry for the confusion! |
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! |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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,
_mouseScrollHandlercallsUpdateScrollbarwhich shifts the viewport, but_lastHoveredCellwas never invalidated. Since the cached viewport coordinates hadn't changed,_updateHoveredCellhit 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 inupdateScrollBarfor non-user-initiated scrolling andUserScrollViewportfor 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