Repository navigation
fix(ui): end the text selection before a click stops propagation - #595
Merged
Merged
Conversation
gpui-base begins a selection gesture on any left MouseDown and ends it from a bubble-phase MouseUp listener on the root. A click handler that only called cx.stop_propagation() swallowed that MouseUp, so the selection kept following the pointer across Markdown until the next release, and the next press could not clear it. One owner, widgets::stop_click_propagation, lets the selection see the release first. Every on_click that stops propagation goes through it, including the Markdown link handlers that already did this by hand. Regression test: a swallowing click above selectable Markdown, then a pointer move over the text, must leave no selection.
Tryanks
enabled auto-merge
October 6, 2026 07:07
a_self_hosted_machine_starts_without_its_manifest_and_applies_it_when_it_arrives reset the pacing stamp while the host's own refresh loop was about to run its first pass. On a slow runner that pass landed after the reset, fetched the manifest itself, and left the test's refresh with nothing to return. The test now aborts the loop first; the contract it proves, applying a manifest that arrives after startup, drives apply_manifest directly and never needed the loop.
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
A click handler that swallowed its MouseUp left gpui-base's window selection in its drag state: the Markdown body followed the pointer until the next release, and the release after that was needed to clear it. Reported on the async question strip's ✕ and earlier on the notification close button.
One owner,
widgets::stop_click_propagation(window, cx), replaces every barecx.stop_propagation()inside anon_click(14 sites) and the four Markdown link handlers that already did the pair by hand.on_mouse_downsites are untouched: they block the gesture from starting, and the release still resets the state.Evidence
cx.stop_propagation():Merge Danger
Door: two-way
Pure
tcode-uichange behind one helper; reverting the commit restores the old behaviour.Blast Radius: clicks
Every click that stops propagation now also ends an in-progress selection gesture.
TextSelection::endkeeps the selection visible, so a click on a button while text is selected still leaves the selection as before.Test met on the way
tcode-traverse host::tests::a_self_hosted_machine_starts_without_its_manifest_and_applies_it_when_it_arrivesfailed on Linux in this PR's first CI run and passes on main's last six. Step 1 (driver): the test reset the manifest loader's pacing stamp while the host's ownspawn_refreshloop was about to run its first pass; when the runtime scheduled that pass after the reset, it fetched the manifest itself and the test's directrefresh()returnedNone. The test now aborts the host's loop before taking over the refresh. The contract (a manifest arriving after startup is applied) was already driven throughapply_manifestdirectly and never depended on the loop. 30 consecutive local runs pass.