Skip to content

Commit e7d9173

Browse files
committed
Stop mouse wheel from leaking escape characters into form fields
Scrolling the mouse wheel while a title or description field was focused could insert stray characters such as "[<65;54;51M" into the field. The submit TUI ran the Bubble Tea program with WithMouseAllMotion (mode 1003), which reports an event on every pointer move. During a wheel scroll that floods the input stream, and under that volume Bubble Tea splits an SGR mouse escape sequence ("\x1b[<Cb;Cx;Cy(M|m)") across input reads; the leftover bytes of a partially-parsed sequence are then emitted as key runes and inserted into the focused text input. Two changes fix this: - Switch to WithMouseCellMotion (mode 1002), which reports clicks, drag, and wheel but not idle pointer motion. That removes the per-move input flood, so under a real terminal's reads (up to 256 bytes) the only fragment that still surfaces is a single, clean burst at each wheel notch boundary. The TUI never used idle-hover for rendering, so cell-motion loses nothing. - Drop any leaked fragments before they reach a field. A split SGR mouse sequence surfaces as an Alt+"[" (the consumed "\x1b[") followed by body fragments ("<65;54;5", "1M"), or occasionally the whole body in one run ("[<65;54;51M"). consumeLeakedMouseKey recognises the start, swallows the body up to its "M"/"m" terminator, and bails out the moment a rune does not fit an SGR body, so ordinary typing (including "<", ";", digits, "M") and bracketed pastes are never eaten. Tests cover every split point of an SGR sequence, single-run tails, preserved real typing, a stray Alt+"[", bracketed paste, and that wheel events never modify the focused field. Verified end-to-end by feeding 1,500 wheel sequences through the real parser under terminal-sized reads and confirming the field stays empty.
1 parent a24bf4f commit e7d9173

5 files changed

Lines changed: 257 additions & 1 deletion

File tree

cmd/submit.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,12 @@ func collectPRDrafts(cfg *config.Config, client github.ClientOps, s *stack.Stack
289289
Version: Version,
290290
})
291291

