fix(daemon): clear stale mouse press state and drag offset

commit: fe1b01a24c3d34862dbef0359cbd21cccaad7136
author: brice · 3mo ago · 2026-07-03 14:39 UTC
parent: 23ebd52
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +122 -4

M internal/usecase/daemon/copymode.go +6 -0


@@ -95,6 +95,12 @@ func (d Daemon) copyMouse(sess session, ac *attachedClient, ev mouse.Event) {
95	95
96	96		tb.mu.Lock()
97	97		if ev.Row >= tb.screen.Frame.Height {
	98	+	if ev.Type == mouse.Press {
	99	+	ac.copyMu.Lock()
	100	+	ac.copyPressRowValid = false
	101	+	ac.copyDragging = false
	102	+	ac.copyMu.Unlock()
	103	+	}
98	104		tb.mu.Unlock()
99	105		return
100	106		}


@@ -95,6 +95,12 @@ func (d Daemon) copyMouse(sess session, ac *attachedClient, ev mouse.Event) {
95		95
96	tb.mu.Lock()	96		tb.mu.Lock()
97	if ev.Row >= tb.screen.Frame.Height {	97		if ev.Row >= tb.screen.Frame.Height {
		98	+	if ev.Type == mouse.Press {
		99	+	ac.copyMu.Lock()
		100	+	ac.copyPressRowValid = false
		101	+	ac.copyDragging = false
		102	+	ac.copyMu.Unlock()
		103	+	}
98	tb.mu.Unlock()	104		tb.mu.Unlock()
99	return	105		return
100	}	106		}

M internal/usecase/daemon/input.go +10 -4


@@ -101,17 +101,23 @@ func (d Daemon) handleMouse(ac attachedClient, ev mouse.Event) {
101	101
102	102		switch ev.Button {
103	103		case mouse.Left:
104		-	if altScreen \|\| ev.Row >= childRows {
105		-	return
106		-	}
107	104		switch ev.Type {
108	105		case mouse.Press:
	106	+	if altScreen \|\| ev.Row >= childRows {
	107	+	ac.copyMu.Lock()
	108	+	ac.normalMousePressValid = false
	109	+	ac.copyMu.Unlock()
	110	+	return
	111	+	}
109	112		ac.copyMu.Lock()
110	113		ac.normalMousePressRow = ev.Row
111	114		ac.normalMousePressTop = scrollbackRows
112	115		ac.normalMousePressValid = true
113	116		ac.copyMu.Unlock()
114	117		case mouse.Motion:
	118	+	if altScreen \|\| ev.Row >= childRows {
	119	+	return
	120	+	}
115	121		ac.copyMu.Lock()
116	122		pressValid := ac.normalMousePressValid
117	123		pressRow := ac.normalMousePressRow

@@ -128,7 +134,7 @@ func (d Daemon) handleMouse(ac attachedClient, ev mouse.Event) {
128	134		ac.copyMu.Lock()
129	135		mode := scopy.NewMode(snap)
130	136		mode.StartSelectionAt(snap, pressTop+pressRow)
131		-	mode.ExtendTo(snap, pressTop+ev.Row)
	137	+	mode.ExtendTo(snap, len(snap.Rows)-snap.Height+ev.Row)
132	138		ac.copyMode = mode
133	139		ac.copyPressRow = pressTop + pressRow
134	140		ac.copyPressRowValid = true

 	switch ev.Button {
 	case mouse.Left:
-		if altScreen || ev.Row >= childRows {
-			return
-		}
 		switch ev.Type {
 		case mouse.Press:
+			if altScreen || ev.Row >= childRows {
+				ac.copyMu.Lock()
+				ac.normalMousePressValid = false
+				ac.copyMu.Unlock()
+				return
+			}
 			ac.copyMu.Lock()
 			ac.normalMousePressRow = ev.Row
 			ac.normalMousePressTop = scrollbackRows
 			ac.normalMousePressValid = true
 			ac.copyMu.Unlock()
 		case mouse.Motion:
+			if altScreen || ev.Row >= childRows {
+				return
+			}
 			ac.copyMu.Lock()
 			pressValid := ac.normalMousePressValid
 			pressRow := ac.normalMousePressRow
 			ac.copyMu.Lock()
 			mode := scopy.NewMode(snap)
 			mode.StartSelectionAt(snap, pressTop+pressRow)
-			mode.ExtendTo(snap, pressTop+ev.Row)
+			mode.ExtendTo(snap, len(snap.Rows)-snap.Height+ev.Row)
 			ac.copyMode = mode
 			ac.copyPressRow = pressTop + pressRow
 			ac.copyPressRowValid = true

M internal/usecase/daemon/input_test.go +106 -0


@@ -11,6 +11,7 @@ import (
11	11
12	12		"github.com/bnema/vev/internal/domain"
13	13		"github.com/bnema/vev/internal/ports"
	14	+	scopy "github.com/bnema/vev/internal/usecase/copy"
14	15		)
15	16
16	17		// --- test doubles -----------------------------------------------------------

@@ -282,6 +283,111 @@ func TestCopyModeMouseDragYanksOSC52AndExits(t *testing.T) {
282	283		require.NotContains(t, exitPaint, "[SELECT]")
283	284		}
284	285
	286	+	// TestMouseNormalScreenStatusRowClearsStalePressState covers a regression
	287	+	// where a Press or Release landing on the status row (row >= childRows)
	288	+	// returned before the inner event-type switch ran, leaving
	289	+	// normalMousePressValid untouched. A later Motion on a content row would
	290	+	// then resurrect the stale anchor and start a selection out of thin air.
	291	+	// The fix requires Release to always clear the press state and Press on the
	292	+	// status row to clear it too, so a following Motion is a no-op.
	293	+	func TestMouseNormalScreenStatusRowClearsStalePressState(t *testing.T) {
	294	+	p, _ := newBlockingPTY(t)
	295	+	d, sess, ac, _ := newManualSessionWithPTYs(t, p)
	296	+	require.Equal(t, 23, sess.tabs[0].screen.Frame.Height, "fixture assumption: status row is wire row 24")
	297	+
	298	+	// Press on a content row establishes a (soon to be stale) anchor.
	299	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;1M"))
	300	+	require.Nil(t, ac.copyMode)
	301	+
	302	+	// Release lands on the status row: must still clear the press state,
	303	+	// regardless of the row it landed on.
	304	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;24m"))
	305	+
	306	+	// A brand new button hold starts with a Press on the status row: must
	307	+	// clear (not silently preserve) any prior press state.
	308	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;24M"))
	309	+
	310	+	// Motion onto a content row must not resurrect a stale anchor.
	311	+	d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
	312	+
	313	+	require.Nil(t, ac.copyMode, "stale press state must not resurrect a selection")
	314	+	}
	315	+
	316	+	// TestCopyModeStatusRowPressClearsDragState covers the copy-mode counterpart
	317	+	// of the same bug class: copyMouse ignored any event whose row landed on the
	318	+	// status row before updating copyPressRow/copyPressRowValid/copyDragging, so
	319	+	// a Press on the status row left the previous drag state in place and a
	320	+	// following Motion on a content row silently extended the old selection
	321	+	// instead of being a no-op.
	322	+	func TestCopyModeStatusRowPressClearsDragState(t *testing.T) {
	323	+	p, _ := newBlockingPTY(t)
	324	+	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
	325	+	copy(sess.tabs[0].screen.Frame.Row(0), testRow("alpha"))
	326	+	copy(sess.tabs[0].screen.Frame.Row(1), testRow("bravo"))
	327	+	copy(sess.tabs[0].screen.Frame.Row(2), testRow("charlie"))
	328	+	require.Equal(t, 23, sess.tabs[0].screen.Frame.Height, "fixture assumption: status row is wire row 24")
	329	+
	330	+	d.enterCopyMode(sess, ac)
	331	+	mustOutputData(t, sends)
	332	+
	333	+	// Full drag: press row0, motion to row1 -> selection [0,1].
	334	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;1M\x1b[<32;1;2M"))
	335	+	mustOutputData(t, sends)
	336	+	require.NotNil(t, ac.copyMode)
	337	+	lo, hi, ok := ac.copyMode.SelectedBounds()
	338	+	require.True(t, ok)
	339	+	require.Equal(t, 0, lo)
	340	+	require.Equal(t, 1, hi)
	341	+
	342	+	// Release, then a new button hold starting with Press on the status row.
	343	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;2m"))
	344	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;24M"))
	345	+
	346	+	// Motion onto a content row must be a no-op: the status-row press must
	347	+	// have cleared copyPressRowValid/copyDragging.
	348	+	d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
	349	+
	350	+	require.NotNil(t, ac.copyMode)
	351	+	lo, hi, ok = ac.copyMode.SelectedBounds()
	352	+	require.True(t, ok)
	353	+	require.Equal(t, 0, lo, "anchor must not have moved")
	354	+	require.Equal(t, 1, hi, "status-row press must invalidate the drag so the motion is a no-op")
	355	+	}
	356	+
	357	+	// TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset covers a
	358	+	// regression where the first drag Motion extended the selection using the
	359	+	// scrollback length captured at Press time instead of the current
	360	+	// scrollback length, so if lines were evicted into scrollback between the
	361	+	// Press and the first Motion, the extend target landed on the wrong row.
	362	+	func TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset(t *testing.T) {
	363	+	p, _ := newBlockingPTY(t)
	364	+	d, sess, ac, _ := newManualSessionWithPTYs(t, p)
	365	+	sess.tabs[0].scrollback = scopy.NewScrollback(50)
	366	+	require.Equal(t, 23, sess.tabs[0].screen.Frame.Height, "fixture assumption: status row is wire row 24")
	367	+
	368	+	// Press on a content row while scrollback is empty: the anchor is
	369	+	// content-stable at pressTop(0)+pressRow(0) = row 0.
	370	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;1M"))
	371	+
	372	+	// Simulate 5 lines evicted into scrollback between the Press and the
	373	+	// first Motion (e.g. the child kept producing output).
	374	+	for i := 0; i < 5; i++ {
	375	+	sess.tabs[0].scrollback.Append(testRow("evicted"))
	376	+	}
	377	+
	378	+	// Motion lands on wire row 3 (0-based row 2 of the current screen).
	379	+	// The extend target must track the pointer's current content row:
	380	+	// current scrollback length (5) + ev.Row (2) = 7 -- not the stale
	381	+	// pressTop(0)+ev.Row(2) = 2.
	382	+	d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
	383	+
	384	+	require.NotNil(t, ac.copyMode)
	385	+	lo, hi, ok := ac.copyMode.SelectedBounds()
	386	+	require.True(t, ok)
	387	+	require.Equal(t, 0, lo, "anchor stays content-stable at the row under the pointer at press time")
	388	+	require.Equal(t, 7, hi, "extend target must track the pointer's current content row, not a stale scrollback offset")
	389	+	}
	390	+
285	391		func TestMouseSplitReportPreservesOrder(t *testing.T) {
286	392		writes := make(chan []byte, 2)
287	393		p, _ := newBlockingPTYWithWrites(t, writes)


@@ -11,6 +11,7 @@ import (
11		11
12	"github.com/bnema/vev/internal/domain"	12		"github.com/bnema/vev/internal/domain"
13	"github.com/bnema/vev/internal/ports"	13		"github.com/bnema/vev/internal/ports"
		14	+	scopy "github.com/bnema/vev/internal/usecase/copy"
14	)	15		)
15		16
16	// --- test doubles -----------------------------------------------------------	17		// --- test doubles -----------------------------------------------------------

@@ -282,6 +283,111 @@ func TestCopyModeMouseDragYanksOSC52AndExits(t *testing.T) {
282	require.NotContains(t, exitPaint, "[SELECT]")	283		require.NotContains(t, exitPaint, "[SELECT]")
283	}	284		}
284		285
		286	+	// TestMouseNormalScreenStatusRowClearsStalePressState covers a regression
		287	+	// where a Press or Release landing on the status row (row >= childRows)
		288	+	// returned before the inner event-type switch ran, leaving
		289	+	// normalMousePressValid untouched. A later Motion on a content row would
		290	+	// then resurrect the stale anchor and start a selection out of thin air.
		291	+	// The fix requires Release to always clear the press state and Press on the
		292	+	// status row to clear it too, so a following Motion is a no-op.
		293	+	func TestMouseNormalScreenStatusRowClearsStalePressState(t *testing.T) {
		294	+	p, _ := newBlockingPTY(t)
		295	+	d, sess, ac, _ := newManualSessionWithPTYs(t, p)
		296	+	require.Equal(t, 23, sess.tabs[0].screen.Frame.Height, "fixture assumption: status row is wire row 24")
		297	+
		298	+	// Press on a content row establishes a (soon to be stale) anchor.
		299	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;1M"))
		300	+	require.Nil(t, ac.copyMode)
		301	+
		302	+	// Release lands on the status row: must still clear the press state,
		303	+	// regardless of the row it landed on.
		304	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;24m"))
		305	+
		306	+	// A brand new button hold starts with a Press on the status row: must
		307	+	// clear (not silently preserve) any prior press state.
		308	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;24M"))
		309	+
		310	+	// Motion onto a content row must not resurrect a stale anchor.
		311	+	d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
		312	+
		313	+	require.Nil(t, ac.copyMode, "stale press state must not resurrect a selection")
		314	+	}
		315	+
		316	+	// TestCopyModeStatusRowPressClearsDragState covers the copy-mode counterpart
		317	+	// of the same bug class: copyMouse ignored any event whose row landed on the
		318	+	// status row before updating copyPressRow/copyPressRowValid/copyDragging, so
		319	+	// a Press on the status row left the previous drag state in place and a
		320	+	// following Motion on a content row silently extended the old selection
		321	+	// instead of being a no-op.
		322	+	func TestCopyModeStatusRowPressClearsDragState(t *testing.T) {
		323	+	p, _ := newBlockingPTY(t)
		324	+	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
		325	+	copy(sess.tabs[0].screen.Frame.Row(0), testRow("alpha"))
		326	+	copy(sess.tabs[0].screen.Frame.Row(1), testRow("bravo"))
		327	+	copy(sess.tabs[0].screen.Frame.Row(2), testRow("charlie"))
		328	+	require.Equal(t, 23, sess.tabs[0].screen.Frame.Height, "fixture assumption: status row is wire row 24")
		329	+
		330	+	d.enterCopyMode(sess, ac)
		331	+	mustOutputData(t, sends)
		332	+
		333	+	// Full drag: press row0, motion to row1 -> selection [0,1].
		334	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;1M\x1b[<32;1;2M"))
		335	+	mustOutputData(t, sends)
		336	+	require.NotNil(t, ac.copyMode)
		337	+	lo, hi, ok := ac.copyMode.SelectedBounds()
		338	+	require.True(t, ok)
		339	+	require.Equal(t, 0, lo)
		340	+	require.Equal(t, 1, hi)
		341	+
		342	+	// Release, then a new button hold starting with Press on the status row.
		343	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;2m"))
		344	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;24M"))
		345	+
		346	+	// Motion onto a content row must be a no-op: the status-row press must
		347	+	// have cleared copyPressRowValid/copyDragging.
		348	+	d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
		349	+
		350	+	require.NotNil(t, ac.copyMode)
		351	+	lo, hi, ok = ac.copyMode.SelectedBounds()
		352	+	require.True(t, ok)
		353	+	require.Equal(t, 0, lo, "anchor must not have moved")
		354	+	require.Equal(t, 1, hi, "status-row press must invalidate the drag so the motion is a no-op")
		355	+	}
		356	+
		357	+	// TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset covers a
		358	+	// regression where the first drag Motion extended the selection using the
		359	+	// scrollback length captured at Press time instead of the current
		360	+	// scrollback length, so if lines were evicted into scrollback between the
		361	+	// Press and the first Motion, the extend target landed on the wrong row.
		362	+	func TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset(t *testing.T) {
		363	+	p, _ := newBlockingPTY(t)
		364	+	d, sess, ac, _ := newManualSessionWithPTYs(t, p)
		365	+	sess.tabs[0].scrollback = scopy.NewScrollback(50)
		366	+	require.Equal(t, 23, sess.tabs[0].screen.Frame.Height, "fixture assumption: status row is wire row 24")
		367	+
		368	+	// Press on a content row while scrollback is empty: the anchor is
		369	+	// content-stable at pressTop(0)+pressRow(0) = row 0.
		370	+	d.handleInput(sess, ac, []byte("\x1b[<0;1;1M"))
		371	+
		372	+	// Simulate 5 lines evicted into scrollback between the Press and the
		373	+	// first Motion (e.g. the child kept producing output).
		374	+	for i := 0; i < 5; i++ {
		375	+	sess.tabs[0].scrollback.Append(testRow("evicted"))
		376	+	}
		377	+
		378	+	// Motion lands on wire row 3 (0-based row 2 of the current screen).
		379	+	// The extend target must track the pointer's current content row:
		380	+	// current scrollback length (5) + ev.Row (2) = 7 -- not the stale
		381	+	// pressTop(0)+ev.Row(2) = 2.
		382	+	d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
		383	+
		384	+	require.NotNil(t, ac.copyMode)
		385	+	lo, hi, ok := ac.copyMode.SelectedBounds()
		386	+	require.True(t, ok)
		387	+	require.Equal(t, 0, lo, "anchor stays content-stable at the row under the pointer at press time")
		388	+	require.Equal(t, 7, hi, "extend target must track the pointer's current content row, not a stale scrollback offset")
		389	+	}
		390	+
285	func TestMouseSplitReportPreservesOrder(t *testing.T) {	391		func TestMouseSplitReportPreservesOrder(t *testing.T) {
286	writes := make(chan []byte, 2)	392		writes := make(chan []byte, 2)
287	p, _ := newBlockingPTYWithWrites(t, writes)	393		p, _ := newBlockingPTYWithWrites(t, writes)