editor: Fix incorrect hover popup row clamping (#41645)

Lukas Wirth created 2 days ago

Fixes ZED-2TR
Fixes ZED-2TQ
Fixes ZED-2TB
Fixes ZED-2SW
Fixes ZED-2SQ

Release Notes:

- Fixed panic in repainting hover popups

Co-authored by: David <[e-mail]>

Change summary

crates/editor/src/element.rs       | 12 +++++++-----
crates/editor/src/hover_popover.rs | 18 +++++++++++-------
2 files changed, 18 insertions(+), 12 deletions(-)

Detailed changes

crates/editor/src/element.rs

@@ -5114,19 +5114,21 @@ impl EditorElement {
                 cx,
             )
         });
-        let Some((position, hover_popovers)) = hover_popovers else {
+        let Some((popover_position, hover_popovers)) = hover_popovers else {
             return;
         };
 
         // This is safe because we check on layout whether the required row is available
-        let hovered_row_layout =
-            &line_layouts[position.row().minus(visible_display_row_range.start) as usize];
+        let hovered_row_layout = &line_layouts[popover_position
+            .row()
+            .minus(visible_display_row_range.start)
+            as usize];
 
         // Compute Hovered Point
-        let x = hovered_row_layout.x_for_index(position.column() as usize)
+        let x = hovered_row_layout.x_for_index(popover_position.column() as usize)
             - Pixels::from(scroll_pixel_position.x);
         let y = Pixels::from(
-            position.row().as_f64() * ScrollPixelOffset::from(line_height)
+            popover_position.row().as_f64() * ScrollPixelOffset::from(line_height)
                 - scroll_pixel_position.y,
         );
         let hovered_point = content_origin + point(x, y);

crates/editor/src/hover_popover.rs

@@ -797,23 +797,22 @@ impl HoverState {
                 })
             })?;
         let mut point = anchor.to_display_point(&snapshot.display_snapshot);
-
         // Clamp the point within the visible rows in case the popup source spans multiple lines
-        if point.row() < visible_rows.start {
-            point = crate::movement::down_by_rows(
+        if visible_rows.end <= point.row() {
+            point = crate::movement::up_by_rows(
                 &snapshot.display_snapshot,
                 point,
-                (visible_rows.start - point.row()).0,
+                1 + (point.row() - visible_rows.end).0,
                 text::SelectionGoal::None,
                 true,
                 text_layout_details,
             )
             .0;
-        } else if visible_rows.end <= point.row() {
-            point = crate::movement::up_by_rows(
+        } else if point.row() < visible_rows.start {
+            point = crate::movement::down_by_rows(
                 &snapshot.display_snapshot,
                 point,
-                (visible_rows.end - point.row()).0,
+                (visible_rows.start - point.row()).0,
                 text::SelectionGoal::None,
                 true,
                 text_layout_details,
@@ -821,6 +820,11 @@ impl HoverState {
             .0;
         }
 
+        if !visible_rows.contains(&point.row()) {
+            log::error!("Hover popover point out of bounds after moving");
+            return None;
+        }
+
         let mut elements = Vec::new();
 
         if let Some(diagnostic_popover) = self.diagnostic_popover.as_ref() {