292-
p := tea.NewProgram(model, tea.WithAltScreen(), tea.WithMouseAllMotion())
292+
// Use cell-motion mouse mode (clicks, drag, and wheel) rather than all-motion.
293+
// All-motion (mode 1003) reports an event on every pointer move, flooding the
294+
// input; under that volume bubbletea can split an SGR mouse sequence across
295+
// reads, leaking its bytes as text into a focused title/description field
296+
// while scrolling. We don't use idle-hover, so cell-motion loses nothing.
297+
p := tea.NewProgram(model, tea.WithAltScreen(), tea.WithMouseCellMotion())
293298
final, err := p.Run()
294299
if err != nil {
295300
return nil, false, fmt.Errorf("running submit TUI: %w", err)

internal/tui/submitview/model.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,12 @@ type Model struct {
103103
// to open an existing PR. Tests inject a no-op so the suite never spawns a
104104
// real browser.
105105
openURL func(string)
106+
107+
// mouseLeak tracks an in-progress terminal mouse escape sequence that the
108+
// Bubble Tea input parser split across reads and surfaced as key runes. See
109+
// consumeLeakedMouseKey for details.
110+
mouseLeakActive bool
111+
mouseLeakBuf string
106112
}
107113

108114
// New constructs a submit TUI model from the given options. The single screen
@@ -185,6 +191,13 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
185191
return m, nil
186192

187193
case tea.KeyMsg:
194+
// Drop fragments of a terminal mouse escape sequence that the input
195+
// parser split across reads and leaked as key runes; otherwise they get
196+
// typed into the focused title/description field while scrolling.
197+
if m.consumeLeakedMouseKey(msg) {
198+
return m, nil
199+
}
200+
188201
// Any key dismisses a transient status hint.
189202
m.statusMessage = ""
190203
m.statusIsError = false
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package submitview
2+
3+
import (
4+
"regexp"
5+
6+
tea "github.com/charmbracelet/bubbletea"
7+
)
8+
9+
var (
10+
// sgrMouseTailRe matches a complete SGR mouse-sequence body: the bytes that
11+
// follow the "\x1b[" prefix, optionally including a leftover "[" that the
12+
// parser surfaced separately. Examples: "[<65;54;51M", "<0;12;7m".
13+
sgrMouseTailRe = regexp.MustCompile(`^\[?<\d+;\d+;\d+[Mm]$`)
14+
// sgrMouseBodyRe matches a partial SGR mouse body that is still accumulating
15+
// its parameters, with no terminator yet. Examples: "<65;54;5", "<65;", "<".
16+
sgrMouseBodyRe = regexp.MustCompile(`^<?[\d;]*$`)
17+
)
18+
19+
// mouseLeakBufCap bounds how many runes we will swallow for a single suspected
20+
// mouse sequence before giving up, so a stray Alt+"[" can never eat an unbounded
21+
// run of real keystrokes. Real SGR mouse bodies are far shorter than this.
22+
const mouseLeakBufCap = 32
23+
24+
// consumeLeakedMouseKey reports whether key is a fragment of a terminal mouse
25+
// escape sequence that Bubble Tea's input parser split across reads and emitted
26+
// as key runes instead of a tea.MouseMsg. Such fragments must be dropped so the
27+
// user does not see them typed into the focused title/description field while
28+
// scrolling the mouse wheel.
29+
//
30+
// A split SGR mouse sequence ("\x1b[<Cb;Cx;Cy(M|m)") surfaces in one of two
31+
// shapes:
32+
//
33+
// - the whole body in a single run, e.g. "[<65;54;51M"; or
34+
// - (the common case under a scroll flood) the consumed "\x1b[" as an Alt+"["
35+
// key, followed by body fragments such as "<65;54;5" then "1M".
36+
//
37+
// We recognise the start, then swallow the body up to its "M"/"m" terminator.
38+
// Anything that is not part of an SGR body — a control key, a bracketed paste,
39+
// or a rune that breaks the pattern — ends the sequence and is handled normally,
40+
// so real typing is never eaten.
41+
func (m *Model) consumeLeakedMouseKey(key tea.KeyMsg) bool {
42+
// Only printable, non-pasted runes can be part of a leaked mouse body. Any
43+
// other key (control key, navigation, bracketed paste) means the leak, if
44+
// any, is over: reset and let the key be handled normally.
45+
if key.Type != tea.KeyRunes || key.Paste {
46+
m.mouseLeakActive = false
47+
m.mouseLeakBuf = ""
48+
return false
49+
}
50+
s := string(key.Runes)
51+
52+
if !m.mouseLeakActive {
53+
// The consumed "\x1b[" prefix surfaces as Alt+"[": the start of a split
54+
// sequence whose body fragments follow.
55+
if key.Alt && s == "[" {
56+
m.mouseLeakActive = true
57+
m.mouseLeakBuf = ""
58+
return true
59+
}
60+
// The entire body can also arrive as a single run.
61+
if !key.Alt && sgrMouseTailRe.MatchString(s) {
62+
return true
63+
}
64+
return false
65+
}
66+
67+
// Mid-sequence: accumulate body fragments until the terminator.
68+
cand := m.mouseLeakBuf + s
69+
switch {
70+
case sgrMouseTailRe.MatchString(cand):
71+
// Reached the "M"/"m" terminator: the sequence is complete.
72+
m.mouseLeakActive = false
73+
m.mouseLeakBuf = ""
74+
return true
75+
case len(cand) <= mouseLeakBufCap && sgrMouseBodyRe.MatchString(cand):
76+
// Still inside the parameter list; keep swallowing.
77+
m.mouseLeakBuf = cand
78+
return true
79+
default:
80+
// Not a mouse body after all (e.g. real typing after a stray Alt+"["):
81+
// stop swallowing and let this key be handled normally.
82+
m.mouseLeakActive = false
83+
m.mouseLeakBuf = ""
84+
return false
85+
}
86+
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
package submitview
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
tea "github.com/charmbracelet/bubbletea"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// runeMsg builds the multi-rune key message Bubble Tea emits for a leaked
13+
// fragment of an escape sequence.
14+
func runeMsg(s string, alt bool) tea.KeyMsg {
15+
return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(s), Alt: alt}
16+
}
17+
18+
// splitSGRBurst reproduces the exact key messages Bubble Tea's input parser
19+
// emits when it splits the SGR mouse sequence seq ("\x1b[<Cb;Cx;Cy(M|m)") at
20+
// byte offset cut (2..len-1): an Alt+"[" for the consumed "\x1b[", an optional
21+
// middle run for the bytes before the cut, then the tail run after it.
22+
func splitSGRBurst(seq string, cut int) []tea.KeyMsg {
23+
burst := []tea.KeyMsg{runeMsg("[", true)}
24+
if cut > 2 {
25+
burst = append(burst, runeMsg(seq[2:cut], false))
26+
}
27+
burst = append(burst, runeMsg(seq[cut:], false))
28+
return burst
29+
}
30+
31+
func TestConsumeLeakedMouseKey_SwallowsSplitSequences(t *testing.T) {
32+
for _, seq := range []string{"\x1b[<65;54;51M", "\x1b[<64;10;20m", "\x1b[<0;1;1M"} {
33+
for cut := 2; cut < len(seq); cut++ {
34+
t.Run(fmt.Sprintf("%q@%d", seq, cut), func(t *testing.T) {
35+
m := testModel(t, newNodes())
36+
require.Equal(t, fieldTitle, m.focusedField)
37+
before := m.titleInput.Value()
38+
39+
for _, msg := range splitSGRBurst(seq, cut) {
40+
updated, _ := m.Update(msg)
41+
m = updated.(Model)
42+
}
43+
44+
assert.Equal(t, before, m.titleInput.Value(), "no fragment should reach the title field")
45+
assert.False(t, m.mouseLeakActive, "the sequence resets once its terminator is consumed")
46+
47+
// A normal keystroke afterwards must still register.
48+
m = sendKey(t, m, runeKey('x'))
49+
assert.Equal(t, before+"x", m.titleInput.Value(), "typing works after a swallowed sequence")
50+
})
51+
}
52+
}
53+
}
54+
55+
func TestConsumeLeakedMouseKey_SwallowsSingleRunTail(t *testing.T) {
56+
// When the parser consumes "\x1b" as a lone Escape, the rest of the body can
57+
// arrive as one run.
58+
m := testModel(t, newNodes())
59+
before := m.titleInput.Value()
60+
m = sendKey(t, m, runeMsg("[<65;54;51M", false))
61+
assert.Equal(t, before, m.titleInput.Value(), "a whole leaked body in one run is dropped")
62+
}
63+
64+
func TestConsumeLeakedMouseKey_PreservesRealTyping(t *testing.T) {
65+
m := testModel(t, newNodes())
66+
before := m.titleInput.Value()
67+
68+
// Characters from the mouse alphabet typed normally must pass through. Each
69+
// real keystroke is a single-rune message, never a complete SGR body.
70+
for _, r := range []rune{'<', '3', ';', '5', 'M', 'm'} {
71+
m = sendKey(t, m, runeKey(r))
72+
}
73+
assert.Equal(t, before+"<3;5Mm", m.titleInput.Value(), "ordinary characters are never filtered")
74+
}
75+
76+
func TestConsumeLeakedMouseKey_StrayAltBracketDoesNotEatNextKey(t *testing.T) {
77+
m := testModel(t, newNodes())
78+
before := m.titleInput.Value()
79+
80+
// A lone Alt+"[" opens a sequence, but the following key is not a mouse body,
81+
// so it must still be handled (here: typed into the field).
82+
m = sendKey(t, m, runeMsg("[", true))
83+
require.True(t, m.mouseLeakActive)
84+
m = sendKey(t, m, runeKey('z'))
85+
assert.False(t, m.mouseLeakActive, "a non-body rune ends the sequence")
86+
assert.Equal(t, before+"z", m.titleInput.Value(), "the following real key is not eaten")
87+
}
88+
89+
func TestConsumeLeakedMouseKey_BracketedPasteIsNotFiltered(t *testing.T) {
90+
m := testModel(t, newNodes())
91+
before := m.titleInput.Value()
92+
93+
// A genuine paste that happens to look like a mouse body is preserved.
94+
paste := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("<65;54;51M"), Paste: true}
95+
updated, _ := m.Update(paste)
96+
m = updated.(Model)
97+
assert.Equal(t, before+"<65;54;51M", m.titleInput.Value(), "pasted content is never filtered")
98+
}
99+
100+
func TestConsumeLeakedMouseKey_DescriptionField(t *testing.T) {
101+
m := testModel(t, newNodes())
102+
m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyTab}) // focus description
103+
require.Equal(t, fieldDescription, m.focusedField)
104+
before := m.descArea.Value()
105+
106+
for _, msg := range splitSGRBurst("\x1b[<65;54;51M", 10) {
107+
updated, _ := m.Update(msg)
108+
m = updated.(Model)
109+
}
110+
assert.Equal(t, before, m.descArea.Value(), "no fragment should reach the description field")
111+
}

