From 6a932f305783919d8f4c2f9eba59d216e355338c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 27 Aug 2026 08:19:18 +0200 Subject: [PATCH 1/4] Read mouse pointer geometry on the UI thread in tests Drag autoscrolling deliberately keeps rendering after the test action has returned, so view bounds can change while the test goroutine prepares its next mouse event. Snapshot the geometry on the event loop before translating view-relative coordinates to avoid data races. This hasn't been a problem so far, but only because we were lucky; the added test assertions later in this branch would cause consistent race detector failures without this fix. --- pkg/integration/components/test_test.go | 43 +++++++++++++++++++++++ pkg/integration/components/view_driver.go | 26 ++++++++++---- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/pkg/integration/components/test_test.go b/pkg/integration/components/test_test.go index bda63e3fb..2495a07c5 100644 --- a/pkg/integration/components/test_test.go +++ b/pkg/integration/components/test_test.go @@ -28,6 +28,9 @@ type fakeGuiDriver struct { heldCoordinates []coordinate movedCoordinates []coordinate releasedCoordinates []coordinate + scrolledCoordinates []coordinate + onUIThread bool + onUIThreadCallCount int } var _ integrationTypes.GuiDriver = &fakeGuiDriver{} @@ -57,13 +60,17 @@ func (self *fakeGuiDriver) MouseRelease(x, y int) { } func (self *fakeGuiDriver) ScrollWheelDown(x, y int) { + self.scrolledCoordinates = append(self.scrolledCoordinates, coordinate{x: x, y: y}) } func (self *fakeGuiDriver) RefreshInBackground() { } func (self *fakeGuiDriver) OnUIThreadAndWait(f func()) { + self.onUIThreadCallCount++ + self.onUIThread = true f() + self.onUIThread = false } func (self *fakeGuiDriver) FocusIn() { @@ -167,6 +174,42 @@ func TestSuccess(t *testing.T) { assert.Equal(t, "", driver.failureMessage) } +func TestViewDriverPointerCoordinates(t *testing.T) { + guiDriver := &fakeGuiDriver{} + testDriver := NewTestDriver(guiDriver, nil, config.KeybindingConfig{}, 0) + view := gocui.NewView("source", 10, 20, 30, 31, gocui.OutputNormal) + targetView := gocui.NewView("target", 40, 50, 60, 61, gocui.OutputNormal) + viewDriver := &ViewDriver{ + getView: func() *gocui.View { + assert.True(t, guiDriver.onUIThread) + return view + }, + t: testDriver, + } + targetViewDriver := &ViewDriver{ + getView: func() *gocui.View { + assert.True(t, guiDriver.onUIThread) + return targetView + }, + t: testDriver, + } + + viewDriver. + Click(1, 2). + FocusInAndClick(3, 4). + ClickAndHold(5, 6). + MouseMove(7, 8). + MouseMoveToBottom(9). + MouseMoveToView(targetViewDriver, 10, 11). + ScrollWheelDown() + + assert.Equal(t, []coordinate{{12, 23}, {14, 25}}, guiDriver.clickedCoordinates) + assert.Equal(t, []coordinate{{16, 27}}, guiDriver.heldCoordinates) + assert.Equal(t, []coordinate{{18, 29}, {20, 30}, {51, 62}}, guiDriver.movedCoordinates) + assert.Equal(t, []coordinate{{11, 21}}, guiDriver.scrolledCoordinates) + assert.Equal(t, 7, guiDriver.onUIThreadCallCount) +} + func TestFailingFixture(t *testing.T) { test := NewIntegrationTest(NewIntegrationTestArgs{ Description: unitTestDescription, diff --git a/pkg/integration/components/view_driver.go b/pkg/integration/components/view_driver.go index 23e3502a1..b743afde7 100644 --- a/pkg/integration/components/view_driver.go +++ b/pkg/integration/components/view_driver.go @@ -517,7 +517,7 @@ func (self *ViewDriver) PressRapidly(keys ...config.Keybinding) *ViewDriver { } func (self *ViewDriver) Click(x, y int) *ViewDriver { - offsetX, offsetY, _, _ := self.getView().Dimensions() + offsetX, offsetY, _ := self.viewGeometry() self.t.click(offsetX+1+x, offsetY+1+y) @@ -525,7 +525,7 @@ func (self *ViewDriver) Click(x, y int) *ViewDriver { } func (self *ViewDriver) FocusInAndClick(x, y int) *ViewDriver { - offsetX, offsetY, _, _ := self.getView().Dimensions() + offsetX, offsetY, _ := self.viewGeometry() self.t.focusInAndClick(offsetX+1+x, offsetY+1+y) @@ -533,7 +533,7 @@ func (self *ViewDriver) FocusInAndClick(x, y int) *ViewDriver { } func (self *ViewDriver) MouseMoveToView(target *ViewDriver, x, y int) *ViewDriver { - offsetX, offsetY, _, _ := target.getView().Dimensions() + offsetX, offsetY, _ := target.viewGeometry() self.t.mouseMove(offsetX+1+x, offsetY+1+y) return self } @@ -543,30 +543,42 @@ func (self *ViewDriver) Drag(fromX, fromY, toX, toY int) *ViewDriver { } func (self *ViewDriver) ClickAndHold(x, y int) *ViewDriver { - offsetX, offsetY, _, _ := self.getView().Dimensions() + offsetX, offsetY, _ := self.viewGeometry() self.t.clickAndHold(offsetX+1+x, offsetY+1+y) return self } func (self *ViewDriver) MouseMove(x, y int) *ViewDriver { - offsetX, offsetY, _, _ := self.getView().Dimensions() + offsetX, offsetY, _ := self.viewGeometry() self.t.mouseMove(offsetX+1+x, offsetY+1+y) return self } func (self *ViewDriver) MouseMoveToBottom(x int) *ViewDriver { - return self.MouseMove(x, self.getView().InnerHeight()-1) + offsetX, offsetY, innerHeight := self.viewGeometry() + self.t.mouseMove(offsetX+1+x, offsetY+innerHeight) + return self } // scrolls the view down by one notch of the mouse wheel, i.e. by // gui.scrollHeight lines. This moves the scroll position without moving the // selection. func (self *ViewDriver) ScrollWheelDown() *ViewDriver { - offsetX, offsetY, _, _ := self.getView().Dimensions() + offsetX, offsetY, _ := self.viewGeometry() self.t.scrollWheelDown(offsetX+1, offsetY+1) return self } +func (self *ViewDriver) viewGeometry() (offsetX int, offsetY int, innerHeight int) { + self.t.gui.OnUIThreadAndWait(func() { + view := self.getView() + offsetX, offsetY, _, _ = view.Dimensions() + innerHeight = view.InnerHeight() + }) + + return offsetX, offsetY, innerHeight +} + func (self *ViewDriver) RepeatMouseMove() *ViewDriver { self.t.repeatMouseMove() return self From c09b682f635d2fd8effd7aae8873649746ffbd44 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 26 Aug 2026 19:01:36 +0200 Subject: [PATCH 2/4] Add some comments to the drag_to_reorder_with_autoscroll test --- .../interactive_rebase/drag_to_reorder_with_autoscroll.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go b/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go index d8086e0a3..5f8101b05 100644 --- a/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go +++ b/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go @@ -19,14 +19,18 @@ var DragToReorderWithAutoscroll = NewIntegrationTest(NewIntegrationTestArgs{ TopLines( Contains("commit-40").IsSelected(), ). + // Click and hold the first commit ClickAndHold(1, 0). + // Move the mouse to the bottom of the panel to trigger autoscroll MouseMoveToBottom(1). + // Verify that the view scrolls OriginYAtLeast(3). MouseRelease(). SelectedLines( Contains("commit-40"), ). SelectedLineIdxAtLeast(3). + // Scroll back to verify that the original commit is no longer at the top GotoTop(). TopLines( Contains("commit-39").IsSelected(), From c8d610cb58228bfa190d52e66f79b4de5c67bd4c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 26 Aug 2026 18:52:23 +0200 Subject: [PATCH 3/4] Demonstrate commit drag scroll reset on re-entry When dragging a commit with auto-scrolling so that the original commit leaves the viewport, dragging back into the view makes the original commit snap back into view. This is a regression that was introduced by aebf495dce7f. --- .../interactive_rebase/drag_to_reorder_with_autoscroll.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go b/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go index 5f8101b05..6d281aa06 100644 --- a/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go +++ b/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go @@ -25,6 +25,14 @@ var DragToReorderWithAutoscroll = NewIntegrationTest(NewIntegrationTestArgs{ MouseMoveToBottom(1). // Verify that the view scrolls OriginYAtLeast(3). + // Move the mouse back into the viewport + MouseMove(1, 1). + /* EXPECTED: + // This keeps the scroll as it was + OriginYAtLeast(3). + ACTUAL: */ + // This snaps back to reveal the original commit + OriginY(0). MouseRelease(). SelectedLines( Contains("commit-40"), From 35753fd042dda6974cb6d7c2a029d43d3b4e50a1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 26 Aug 2026 18:53:55 +0200 Subject: [PATCH 4/4] Fix scrolling back when dragging a commit During auto-scrolling, turn off the automatic scroll-to-make-the-selected-item-visible functionality of PostRefreshUpdate. --- pkg/gui/controllers/local_commits_controller.go | 2 +- .../interactive_rebase/drag_to_reorder_with_autoscroll.go | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 0a02e7398..9407e3dee 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -191,7 +191,7 @@ func (self *LocalCommitsController) handleCommitDrag(opts gocui.ViewMouseBinding self.commitDrag.hasMoved = true if self.updateCommitDragInsertion(opts.Y) { - self.c.PostRefreshUpdate(self.context()) + self.c.PostRefreshUpdateKeepingScrollPosition(self.context()) } originY := self.context().GetView().OriginY() self.dragAutoscroller.Update(opts.Y - originY) diff --git a/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go b/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go index 6d281aa06..fdfdb8bc8 100644 --- a/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go +++ b/pkg/integration/tests/interactive_rebase/drag_to_reorder_with_autoscroll.go @@ -27,12 +27,8 @@ var DragToReorderWithAutoscroll = NewIntegrationTest(NewIntegrationTestArgs{ OriginYAtLeast(3). // Move the mouse back into the viewport MouseMove(1, 1). - /* EXPECTED: // This keeps the scroll as it was OriginYAtLeast(3). - ACTUAL: */ - // This snaps back to reveal the original commit - OriginY(0). MouseRelease(). SelectedLines( Contains("commit-40"),