diff --git a/pkg/driver/devicelab/commands.go b/pkg/driver/devicelab/commands.go index b0bb789..5ecd4cd 100644 --- a/pkg/driver/devicelab/commands.go +++ b/pkg/driver/devicelab/commands.go @@ -845,10 +845,15 @@ func (d *Driver) scrollByAdb(direction string, screenWidth, screenHeight int, pe const scrollDurationMs = 300 // isElementOnScreen reports whether an element's bounds overlap the visible -// viewport. Zero-area bounds count as off-screen. +// viewport. Malformed bounds — non-positive width/height, e.g. a clipped +// below-the-fold ScrollView child reported with top>bottom (negative height) — +// count as off-screen, matching boundsTappable's #94 guard. Without this, +// scrollUntilVisible short-circuits to success on a degenerate rect that tapOn +// then refuses to tap ("element rect not tappable"), looping to the deadline +// instead of scrolling the element fully into view. func isElementOnScreen(info *core.ElementInfo, screenWidth, screenHeight int) bool { b := info.Bounds - if b.Width == 0 || b.Height == 0 { + if b.Width <= 0 || b.Height <= 0 { return false } return b.X+b.Width > 0 && b.X < screenWidth && b.Y+b.Height > 0 && b.Y < screenHeight diff --git a/pkg/driver/devicelab/commands_test.go b/pkg/driver/devicelab/commands_test.go index 8f69225..6da0ec4 100644 --- a/pkg/driver/devicelab/commands_test.go +++ b/pkg/driver/devicelab/commands_test.go @@ -180,6 +180,13 @@ func TestIsElementOnScreen(t *testing.T) { {"entirely right of screen", core.Bounds{X: 1080, Y: 100, Width: 200, Height: 200}, false}, {"zero width", core.Bounds{X: 100, Y: 100, Width: 0, Height: 200}, false}, {"zero height", core.Bounds{X: 100, Y: 100, Width: 200, Height: 0}, false}, + {"negative width", core.Bounds{X: 100, Y: 100, Width: -50, Height: 200}, false}, + // Repro from the field: a clipped below-the-fold ScrollView button the + // agent reports with top>bottom (raw [270,2300][1080,2274] → h=-26, + // centre (675,2287)). Overlaps the viewport numerically, but is a + // degenerate rect tapOn refuses (boundsTappable's #94 guard); the scroll + // success predicate must reject it too so the loop keeps scrolling. + {"negative height (clipped below-fold)", core.Bounds{X: 270, Y: 2300, Width: 810, Height: -26}, false}, } for _, tt := range tests { diff --git a/pkg/driver/uiautomator2/commands.go b/pkg/driver/uiautomator2/commands.go index 054248a..8700ebe 100644 --- a/pkg/driver/uiautomator2/commands.go +++ b/pkg/driver/uiautomator2/commands.go @@ -523,10 +523,12 @@ func (d *Driver) scrollByAdb(direction string, screenWidth, screenHeight int, pe } // isElementOnScreen reports whether an element's bounds overlap the visible -// viewport. Zero-area bounds count as off-screen. +// viewport. Malformed bounds (non-positive width/height — e.g. a clipped rect +// with top>bottom) count as off-screen, so scrollUntilVisible keeps scrolling +// instead of declaring success on a degenerate rect. func isElementOnScreen(info *core.ElementInfo, screenWidth, screenHeight int) bool { b := info.Bounds - if b.Width == 0 || b.Height == 0 { + if b.Width <= 0 || b.Height <= 0 { return false } return b.X+b.Width > 0 && b.X < screenWidth && b.Y+b.Height > 0 && b.Y < screenHeight diff --git a/pkg/driver/uiautomator2/commands_test.go b/pkg/driver/uiautomator2/commands_test.go index 744db0e..2270172 100644 --- a/pkg/driver/uiautomator2/commands_test.go +++ b/pkg/driver/uiautomator2/commands_test.go @@ -4180,6 +4180,10 @@ func TestIsElementOnScreen(t *testing.T) { {"entirely above screen", core.Bounds{X: 100, Y: -300, Width: 200, Height: 200}, false}, {"zero width", core.Bounds{X: 100, Y: 100, Width: 0, Height: 200}, false}, {"zero height", core.Bounds{X: 100, Y: 100, Width: 200, Height: 0}, false}, + {"negative width", core.Bounds{X: 100, Y: 100, Width: -50, Height: 200}, false}, + // Malformed clipped rect (top>bottom → negative height): degenerate, must + // count as off-screen so scrollUntilVisible keeps scrolling. + {"negative height (clipped)", core.Bounds{X: 270, Y: 2300, Width: 810, Height: -26}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {