Drop the link highlight when the alt screen scrolls - #14748
Drop the link highlight when the alt screen scrolls#14748warp-agent-staging[bot] wants to merge 2 commits into
Conversation
A highlighted link is anchored to alt-screen grid coordinates and the grid renderer paints its color and underline straight from `highlighted_link`, so it has to go when a scroll moves the content it described. `alt_scroll` only invalidated it on the arrow-key path, and invalidation is a hint for the next hover scan that rendering never reads, so an app that asked for mouse reports left a link underline stranded over whatever it scrolled into place. Co-Authored-By: Warp Agent <agent@warp.dev>
There was a problem hiding this comment.
Overview
This PR removes stale alt-screen link decoration, but clearing the hover before the arrow-key fallback has changed the grid regresses a visible interaction. The fix also remains unverified on the required rendered surface.
Concerns
No screenshot or recording exercises the required hover-then-wheel flow. Capture desktop/manual evidence that proves no underline remains on blank cells and that a still-visible link remains correctly highlighted after scrolling.
CI is red: Miscellaneous checks fails because PSScriptAnalyzer reports eight warnings in unchanged script/windows files. A repository/CI owner must confirm or remediate that baseline failure before this PR can be accepted.
The local format check passed. The focused test, clippy, terminal suite, and presubmit could not complete in this runner: Cargo first hit the read-only system cache, and the bounded writable-cache retry was OOM-killed.
Verdict
Checks: formatting ✅ · CI ❌ (PSScriptAnalyzer) · local Cargo checks unavailable (cache permissions/OOM) · visual proof ❌
Found: 0 critical, 1 important, 0 suggestions
Request changes
The implementation-addressable highlight regression is being relayed to implementation automatically.
Review run
https://oz.staging.warp.dev/runs/019fd418-6e0c-7d1e-9584-9b4971c4d111
Clearing the highlight before the wheel had produced anything to send also cleared it for scrolls the alt screen never sees: zero-line movement, or a mode that yields no bytes. Nothing moves in those cases, so the highlight still describes what is on screen and the link under the pointer went unhighlighted until the pointer moved again. Gate the drop on a scroll actually being written instead. An app that takes the wheel as a mouse report still loses the stale highlight, and so does the arrow-key fallback once it sends something. Co-Authored-By: Warp Agent <agent@warp.dev>
There was a problem hiding this comment.
Overview
The rework correctly preserves a hover when alt_scroll has no PTY output, and CI is now green. It does not establish that every emitted arrow sequence moves the alternate-screen grid, and the supplied visual artifacts remain explicitly inconclusive.
Concerns
The attached Linux/Xvfb videos and screenshots show neither the base nor fixed build reproducing the underline artifact. They therefore do not prove the acceptance criteria on the reporter’s rendered surface: that scrolling clears underline from non-link cells while retaining the correct link decoration. The pending macOS/manual before-and-after verification is required before this change can be accepted.
Verification
CI passed across Linux, macOS, Windows, and wasm, including formatting, Clippy, tests, and the Miscellaneous checks. ./script/format --check and git diff --check origin/master...HEAD also passed locally. The new zero-delta test would fail on the immediate parent: its alt_scroll unconditionally called highlighted_link.take() before constructing PTY bytes.
Verdict
Blocked pending human verification/decision.
Review run
https://oz.staging.warp.dev/runs/019fd418-6e0c-7d1e-9584-9b4971c4d111
| // drops the decoration the renderer paints from it, which flagging it as | ||
| // invalidated does not; the next hover re-detects whatever the pointer | ||
| // has ended up on. | ||
| if self.highlighted_link.take(&mut self.model.lock()).is_some() { |
There was a problem hiding this comment.
❓ [QUESTION] Some(bytes) proves only that Warp queued an arrow sequence; an app can ignore it or already be at its scroll boundary, leaving its alt-screen grid unchanged. This still clears a correctly anchored hover until pointer movement. The new regression test covers lines_to_scroll == 0, which emits no bytes, but not a nonzero arrow that the app ignores. With the automatic rework budget exhausted, please decide whether losing the hover on a no-op app scroll is acceptable; otherwise the fix needs to clear/recompute only after the grid actually changes.
Summary
Scrolling a full-screen terminal app (for example the Warp Agent CLI TUI) could leave a blue link underline stranded on screen, floating over blank cells where a URL used to be.
Root cause. Warp highlights the link under the pointer by storing a
GridHighlightedLinkonTerminalView::highlighted_link, anchored to alt-screen grid coordinates. The grid renderer paints that highlight's color and underline straight from it (TerminalViewRenderContext::highlighted_url=highlighted_link.clone_inner()→cell_type.is_url→URL_COLOR+ an underline decoration).alt_scrollonly dealt with the highlight on one of its two branches:Two problems:
invalidate()only sets a flag that the next hover scan consults. Rendering readsclone_inner()and ignores it, so even the arrow-key branch never removed the decoration; it just waited for the pointer to move.The result is a highlight still painted at coordinates whose content has moved. When the line that scrolls into that row is shorter than the one that left, the underline lands on blank cells — exactly the reported artifact: a single row, an underline starting a couple of columns after the text ends and running for the width of the departed URL.
Fix. Drop the highlight on any alt-screen scroll instead of conditionally flagging it.
takealso clears the smart-select override that is anchored to the same stale range, and resets the invalidated flag; the next hover re-detects whatever the pointer now sits on.Verification
Regression test (fails before this change, passes after):
app/src/terminal/view_tests.rs::alt_screen_scroll_drops_a_highlighted_link_when_the_app_handles_the_wheel— puts the terminal in the alt screen with SGR mouse enabled (assertingshould_intercept_scrollis false, i.e. the mouse-report branch), highlights a link, scrolls, and asserts the highlight is gone.Repository checks:
./script/format --check— cleancargo clippy -p warp --all-targets --tests -- -D warnings— cleancargo nextest run -p warp -E 'test(terminal::)'— 1573 run, 1571 passed. The two failures (terminal::input::tests::test_histignorespace_support_in_zsh,terminal::input::decorations::tests::test_decorations_with_multibyte_chars) reproduce identically on a clean stashed tree, so they are pre-existing and environmental, not caused by this change.Visual verification attempt — inconclusive, reported as such
I built two GUI binaries differing only in
alt_scrolland drove both on a Linux/Xvfb display: an alt-screen program that requests SGR mouse reporting and prints the alternating long-URL / short-line cargo log, hovering a URL and then scrolling the wheel without moving the pointer. Both builds launched, both showed the hovered URL turning blue and underlined.On both builds the underline cleared cleanly after the scroll — the artifact did not reproduce, so the recording does not demonstrate the fix. The key case was forced deliberately (an odd-line scroll offset so a short
Compiling unindent v0.2.4lands under the stationary pointer) and the unfixed build still cleared.A hypothesis for why, not a conclusion: a hover recompute appears to run after each scroll on this platform — scrolling a different URL under the stationary pointer re-underlines it — which would re-resolve the highlight and hide the symptom even on the unfixed build. The report came from macOS. Treat the media below as evidence of the attempt, not as before/after proof; a macOS run is what would actually settle it.
The deterministic evidence for the defect is unaffected: before the fix the highlight provably survived a scroll (test above), and the renderer paints from
highlighted_link.clone_inner(), which ignores theinvalidatedflag the old code set.Rework changes
Addressing the review on this PR (review 4869501304 — no inline threads were posted, so this section is the response):
app/src/terminal/view.rs:9393— clearing the highlight before the grid had changed. Fixed.alt_scrollnow computes the PTY bytes first and drops the highlight only when a scroll is actually written. A wheel movement that sends nothing — zero lines, or a mode that yields no bytes — returns early and leaves the highlight alone, so a link under the pointer stays highlighted. A boundary the running app enforces is not observable synchronously (we hand it arrow keys or a mouse report and it decides), so the gate is "a scroll was actually sent"; the highlight is re-detected on the next hover.ignored_alt_screen_scroll_keeps_the_highlighted_link(no SGR mouse, so the arrow-key fallback; zero-line scroll) — verified failing against the previous revision of this PR. Also addedarrow_key_alt_screen_scroll_drops_the_highlighted_linkto pin the arrow-key path that does send.alt_screen_scroll_drops_a_highlighted_link_when_the_app_handles_the_wheelstill passes.Miscellaneous checks. Red from eight PSScriptAnalyzer warnings in unchangedscript/windowsfiles; a pre-existing repository baseline, untouched here.Ruled out along the way
Three earlier hypotheses on the ticket were investigated and eliminated with a harness that replays
TuiFrameRenderer's emitted byte stream through a terminal simulator and diffs its underline state against the painted frame buffer, over ~500 randomized transcript contents x up to 14 scroll positions:TuiClipped/TuiViewportedList) — paints into a freshTuiBufferevery frame; underlined cells always match the link spans.TuiFrameRendererfailing to emit SGR underline-off — the crossterm backend resets attributes at the end of every draw and emits\e[24mon every underline-to-plain transition.ansi_handler.rs::write_at_cursor).CHANGELOG-BUG-FIX: Fixed a link underline that could stay on screen after scrolling a full-screen terminal app.
Originating thread: https://warpdev.slack.com/archives/C0BDQDW8V5E/p1785961533891659
Computer-use video recordings (3)
warp-unfixed: hover URL then wheel-scroll: Running scrolllog.py in warp-unfixed, resting the pointer on a long URL, then scrolling the wheel down without moving the pointer to observe any stray colored underline.
warp-fixed: hover URL then wheel-scroll: Running scrolllog.py in warp-fixed, resting the pointer on a long URL, then scrolling the wheel down without moving the pointer to observe any stray colored underline.
warp-unfixed: hover URL then real wheel-notch scroll: Running scrolllog on warp-unfixed, hovering a long URL, then a genuine wheel-notch scroll (odd offset) without moving the pointer so a short line lands under it, holding to observe any stray underline.
Computer-use screenshots (8)
warp-unfixed: pointer resting on the crate_025 long URL, which is highlighted in blue with an underline, before scrolling.
warp-unfixed: crate_125 long URL hovered (blue, underlined) between two short 'Compiling unindent' rows, before scrolling.
warp-unfixed: after a single 3-line wheel scroll without moving the pointer; the pointer now rests over a short 'Compiling unindent v0.2.4' line and no blue underline remains anywhere on screen.
warp-fixed: pointer resting on the crate_025 long URL, highlighted blue with an underline, before scrolling.
warp-fixed: after a single 3-line wheel scroll without moving the pointer; the pointer rests over a short 'Compiling unindent v0.2.4' line and no blue underline remains anywhere on screen.
warp-fixed: after a genuine press+release wheel notch (odd scroll offset) without moving the pointer; the pointer rests over a short 'Compiling unindent v0.2.4' line with no underline and no stray blue segment anywhere.
warp-unfixed: pointer resting on crate_187 long URL (blue, underlined) at scroll offset top=156, before a real wheel-notch scroll.
warp-unfixed: after a genuine press+release wheel notch (odd scroll offset) without moving the pointer; the pointer rests over a short 'Compiling unindent v0.2.4' line with no underline and no stray blue segment anywhere — identical to warp-fixed.
Conversation: https://staging.warp.dev/conversation/a83f9572-8bdf-4064-8e1e-aba1a5c3d406
Run: https://oz.staging.warp.dev/runs/019fd3b3-2b71-785c-86d9-4ecacc65b28f
This PR was generated with Oz.