Skip to content

Drop the link highlight when the alt screen scrolls - #14748

Open
warp-agent-staging[bot] wants to merge 2 commits into
masterfrom
factory/app-5187-alt-screen-link-highlight
Open

Drop the link highlight when the alt screen scrolls#14748
warp-agent-staging[bot] wants to merge 2 commits into
masterfrom
factory/app-5187-alt-screen-link-highlight

Conversation

@warp-agent-staging

@warp-agent-staging warp-agent-staging Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

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 GridHighlightedLink on TerminalView::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_urlURL_COLOR + an underline decoration).

alt_scroll only dealt with the highlight on one of its two branches:

let report_mouse = !should_intercept_scroll(&self.model.lock(), ctx);
if !report_mouse {
    // Arrow-key scrolling can change the alt-screen grid content, so
    // any link highlights are no longer valid.
    self.highlighted_link.invalidate();
}

Two problems:

  1. When the app has asked for SGR mouse reporting, the wheel is forwarded to it as a mouse report and the app scrolls its own viewport — changing the grid just as much — but that branch did nothing. The Warp TUI enables mouse capture on startup, so it always takes this branch.
  2. invalidate() only sets a flag that the next hover scan consults. Rendering reads clone_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. take also 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 (asserting should_intercept_scroll is false, i.e. the mouse-report branch), highlights a link, scrolls, and asserts the highlight is gone.

Repository checks:

  • ./script/format --check — clean
  • cargo clippy -p warp --all-targets --tests -- -D warnings — clean
  • cargo 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_scroll and 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.4 lands 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 the invalidated flag 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_scroll now 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.
  • Non-mouse-reporting regression test. Added 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 added arrow_key_alt_screen_scroll_drops_the_highlighted_link to pin the arrow-key path that does send. alt_screen_scroll_drops_a_highlighted_link_when_the_app_handles_the_wheel still passes.
  • Visual proof. Attempted properly this time rather than declared infeasible — see the section above and the attached media. It came back inconclusive.
  • CI Miscellaneous checks. Red from eight PSScriptAnalyzer warnings in unchanged script/windows files; 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:

  • The TUI's clipped/viewported paint (TuiClipped / TuiViewportedList) — paints into a fresh TuiBuffer every frame; underlined cells always match the link spans.
  • TuiFrameRenderer failing to emit SGR underline-off — the crossterm backend resets attributes at the end of every draw and emits \e[24m on every underline-to-plain transition.
  • A wide grapheme's SGR outliving its continuation column — real emulators reset both halves of a wide pair when either half is overwritten (verified in tmux, and in 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)

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-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.

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-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.

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.
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: 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: 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-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: 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 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-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: 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.
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.

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>
@cla-bot cla-bot Bot added the cla-signed label Aug 5, 2026
@warp-agent-staging warp-agent-staging Bot added the warpy-factory Label associated to the "Wob the Wuilder" factory on staging, also known as Warpy Factory. label Aug 5, 2026
@warp-agent-staging
warp-agent-staging Bot marked this pull request as ready for review August 5, 2026 22:40
@warp-agent-staging
warp-agent-staging Bot requested a review from moirahuang August 5, 2026 22:40

@warp-agent-staging warp-agent-staging Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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>

@warp-agent-staging warp-agent-staging Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Comment thread app/src/terminal/view.rs
// 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() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed warpy-factory Label associated to the "Wob the Wuilder" factory on staging, also known as Warpy Factory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant