From 27a825ebdcae75d00a90396acf861fe1c90a2a6d Mon Sep 17 00:00:00 2001 From: Satoru Higa Date: Fri, 3 Apr 2026 17:21:34 +0900 Subject: [PATCH] fix: position host cursor for CJK IME even when app hides cursor (#4951) * fix: position host cursor for CJK IME even when app hides cursor When apps like Claude Code (ink/React library) hide the cursor during rendering, Zellij's render_cursor() was not sending a CUP sequence to the host terminal. This caused IME composition candidates to appear at the wrong position, breaking CJK input in these apps. Change cursor_coordinates() to return the cursor position unconditionally (as long as it is within bounds) along with a bool indicating visibility. render_cursor() now positions the host cursor even when the app has hidden it, so the IME subsystem can read the correct cursor position. * add test * rustfmt --------- Co-authored-by: Aram Drevekenin --- src/tests/e2e/remote_runner.rs | 23 ++-- zellij-server/src/panes/grid.rs | 10 +- zellij-server/src/panes/plugin_pane.rs | 4 +- zellij-server/src/panes/terminal_pane.rs | 12 +- .../tiled_panes/unit/stacked_panes_tests.rs | 2 +- zellij-server/src/panes/unit/grid_tests.rs | 17 ++- zellij-server/src/tab/mod.rs | 108 ++++++++++-------- .../src/tab/unit/tab_integration_tests.rs | 49 +++++++- zellij-server/src/ui/pane_contents_and_ui.rs | 16 ++- zellij-server/src/unit/screen_tests.rs | 5 +- 10 files changed, 168 insertions(+), 78 deletions(-) diff --git a/src/tests/e2e/remote_runner.rs b/src/tests/e2e/remote_runner.rs index 20f4e0c3d..958b8f1e9 100644 --- a/src/tests/e2e/remote_runner.rs +++ b/src/tests/e2e/remote_runner.rs @@ -337,8 +337,10 @@ fn read_from_channel( Ok(0) => { let current_snapshot = take_snapshot(&mut terminal_output); let mut last_snapshot = last_snapshot.lock().unwrap(); - *cursor_coordinates.lock().unwrap() = - terminal_output.cursor_coordinates().unwrap_or((0, 0)); + *cursor_coordinates.lock().unwrap() = terminal_output + .cursor_coordinates() + .map(|(x, y, _)| (x, y)) + .unwrap_or((0, 0)); *last_snapshot = current_snapshot; should_sleep = true; }, @@ -348,8 +350,11 @@ fn read_from_channel( } let current_snapshot = take_snapshot(&mut terminal_output); let mut last_snapshot = last_snapshot.lock().unwrap(); - *cursor_coordinates.lock().unwrap() = - terminal_output.grid.cursor_coordinates().unwrap_or((0, 0)); + *cursor_coordinates.lock().unwrap() = terminal_output + .grid + .cursor_coordinates() + .map(|(x, y, _)| (x, y)) + .unwrap_or((0, 0)); *last_snapshot = current_snapshot; should_sleep = true; }, @@ -357,8 +362,10 @@ fn read_from_channel( if e.kind() == std::io::ErrorKind::WouldBlock { let current_snapshot = take_snapshot(&mut terminal_output); let mut last_snapshot = last_snapshot.lock().unwrap(); - *cursor_coordinates.lock().unwrap() = - terminal_output.cursor_coordinates().unwrap_or((0, 0)); + *cursor_coordinates.lock().unwrap() = terminal_output + .cursor_coordinates() + .map(|(x, y, _)| (x, y)) + .unwrap_or((0, 0)); *last_snapshot = current_snapshot; should_sleep = true; } else if retries_left > 0 { @@ -377,7 +384,9 @@ fn read_from_channel( pub fn take_snapshot(terminal_output: &mut TerminalPane) -> String { let output_lines = terminal_output.read_buffer_as_lines(); - let cursor_coordinates = terminal_output.cursor_coordinates(); + let cursor_coordinates = terminal_output + .cursor_coordinates() + .and_then(|(x, y, visible)| if visible { Some((x, y)) } else { None }); let mut snapshot = String::new(); for (line_index, line) in output_lines.iter().enumerate() { for (character_index, terminal_character) in line.iter().enumerate() { diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 79a9a7b4d..ddb462cd8 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -1574,11 +1574,15 @@ impl Grid { sixel_image_chunks, ))); } - pub fn cursor_coordinates(&self) -> Option<(usize, usize)> { - if self.cursor_is_hidden || self.cursor.x >= self.width || self.cursor.y >= self.height { + /// Returns the cursor position and whether it is visible. + /// The position is returned unconditionally (as long as the cursor is within + /// bounds) so that the host terminal can position the cursor for IME even + /// when the app has hidden it. The bool is true when the cursor is visible. + pub fn cursor_coordinates(&self) -> Option<(usize, usize, bool)> { + if self.cursor.x >= self.width || self.cursor.y >= self.height { None } else { - Some((self.cursor.x, self.cursor.y)) + Some((self.cursor.x, self.cursor.y, !self.cursor_is_hidden)) } } pub fn is_mid_frame(&self) -> bool { diff --git a/zellij-server/src/panes/plugin_pane.rs b/zellij-server/src/panes/plugin_pane.rs index aeed38c7d..10e7186ca 100644 --- a/zellij-server/src/panes/plugin_pane.rs +++ b/zellij-server/src/panes/plugin_pane.rs @@ -252,7 +252,7 @@ impl Pane for PluginPane { self.should_render.insert(client_id, true); } - fn cursor_coordinates(&self, client_id: Option) -> Option<(usize, usize)> { + fn cursor_coordinates(&self, client_id: Option) -> Option<(usize, usize, bool)> { let own_content_columns = self.get_content_columns(); let own_content_rows = self.get_content_rows(); let Offset { top, left, .. } = self.content_offset; @@ -265,7 +265,7 @@ impl Pane for PluginPane { if x >= own_content_columns || y >= own_content_rows { None } else { - Some((x, y)) // these are 0 indexed + Some((x, y, true)) // plugins always show cursor when position is set } }) } else { diff --git a/zellij-server/src/panes/terminal_pane.rs b/zellij-server/src/panes/terminal_pane.rs index 8a114e603..bed9881d0 100644 --- a/zellij-server/src/panes/terminal_pane.rs +++ b/zellij-server/src/panes/terminal_pane.rs @@ -207,8 +207,8 @@ impl Pane for TerminalPane { self.vte_parser.advance(&mut self.grid, byte); } } - fn cursor_coordinates(&self, _client_id: Option) -> Option<(usize, usize)> { - // (x, y) + fn cursor_coordinates(&self, _client_id: Option) -> Option<(usize, usize, bool)> { + // (x, y, is_visible) if self.get_content_rows() < 1 || self.get_content_columns() < 1 { // do not render cursor if there's no room for it return None; @@ -216,7 +216,7 @@ impl Pane for TerminalPane { let Offset { top, left, .. } = self.content_offset; self.grid .cursor_coordinates() - .map(|(x, y)| (x + left, y + top)) + .map(|(x, y, is_visible)| (x + left, y + top, is_visible)) } fn is_mid_frame(&self) -> bool { self.grid.is_mid_frame() @@ -458,7 +458,7 @@ impl Pane for TerminalPane { text_color: PaletteColor, ) -> Option { let mut vte_output = None; - if let Some((cursor_x, cursor_y)) = self.cursor_coordinates() { + if let Some((cursor_x, cursor_y, true)) = self.cursor_coordinates() { let mut character_under_cursor = self .grid .get_character_under_cursor() @@ -1117,8 +1117,8 @@ impl TerminalPane { pub fn read_buffer_as_lines(&self) -> Vec> { self.grid.as_character_lines() } - pub fn cursor_coordinates(&self) -> Option<(usize, usize)> { - // (x, y) + pub fn cursor_coordinates(&self) -> Option<(usize, usize, bool)> { + // (x, y, is_visible) if self.get_content_rows() < 1 || self.get_content_columns() < 1 { // do not render cursor if there's no room for it return None; diff --git a/zellij-server/src/panes/tiled_panes/unit/stacked_panes_tests.rs b/zellij-server/src/panes/tiled_panes/unit/stacked_panes_tests.rs index 651b61ddd..045cbafe2 100644 --- a/zellij-server/src/panes/tiled_panes/unit/stacked_panes_tests.rs +++ b/zellij-server/src/panes/tiled_panes/unit/stacked_panes_tests.rs @@ -1085,7 +1085,7 @@ impl Pane for MockPane { fn handle_plugin_bytes(&mut self, _client_id: ClientId, _bytes: VteBytes) { unimplemented!() } - fn cursor_coordinates(&self, _client_id: Option) -> Option<(usize, usize)> { + fn cursor_coordinates(&self, _client_id: Option) -> Option<(usize, usize, bool)> { unimplemented!() } diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index e603c5eac..ed3bc3ac7 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -3797,15 +3797,17 @@ pub fn cursor_hide_persists_through_alternate_screen() { for byte in hide_cursor.as_bytes() { vte_parser.advance(&mut grid, *byte); } - assert_eq!(grid.cursor_coordinates(), None, "Cursor hidden properly"); + assert!( + matches!(grid.cursor_coordinates(), Some((_, _, false))), + "Cursor hidden properly" + ); let move_to_alternate_screen = "\u{1b}[?1049h"; for byte in move_to_alternate_screen.as_bytes() { vte_parser.advance(&mut grid, *byte); } - assert_eq!( - grid.cursor_coordinates(), - None, + assert!( + matches!(grid.cursor_coordinates(), Some((_, _, false))), "Cursor still hidden in alternate screen" ); @@ -3813,14 +3815,17 @@ pub fn cursor_hide_persists_through_alternate_screen() { for byte in show_cursor.as_bytes() { vte_parser.advance(&mut grid, *byte); } - assert!(grid.cursor_coordinates().is_some(), "Cursor shown"); + assert!( + matches!(grid.cursor_coordinates(), Some((_, _, true))), + "Cursor shown" + ); let move_away_from_alternate_screen = "\u{1b}[?1049l"; for byte in move_away_from_alternate_screen.as_bytes() { vte_parser.advance(&mut grid, *byte); } assert!( - grid.cursor_coordinates().is_some(), + matches!(grid.cursor_coordinates(), Some((_, _, true))), "Cursor still shown away from alternate screen" ); } diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs index 7b348fbdb..c78395bf8 100644 --- a/zellij-server/src/tab/mod.rs +++ b/zellij-server/src/tab/mod.rs @@ -240,7 +240,11 @@ pub trait Pane { fn handle_pty_bytes(&mut self, _bytes: VteBytes) {} fn handle_plugin_bytes(&mut self, _client_id: ClientId, _bytes: VteBytes) {} fn show_cursor(&mut self, _client_id: ClientId, _cursor_position: Option<(usize, usize)>) {} - fn cursor_coordinates(&self, _client_id: Option) -> Option<(usize, usize)>; + /// Returns the cursor position and whether it is visible. + /// The position is returned unconditionally (as long as the cursor is within + /// bounds) so that the host terminal can position the cursor for IME even + /// when the app has hidden it. The bool is true when the cursor is visible. + fn cursor_coordinates(&self, _client_id: Option) -> Option<(usize, usize, bool)>; fn is_mid_frame(&self) -> bool { false } @@ -3037,8 +3041,8 @@ impl Tab { pub fn get_active_terminal_cursor_position( &self, client_id: ClientId, - ) -> Option<(usize, usize)> { - // (x, y) + ) -> Option<(usize, usize, bool)> { + // (x, y, is_cursor_visible) let active_pane_id = if self.floating_panes.panes_are_visible() { self.floating_panes .get_active_pane_id(client_id) @@ -3050,13 +3054,13 @@ impl Tab { .floating_panes .get(&active_pane_id) .or_else(|| self.tiled_panes.get_pane(active_pane_id))?; - active_terminal - .cursor_coordinates(Some(client_id)) - .map(|(x_in_terminal, y_in_terminal)| { + active_terminal.cursor_coordinates(Some(client_id)).map( + |(x_in_terminal, y_in_terminal, is_visible)| { let x = active_terminal.x() + x_in_terminal; let y = active_terminal.y() + y_in_terminal; - (x, y) - }) + (x, y, is_visible) + }, + ) } pub fn toggle_active_pane_fullscreen(&mut self, client_id: ClientId) { if self.floating_panes.panes_are_visible() { @@ -3236,36 +3240,16 @@ impl Tab { let connected_clients: Vec = { self.connected_clients.borrow().iter().copied().collect() }; for client_id in connected_clients { - match self - .get_active_terminal_cursor_position(client_id) - .and_then(|(cursor_position_x, cursor_position_y)| { + match self.get_active_terminal_cursor_position(client_id) { + Some((cursor_position_x, cursor_position_y, is_cursor_visible)) => { let active_pane_z_index = self .get_active_pane_id(client_id) .and_then(|pane_id| self.floating_panes.get_pane_z_index(pane_id)); - if output.cursor_is_visible( + let not_occluded = output.cursor_is_visible( cursor_position_x, cursor_position_y, active_pane_z_index, - ) { - Some((cursor_position_x, cursor_position_y)) - } else { - None - } - }) { - Some((cursor_position_x, cursor_position_y)) => { - let desired_cursor_shape = self - .get_active_pane(client_id) - .map(|ap| ap.cursor_shape_csi()) - .unwrap_or_default(); - let cursor_changed_position_or_shape = self - .cursor_positions_and_shape - .get(&client_id) - .map(|(previous_x, previous_y, previous_shape)| { - previous_x != &cursor_position_x - || previous_y != &cursor_position_y - || previous_shape != &desired_cursor_shape - }) - .unwrap_or(true); + ); let active_terminal_is_mid_frame = self .active_terminal_is_mid_frame(client_id) .unwrap_or(false); @@ -3274,22 +3258,54 @@ impl Tab { // no-op, this means the active terminal is currently rendering a frame, // which means the cursor can be jumping around and we definitely do not // want to render it - // - // (I felt this was clearer than expanding the if conditional below) - } else if output.is_dirty() || cursor_changed_position_or_shape { - let show_cursor = "\u{1b}[?25h"; + } else if not_occluded && is_cursor_visible { + let desired_cursor_shape = self + .get_active_pane(client_id) + .map(|ap| ap.cursor_shape_csi()) + .unwrap_or_default(); + let cursor_changed_position_or_shape = self + .cursor_positions_and_shape + .get(&client_id) + .map(|(previous_x, previous_y, previous_shape)| { + previous_x != &cursor_position_x + || previous_y != &cursor_position_y + || previous_shape != &desired_cursor_shape + }) + .unwrap_or(true); + if output.is_dirty() || cursor_changed_position_or_shape { + let show_cursor = "\u{1b}[?25h"; + let goto_cursor_position = &format!( + "\u{1b}[{};{}H\u{1b}[m{}", + cursor_position_y + 1, + cursor_position_x + 1, + desired_cursor_shape + ); // goto row/col + output.add_post_vte_instruction_to_client(client_id, show_cursor); + output.add_post_vte_instruction_to_client( + client_id, + goto_cursor_position, + ); + self.cursor_positions_and_shape.insert( + client_id, + (cursor_position_x, cursor_position_y, desired_cursor_shape), + ); + } + } else if not_occluded { + // Cursor is hidden by the app but not occluded by a floating + // pane. Position the host terminal cursor at the correct + // location (for IME) then hide it. The IME subsystem reads + // the cursor position regardless of visibility. + let hide_cursor = "\u{1b}[?25l"; let goto_cursor_position = &format!( - "\u{1b}[{};{}H\u{1b}[m{}", + "\u{1b}[{};{}H", cursor_position_y + 1, cursor_position_x + 1, - desired_cursor_shape - ); // goto row/col - output.add_post_vte_instruction_to_client(client_id, show_cursor); - output.add_post_vte_instruction_to_client(client_id, goto_cursor_position); - self.cursor_positions_and_shape.insert( - client_id, - (cursor_position_x, cursor_position_y, desired_cursor_shape), ); + output.add_post_vte_instruction_to_client(client_id, hide_cursor); + output.add_post_vte_instruction_to_client(client_id, goto_cursor_position); + } else { + let hide_cursor = "\u{1b}[?25l"; + output.add_post_vte_instruction_to_client(client_id, hide_cursor); } }, None => { @@ -6004,7 +6020,9 @@ pub fn pane_info_for_pane( pane_info.pane_content_rows = pane.get_content_rows(); pane_info.pane_columns = pane.cols(); pane_info.pane_content_columns = pane.get_content_columns(); - pane_info.cursor_coordinates_in_pane = pane.cursor_coordinates(None); + pane_info.cursor_coordinates_in_pane = pane + .cursor_coordinates(None) + .and_then(|(x, y, is_visible)| if is_visible { Some((x, y)) } else { None }); pane_info.is_selectable = pane.selectable(); pane_info.title = pane.current_title(); pane_info.exited = pane.exited(); diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs index 17ebe9552..46e796b61 100644 --- a/zellij-server/src/tab/unit/tab_integration_tests.rs +++ b/zellij-server/src/tab/unit/tab_integration_tests.rs @@ -990,7 +990,10 @@ fn take_snapshot_and_cursor_position( for &byte in ansi_instructions.as_bytes() { vte_parser.advance(&mut grid, byte); } - (format!("{:?}", grid), grid.cursor_coordinates()) + let coords = grid + .cursor_coordinates() + .and_then(|(x, y, visible)| if visible { Some((x, y)) } else { None }); + (format!("{:?}", grid), coords) } #[test] @@ -14798,3 +14801,47 @@ fn osc99_query_flag_roundtrip() { assert!(!wants_report); assert!(!is_query); } + +#[test] +fn hidden_cursor_still_emits_cup_for_host_terminal_positioning() { + // When an app hides the cursor the host + // terminal still needs a CUP sequence so that IME composition candidates and + // other cursor-position-dependent features appear in the correct location. + let size = Size { cols: 80, rows: 24 }; + let client_id = 1; + let mut tab = create_new_tab(size, ModeInfo::default()); + + // Move cursor to a known position, then hide it + tab.handle_pty_bytes(1, Vec::from("\u{1b}[10;20H\u{1b}[?25l".as_bytes())) + .unwrap(); + + let mut output = Output::default(); + tab.render(&mut output, None).unwrap(); + let serialized = output.serialize().unwrap(); + let client_output = serialized.get(&client_id).unwrap(); + + // The output must contain a hide-cursor instruction + assert!( + client_output.contains("\u{1b}[?25l"), + "Expected hide-cursor sequence in output" + ); + + // The output must contain a CUP sequence positioning the cursor where the app + // left it (row 10, col 20 — 1-indexed in the pane, plus the content offset + // from the pane frame). The key point is that a CUP is emitted at all, even + // though the cursor is hidden. + // + // With draw_pane_frames=true the content offset is (1, 1), so the absolute + // position is row 11, col 21. + assert!( + client_output.contains("\u{1b}[11;21H"), + "Expected CUP sequence for hidden cursor positioning, got: {:?}", + client_output + ); + + // The output must NOT contain a show-cursor instruction + assert!( + !client_output.contains("\u{1b}[?25h"), + "Show-cursor sequence must not be present when app has hidden the cursor" + ); +} diff --git a/zellij-server/src/ui/pane_contents_and_ui.rs b/zellij-server/src/ui/pane_contents_and_ui.rs index 694592af8..db7138f5a 100644 --- a/zellij-server/src/ui/pane_contents_and_ui.rs +++ b/zellij-server/src/ui/pane_contents_and_ui.rs @@ -163,12 +163,16 @@ impl<'a> PaneContentsAndUi<'a> { let cursor_is_visible = self .pane .cursor_coordinates(Some(*fake_cursor_client_id)) - .map(|(x, y)| { - self.output.cursor_is_visible( - self.pane.x() + x, - self.pane.y() + y, - self.z_index, - ) + .and_then(|(x, y, is_visible)| { + if is_visible { + Some(self.output.cursor_is_visible( + self.pane.x() + x, + self.pane.y() + y, + self.z_index, + )) + } else { + None + } }) .unwrap_or(false); if cursor_is_visible { diff --git a/zellij-server/src/unit/screen_tests.rs b/zellij-server/src/unit/screen_tests.rs index 0897ec49d..07ec8623b 100644 --- a/zellij-server/src/unit/screen_tests.rs +++ b/zellij-server/src/unit/screen_tests.rs @@ -63,7 +63,10 @@ fn take_snapshot_and_cursor_coordinates( for &byte in ansi_instructions.as_bytes() { vte_parser.advance(grid, byte); } - (grid.cursor_coordinates(), format!("{:?}", grid)) + let coords = grid + .cursor_coordinates() + .and_then(|(x, y, visible)| if visible { Some((x, y)) } else { None }); + (coords, format!("{:?}", grid)) } fn take_snapshots_and_cursor_coordinates_from_render_events<'a>(