diff --git a/crates/terminator/src/element.rs b/crates/terminator/src/element.rs index 12e5cee9..df9cad32 100644 --- a/crates/terminator/src/element.rs +++ b/crates/terminator/src/element.rs @@ -1637,6 +1637,12 @@ impl UIElement { // Iteratively adjust let mut steps_taken: usize = 0; + let mut last_direction: Option<&'static str> = None; + let mut oscillation_count: usize = 0; + let mut stale_bounds_count: usize = 0; + let mut last_bounds: Option<(f64, f64, f64, f64)> = None; + const MAX_OSCILLATIONS: usize = 3; // Stop if direction flips more than 3 times + const MAX_STALE_BOUNDS: usize = 4; // Stop if bounds don't change for 4 steps loop { // Refresh visibility and geometry each iteration let visible = self.is_visible().unwrap_or(false); @@ -1648,6 +1654,29 @@ impl UIElement { } }; + // Detect if bounds aren't changing (scrolling has no effect on this element) + if let Some(prev_bounds) = last_bounds { + if prev_bounds == elem_bounds { + stale_bounds_count += 1; + if stale_bounds_count >= MAX_STALE_BOUNDS { + warn!( + "scroll_into_view:STOPPING_STALE_BOUNDS bounds unchanged for {} steps - scrolling has no effect on this element", + stale_bounds_count + ); + return Ok(()); // Return Ok to not block the action + } + } else { + stale_bounds_count = 0; // Reset if bounds changed + } + } + last_bounds = Some(elem_bounds); + + // Log detailed state each iteration for debugging scroll oscillation + debug!( + "scroll_into_view:step={} visible={} elem_bounds={:?} window_bounds={:?} last_dir={:?} stale_count={}", + steps_taken, visible, elem_bounds, window_bounds, last_direction, stale_bounds_count + ); + // Early exit if bounds became invalid during scrolling (e.g., element got hidden/repositioned) let (x, y, width, height) = elem_bounds; if width <= MIN_ELEMENT_SIZE && height <= MIN_ELEMENT_SIZE { @@ -1718,6 +1747,25 @@ impl UIElement { // Perform one vertical step if needed if let Some(dir) = vertical_dir { + // Detect oscillation: direction flipped from last step + if let Some(last_dir) = last_direction { + if last_dir != dir { + oscillation_count += 1; + warn!( + "scroll_into_view:OSCILLATION_DETECTED count={} step={} dir_changed {} -> {} elem_bounds={:?} window_bounds={:?}", + oscillation_count, steps_taken, last_dir, dir, elem_bounds, window_bounds + ); + + // Stop early if oscillating too much - element likely can't be scrolled into view + if oscillation_count >= MAX_OSCILLATIONS { + warn!( + "scroll_into_view:STOPPING_DUE_TO_OSCILLATION count={} - element cannot be scrolled into view reliably", + oscillation_count + ); + return Ok(()); // Return Ok to not block the action, just skip scrolling + } + } + } debug!( "scroll_into_view:vertical_step dir={} step={} amount={}", dir, @@ -1726,6 +1774,7 @@ impl UIElement { ); // Ignore individual step errors and continue to try alternate axes let _ = self.scroll(dir, STEP_AMOUNT); + last_direction = Some(dir); steps_taken += 1; } diff --git a/crates/terminator/src/platforms/windows/element.rs b/crates/terminator/src/platforms/windows/element.rs index dece00c5..5fc428a0 100644 --- a/crates/terminator/src/platforms/windows/element.rs +++ b/crates/terminator/src/platforms/windows/element.rs @@ -1390,15 +1390,25 @@ impl UIElementImpl for WindowsUIElement { .map_err(|e| AutomationError::ElementNotFound(e.to_string()))?; if is_offscreen { - tracing::debug!("Element is offscreen"); + tracing::debug!( + "is_visible:false reason=offscreen element={:?}", + self.element.0.get_name().unwrap_or_default() + ); return Ok(false); } // Check bounds - element must have non-zero size to be visible - if let Ok((_x, _y, width, height)) = self.bounds() { + if let Ok((x, y, width, height)) = self.bounds() { // Check for non-zero bounds (critical for preventing false positives) if width <= 0.0 || height <= 0.0 { - tracing::debug!("Element has zero-size bounds: {}x{}", width, height); + tracing::debug!( + "is_visible:false reason=zero_bounds bounds=({}, {}, {}, {}) element={:?}", + x, + y, + width, + height, + self.element.0.get_name().unwrap_or_default() + ); return Ok(false); } @@ -1406,10 +1416,22 @@ impl UIElementImpl for WindowsUIElement { // which broke multi-monitor support. The is_offscreen() check above // and bounds check are sufficient for visibility detection. + tracing::debug!( + "is_visible:true bounds=({}, {}, {}, {}) element={:?}", + x, + y, + width, + height, + self.element.0.get_name().unwrap_or_default() + ); return Ok(true); } // If we can't get bounds, consider not visible + tracing::debug!( + "is_visible:false reason=no_bounds element={:?}", + self.element.0.get_name().unwrap_or_default() + ); Ok(false) }