From a81c82df4e6a91e87ddd94ffca20523af4615839 Mon Sep 17 00:00:00 2001
From: Drew Weymouth <dweymouth@gmail.com>
Date: Thu, 22 Feb 2024 08:05:48 -0800
Subject: [PATCH] properly validate offset in ScrollToOffset + tests

---
 widget/list.go      | 71 ++++++++++++++++++++++++++-------------------
 widget/list_test.go | 10 +++++--
 2 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/widget/list.go b/widget/list.go
index 4bae8b7229..a80e505077 100644
--- a/widget/list.go
+++ b/widget/list.go
@@ -261,10 +261,17 @@ func (l *List) ScrollToTop() {
 //
 // Since: 2.5
 func (l *List) ScrollToOffset(offset float32) {
-	if l.scroller != nil {
-		l.scroller.Offset.Y = offset
-		l.offsetUpdated(l.scroller.Offset)
+	if l.scroller == nil {
+		return
+	}
+	if offset < 0 {
+		offset = 0
+	} else if h := l.contentMinSize().Height; offset > h {
+		offset = h
 	}
+	l.scroller.Offset.Y = offset
+	l.offsetUpdated(l.scroller.Offset)
+	l.Refresh()
 }
 
 // GetScrollOffset returns the current scroll offset position
@@ -338,6 +345,36 @@ func (l *List) UnselectAll() {
 	}
 }
 
+func (l *List) contentMinSize() fyne.Size {
+	l.propertyLock.Lock()
+	defer l.propertyLock.Unlock()
+	items := 0
+	if f := l.Length; f == nil {
+		return fyne.NewSize(0, 0)
+	} else {
+		items = f()
+	}
+
+	separatorThickness := theme.Padding()
+	if l.itemHeights == nil || len(l.itemHeights) == 0 {
+		return fyne.NewSize(l.itemMin.Width,
+			(l.itemMin.Height+separatorThickness)*float32(items)-separatorThickness)
+	}
+
+	height := float32(0)
+	templateHeight := l.itemMin.Height
+	for item := 0; item < items; item++ {
+		itemHeight, ok := l.itemHeights[item]
+		if ok {
+			height += itemHeight
+		} else {
+			height += templateHeight
+		}
+	}
+
+	return fyne.NewSize(l.itemMin.Width, height+separatorThickness*float32(items-1))
+}
+
 // fills l.visibleRowHeights and also returns offY and minRow
 func (l *listLayout) calculateVisibleRowHeights(itemHeight float32, length int) (offY float32, minRow int) {
 	rowOffset := float32(0)
@@ -575,33 +612,7 @@ func (l *listLayout) Layout([]fyne.CanvasObject, fyne.Size) {
 }
 
 func (l *listLayout) MinSize([]fyne.CanvasObject) fyne.Size {
-	l.list.propertyLock.Lock()
-	defer l.list.propertyLock.Unlock()
-	items := 0
-	if f := l.list.Length; f == nil {
-		return fyne.NewSize(0, 0)
-	} else {
-		items = f()
-	}
-
-	separatorThickness := theme.Padding()
-	if l.list.itemHeights == nil || len(l.list.itemHeights) == 0 {
-		return fyne.NewSize(l.list.itemMin.Width,
-			(l.list.itemMin.Height+separatorThickness)*float32(items)-separatorThickness)
-	}
-
-	height := float32(0)
-	templateHeight := l.list.itemMin.Height
-	for item := 0; item < items; item++ {
-		itemHeight, ok := l.list.itemHeights[item]
-		if ok {
-			height += itemHeight
-		} else {
-			height += templateHeight
-		}
-	}
-
-	return fyne.NewSize(l.list.itemMin.Width, height+separatorThickness*float32(items-1))
+	return l.list.contentMinSize()
 }
 
 func (l *listLayout) getItem() *listItem {
diff --git a/widget/list_test.go b/widget/list_test.go
index 1f48c67a33..3c2f814b1c 100644
--- a/widget/list_test.go
+++ b/widget/list_test.go
@@ -241,13 +241,17 @@ func TestList_ScrollToTop(t *testing.T) {
 }
 
 func TestList_ScrollOffset(t *testing.T) {
-	list := createList(1000)
+	list := createList(10)
 
 	offset := float32(25)
 	list.ScrollToOffset(25)
-	assert.Equal(t, offset, list.offsetY)
-	assert.Equal(t, offset, list.scroller.Offset.Y)
 	assert.Equal(t, offset, list.GetScrollOffset())
+
+	list.ScrollToOffset(-2)
+	assert.Equal(t, float32(0), list.GetScrollOffset())
+
+	list.ScrollToOffset(1000)
+	assert.LessOrEqual(t, list.GetScrollOffset(), float32(500) /*upper bound on content height*/)
 }
 
 func TestList_Selection(t *testing.T) {