Minor fixes for 45 patch (#5550)

* fix: stale theme and floating visibility issues, notification bounding

* fix: repaint excess tab surface when closing tab and moving to a smaller one

Author: imsnif

zellij-integration-tests/tests/clients.rs

@@ -522,6 +522,44 @@ fn focusing_a_smaller_tab_leaves_nothing_behind_outside_it() {
     zellij.quit();
 }
 
+#[test]
+fn closing_a_larger_tab_leaves_nothing_behind_outside_the_one_returned_to() {
+    let mut zellij = start_zellij();
+    let first_terminal = claim_first_terminal_and_wait_for_prompt(&zellij);
+    mark_pane(zellij.main_client(), &first_terminal, "oneone");
+
+    let second_client = zellij.attach_client(LARGER_CLIENT_SIZE);
+    second_client.wait_until(
+        "second client attached to the shared tab",
+        |grid_snapshot| settled_in_tab_sized(grid_snapshot, TERMINAL_SIZE, "oneone"),
+    );
+    let second_tab_terminal = open_marked_tab(&zellij, &second_client, "twotwo");
+    second_tab_terminal.wait_for_size("second tab laid out for its lone viewer", |cols, rows| {
+        (cols, rows) == pane_size_in_tab_sized(LARGER_CLIENT_SIZE)
+    });
+
+    close_focused_tab(&second_client);
+
+    let main_grid = zellij.wait_until("main client settled on the shared tab", |grid_snapshot| {
+        settled_in_normal_mode(grid_snapshot, TERMINAL_SIZE, "oneone")
+            && grid_snapshot.contains("Tab #1 [ ]")
+    });
+    let second_grid =
+        second_client.wait_until("larger client settled on the shared tab", |grid_snapshot| {
+            settled_in_normal_mode(grid_snapshot, TERMINAL_SIZE, "oneone")
+                && grid_snapshot.contains("Tab #1 [ ]")
+                && !grid_snapshot.contains("twotwo")
+        });
+
+    assert_eq!(
+        rendered_content(&second_grid),
+        rendered_content(&main_grid),
+        "a client returning from a tab larger than the one it lands in must not leave that tab's leftovers around it"
+    );
+    second_client.quit();
+    zellij.quit();
+}
+
 #[test]
 fn opening_a_new_tab_regrows_the_tab_its_creator_left() {
     let mut zellij = start_zellij();

zellij-server/src/panes/grid.rs

@@ -45,6 +45,7 @@ const BASE64_DECODER: GeneralPurpose = GeneralPurpose::new(
 );
 
 const MAX_TRACKED_NOTIFICATION_IDS: usize = 256;
+const MAX_NOTIFICATION_ASSEMBLY_BYTES: usize = 4096;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PendingNotification {
@@ -129,6 +130,22 @@ struct NotificationAssembly {
     body: String,
 }
 
+fn append_bounded(destination: &mut String, payload: &str) {
+    let remaining = MAX_NOTIFICATION_ASSEMBLY_BYTES.saturating_sub(destination.len());
+    if remaining == 0 {
+        return;
+    }
+    if payload.len() <= remaining {
+        destination.push_str(payload);
+        return;
+    }
+    let mut truncate_at = remaining;
+    while truncate_at > 0 && !payload.is_char_boundary(truncate_at) {
+        truncate_at -= 1;
+    }
+    destination.push_str(&payload[..truncate_at]);
+}
+
 #[derive(Debug, Clone, Default)]
 pub struct NotificationTracker {
     wants_report: HashMap<String, bool>,
@@ -180,8 +197,8 @@ impl NotificationTracker {
             self.remember(id);
             let assembly = self.assemblies.entry(id.to_owned()).or_default();
             match payload_type {
-                Osc99PayloadType::Title => assembly.title.push_str(&payload),
-                Osc99PayloadType::Body => assembly.body.push_str(&payload),
+                Osc99PayloadType::Title => append_bounded(&mut assembly.title, &payload),
+                Osc99PayloadType::Body => append_bounded(&mut assembly.body, &payload),
                 _ => {},
             }
         }

zellij-server/src/panes/unit/grid_tests.rs

@@ -9085,6 +9085,42 @@ fn the_notification_state_remembered_per_pane_is_bounded() {
     );
 }
 
+#[test]
+fn a_single_notification_being_assembled_is_bounded() {
+    let mut grid = new_grid_for_forwarding_test();
+    let mut vte_parser = vte::Parser::new();
+    let chunk = "\u{5efa}".repeat(300);
+    for _ in 0..100 {
+        vte_parser.advance(
+            &mut grid,
+            format!("\x1b]99;i=1:d=0;{}\x1b\\", chunk).as_bytes(),
+        );
+    }
+
+    let assembled_bytes = grid
+        .notification_tracker
+        .assemblies
+        .get("1")
+        .map(|assembly| assembly.title.len())
+        .unwrap_or(0);
+    assert!(
+        assembled_bytes <= 4096,
+        "an app streaming an endless single notification does not grow the pane's state without \
+         bound, got {} bytes",
+        assembled_bytes
+    );
+
+    vte_parser.advance(&mut grid, b"\x1b]99;i=1:d=1;\x1b\\");
+    let last_notification = grid.pending_desktop_notifications.len() - 1;
+    let (title, _body) =
+        osc99_display(&grid, last_notification).expect("the truncated notification is still shown");
+    assert_eq!(
+        title,
+        "\u{5efa}".repeat(1365),
+        "the assembled text is truncated on a character boundary"
+    );
+}
+
 fn rendered_row(grid: &Grid, row_index: usize) -> String {
     grid.viewport[row_index]
         .columns

zellij-server/src/screen.rs

@@ -1913,7 +1913,7 @@ impl Screen {
             .next()
             .context("screen contained no tabs")
             .with_context(err_context)?;
-        let mut destination_tab_ids = HashSet::new();
+        let mut arriving_client_ids: HashMap<usize, Vec<ClientId>> = HashMap::new();
         for (client_id, client_mode_info) in client_ids_and_mode_infos {
             let client_tab_history = self.tab_history.entry(client_id).or_insert_with(Vec::new);
             if let Some(client_previous_tab) = client_tab_history.pop() {
@@ -1922,7 +1922,10 @@ impl Screen {
                     client_active_tab
                         .add_client(client_id, Some(client_mode_info))
                         .with_context(err_context)?;
-                    destination_tab_ids.insert(client_previous_tab);
+                    arriving_client_ids
+                        .entry(client_previous_tab)
+                        .or_default()
+                        .push(client_id);
                     continue;
                 }
             }
@@ -1932,13 +1935,26 @@ impl Screen {
                 .with_context(err_context)?
                 .add_client(client_id, Some(client_mode_info))
                 .with_context(err_context)?;
-            destination_tab_ids.insert(first_tab_index);
+            arriving_client_ids
+                .entry(first_tab_index)
+                .or_default()
+                .push(client_id);
         }
-        for destination_tab_id in destination_tab_ids {
-            if let Some(destination_tab) = self.tabs.get_mut(&destination_tab_id) {
+        let destinations_overflowed_by_arriving_clients: HashSet<usize> = arriving_client_ids
+            .iter()
+            .filter(|(destination_tab_id, client_ids)| {
+                self.clients_are_larger_than_tab(**destination_tab_id, client_ids)
+            })
+            .map(|(destination_tab_id, _client_ids)| *destination_tab_id)
+            .collect();
+        for destination_tab_id in arriving_client_ids.keys() {
+            if let Some(destination_tab) = self.tabs.get_mut(destination_tab_id) {
                 destination_tab
                     .update_input_modes()
                     .with_context(err_context)?;
+                if destinations_overflowed_by_arriving_clients.contains(destination_tab_id) {
+                    destination_tab.set_should_clear_display_before_rendering();
+                }
             }
         }
         Ok(())
@@ -5070,8 +5086,12 @@ impl Screen {
             .tabs
             .get_mut(&tab_index)
             .with_context(|| err_context(tab_index))?;
+        let tab_was_empty = tab.has_no_connected_clients();
         tab.add_client(client_id, None)
             .with_context(|| err_context(tab_index))?;
+        if tab_was_empty {
+            tab.visible(true).with_context(|| err_context(tab_index))?;
+        }
         if attach_to_first_tab_on_tiled_surface && tab.are_floating_panes_visible() {
             tab.hide_floating_panes();
         }
@@ -7100,6 +7120,7 @@ impl Screen {
         explicit_theme_hue: Option<ThemeHue>,
     ) -> Result<()> {
         self.configured_explicit_theme_hue = explicit_theme_hue;
+        self.host_terminal_theme_mode = None;
         match explicit_theme_hue {
             Some(hue) => {
                 let mode = HostTerminalThemeMode::from(hue);

zellij-server/src/unit/screen_tests.rs

@@ -9770,6 +9770,19 @@ impl ThemeCapture {
         }
         out
     }
+    fn drain_visible_events(&self) -> Vec<(Option<u32>, bool)> {
+        let mut out = Vec::new();
+        while let Ok((instr, _ctx)) = self.plugin_rx.try_recv() {
+            if let PluginInstruction::Update(updates) = instr {
+                for (pid, _cid, ev) in updates {
+                    if let Event::Visible(is_visible) = ev {
+                        out.push((pid, is_visible));
+                    }
+                }
+            }
+        }
+        out
+    }
     fn drain_pty_writes(&self) -> Vec<(Vec<u8>, u32)> {
         let mut out = Vec::new();
         while let Ok((instr, _ctx)) = self.pty_writer_rx.try_recv() {
@@ -9781,6 +9794,41 @@ impl ThemeCapture {
     }
 }
 
+#[test]
+fn reattaching_a_client_restores_floating_pane_visibility_notifications() {
+    let size = Size {
+        cols: 121,
+        rows: 20,
+    };
+    let (mut screen, capture) = create_new_screen_with_theme_capture(size);
+    new_tab(&mut screen, 1, 0);
+    screen
+        .get_active_tab_mut(1)
+        .unwrap()
+        .new_pane(
+            PaneId::Plugin(2),
+            None,
+            None,
+            false,
+            true,
+            NewPanePlacement::Floating(None),
+            Some(1),
+            None,
+        )
+        .unwrap();
+
+    screen.remove_client(1).expect("TEST");
+    screen.add_client(1, false).expect("TEST");
+    let _ = capture.drain_visible_events();
+    screen.get_active_tab_mut(1).unwrap().hide_floating_panes();
+
+    assert!(
+        capture.drain_visible_events().contains(&(Some(2), false)),
+        "a floating plugin must still be told when its surface is hidden after a reattach, \
+         otherwise plugins idling on a timer keep working while off screen"
+    );
+}
+
 fn create_new_screen_with_theme_capture(size: Size) -> (Screen, ThemeCapture) {
     let (plugin_tx, plugin_rx) = channels::unbounded::<(PluginInstruction, ErrorContext)>();
     let (pty_writer_tx, pty_writer_rx) =
@@ -10269,6 +10317,50 @@ fn removing_explicit_theme_hue_hands_authority_back_to_the_host() {
     );
 }
 
+#[test]
+fn pinning_the_current_hue_repaints_the_resolved_palette() {
+    let size = Size { cols: 80, rows: 20 };
+    let (mut screen, _capture) = create_new_screen_with_dark_and_light_themes(size);
+    screen
+        .update_host_terminal_theme_mode(zellij_utils::data::HostTerminalThemeMode::Dark)
+        .expect("host report applied");
+    screen.style.colors = styling_with_background((1, 2, 3));
+
+    screen
+        .apply_configured_explicit_theme_hue(Some(zellij_utils::data::ThemeHue::Dark))
+        .expect("explicit hue applied");
+
+    assert_eq!(
+        screen.style.colors.text_unselected.background,
+        zellij_utils::data::PaletteColor::Rgb(TEST_DARK_BG),
+        "pinning the hue the session is already in must still resolve the palette, \
+         otherwise a reconfigure leaves the session painted with the static theme"
+    );
+}
+
+#[test]
+fn unpinning_back_to_the_current_hue_repaints_the_resolved_palette() {
+    let size = Size { cols: 80, rows: 20 };
+    let (mut screen, _capture) = create_new_screen_with_dark_and_light_themes(size);
+    screen
+        .update_host_terminal_theme_mode(zellij_utils::data::HostTerminalThemeMode::Dark)
+        .expect("host report applied");
+    screen
+        .apply_configured_explicit_theme_hue(Some(zellij_utils::data::ThemeHue::Dark))
+        .expect("explicit hue applied");
+    screen.style.colors = styling_with_background((1, 2, 3));
+
+    screen
+        .apply_configured_explicit_theme_hue(None)
+        .expect("unpinned");
+
+    assert_eq!(
+        screen.style.colors.text_unselected.background,
+        zellij_utils::data::PaletteColor::Rgb(TEST_DARK_BG),
+        "unpinning to the mode the session is already in must still resolve the palette"
+    );
+}
+
 #[test]
 fn effective_theme_mode_is_reasserted_after_a_theme_definition_change() {
     let size = Size { cols: 80, rows: 20 };