internal/tui/submitview/screen_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,47 @@ func TestMouse_WheelScrollsDescriptionViewport(t *testing.T) {
605605
assert.False(t, m.descScrollPinned, "typing returns to the cursor-following view")
606606
}
607607

608+
// TestMouse_WheelDoesNotEnterFieldText guards against the regression where
609+
// scrolling the mouse wheel leaked escape-sequence bytes as text into the
610+
// focused title/description field. handleMouse must consume every wheel event
611+
// (returning without forwarding it to the text inputs), so the field contents
612+
// never change regardless of which panel the pointer is over.
613+
func TestMouse_WheelDoesNotEnterFieldText(t *testing.T) {
614+
m := testModel(t, newNodes())
615+
require.Equal(t, fieldTitle, m.focusedField)
616+
617+
leftW, _ := m.panelWidths()
618+
leftX := leftW / 2 // over the left timeline
619+
rightX := leftW + 5 // over the right editor
620+
titleLine, _, _, _, _ := m.rightZones()
621+
y := m.panelTopRow() + titleLine
622+
623+
wheel := func(m Model) Model {
624+
t.Helper()
625+
for _, x := range []int{leftX, rightX} {
626+
for _, b := range []tea.MouseButton{tea.MouseButtonWheelUp, tea.MouseButtonWheelDown} {
627+
u, _ := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: b, X: x, Y: y})
628+
m = u.(Model)
629+
}
630+
}
631+
return m
632+
}
633+
634+
// Title focused: wheeling over either panel must not alter the title.
635+
titleBefore := m.nodes[m.cursor].Title
636+
m = wheel(m)
637+
assert.Equal(t, titleBefore, m.nodes[m.cursor].Title, "wheel must not modify the focused title")
638+
assert.Equal(t, titleBefore, m.titleInput.Value(), "wheel must not modify the title input")
639+
640+
// Description focused: same guarantee.
641+
m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyTab})
642+
require.Equal(t, fieldDescription, m.focusedField)
643+
descBefore := m.nodes[m.cursor].Description
644+
m = wheel(m)
645+
assert.Equal(t, descBefore, m.nodes[m.cursor].Description, "wheel must not modify the focused description")
646+
assert.Equal(t, descBefore, m.descArea.Value(), "wheel must not modify the description input")
647+
}
648+
608649
func TestMouse_WheelScrollBoundedByContent(t *testing.T) {
609650
// The over-scroll bug only appears with a real color profile (the textarea
610651
// pads blank rows with styled spaces), so force one here.

0 commit comments

Comments
 (0)