From 76211757ecce73cba981bdf6704954b25a100a8c Mon Sep 17 00:00:00 2001 From: akbash Date: Tue, 18 Aug 2026 00:11:20 +0300 Subject: fix: reap ctrl-click url openers (#2906) * fix: reap ctrl-click url openers refs #2903 * fix: scope linux opener test import refs #2903 * test: isolate url opener regression refs #2903 --------- Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- src/app/input/mod.rs | 29 ++++++++++++++++++++-- src/app/input/navigate.rs | 16 ++++--------- src/app/input/terminal.rs | 61 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 13 deletions(-) (limited to 'src/app/input') diff --git a/src/app/input/mod.rs b/src/app/input/mod.rs index 45124107..597d9296 100644 --- a/src/app/input/mod.rs +++ b/src/app/input/mod.rs @@ -566,6 +566,15 @@ impl App { &mut self, source_id: super::InputSourceId, mouse: MouseEvent, + ) -> bool { + self.handle_modified_url_click_with(source_id, mouse, crate::platform::open_url) + } + + fn handle_modified_url_click_with( + &mut self, + source_id: super::InputSourceId, + mouse: MouseEvent, + open_url: impl FnOnce(&str) -> std::io::Result>, ) -> bool { if self.state.mode != Mode::Terminal || !matches!(mouse.kind, MouseEventKind::Down(MouseButton::Left)) @@ -595,8 +604,12 @@ impl App { tracing::warn!(err = %err, url = %url, "failed to invoke plugin link handler"); } } - if let Err(err) = crate::platform::open_url(&url) { - tracing::warn!(err = %err, url = %url, "failed to open pane URL"); + match open_url(&url) { + Ok(Some(child)) => self.detached_process_children.push(child), + Ok(None) => {} + Err(err) => { + tracing::warn!(err = %err, url = %url, "failed to open pane URL"); + } } true } @@ -897,6 +910,18 @@ fn wait_for_file(path: &std::path::Path) -> String { panic!("timed out waiting for {}", path.display()); } +#[cfg(test)] +#[cfg(unix)] +async fn wait_for_detached_process_reap(app: &mut App, pid: u32) -> bool { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(2); + while crate::platform::process_exists(pid) && tokio::time::Instant::now() < deadline { + app.reap_finished_detached_processes(); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + app.reap_finished_detached_processes(); + !crate::platform::process_exists(pid) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index afe9e6e0..2cb657a7 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -937,7 +937,7 @@ impl App { command.current_dir(cwd); } let child = command.spawn()?; - self.detached_custom_command_children.push(child); + self.detached_process_children.push(child); Ok(()) } @@ -2015,9 +2015,9 @@ mod tests { use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, ModifierKeyCode}; use ratatui::layout::Direction; - #[cfg(unix)] - use super::super::wait_for_file; use super::super::{state_with_workspaces, unique_temp_path}; + #[cfg(unix)] + use super::super::{wait_for_detached_process_reap, wait_for_file}; use super::*; use crate::{ app::App, @@ -3543,16 +3543,10 @@ navigate_pane_down = "ctrl+j" assert_eq!(app.state.mode, Mode::Terminal); std::fs::write(&release_path, b"release").expect("release command"); - let deadline = tokio::time::Instant::now() + Duration::from_secs(2); - while crate::platform::process_exists(pid) && tokio::time::Instant::now() < deadline { - app.reap_finished_custom_commands(); - tokio::time::sleep(Duration::from_millis(20)).await; - } - app.reap_finished_custom_commands(); - let reaped_by_runtime = !crate::platform::process_exists(pid); + let reaped_by_runtime = wait_for_detached_process_reap(&mut app, pid).await; if !reaped_by_runtime { if let Some(child) = app - .detached_custom_command_children + .detached_process_children .iter_mut() .find(|child| child.id() == pid) { diff --git a/src/app/input/terminal.rs b/src/app/input/terminal.rs index 12d09e6a..e5b40a74 100644 --- a/src/app/input/terminal.rs +++ b/src/app/input/terminal.rs @@ -411,6 +411,8 @@ mod tests { use crossterm::event::{KeyCode, KeyEventKind, KeyModifiers, MouseButton, MouseEventKind}; use ratatui::layout::Rect; + #[cfg(target_os = "linux")] + use super::super::wait_for_detached_process_reap; use super::super::{app_for_mouse_test, mouse, numbered_lines_bytes}; #[cfg(unix)] use super::super::{unique_temp_path, wait_for_file}; @@ -885,6 +887,65 @@ mod tests { assert!(app.selection_highlight_clear_deadline.is_none()); } + #[cfg(target_os = "linux")] + #[tokio::test] + async fn ctrl_click_url_reaps_failed_opener() { + let opener_dir = unique_temp_path("url-opener"); + let record_path = opener_dir.join("record"); + let opener_path = opener_dir.join("xdg-open"); + std::fs::create_dir_all(&opener_dir).expect("fake opener directory"); + std::fs::write( + &opener_path, + "#!/bin/sh\nprintf '%s\\n%s\\n' \"$$\" \"$1\" > \"$2\"\nexit 3\n", + ) + .expect("fake opener script"); + + let url = "https://example.com/akbash-2903"; + let line = format!("see {url}"); + let (mut app, info) = app_with_screen_bytes(line.as_bytes()); + let col = info.inner_rect.x + line.find("example").expect("url host") as u16; + let handled = app.handle_modified_url_click_with( + 41, + modified_mouse( + MouseEventKind::Down(MouseButton::Left), + col, + info.inner_rect.y, + KeyModifiers::CONTROL, + ), + |clicked_url| { + std::process::Command::new("/bin/sh") + .arg(&opener_path) + .arg(clicked_url) + .arg(&record_path) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .map(Some) + }, + ); + assert!(handled); + + let record = wait_for_file(&record_path); + let mut lines = record.lines(); + let pid = lines + .next() + .expect("opener pid") + .parse::() + .expect("numeric opener pid"); + assert_eq!(lines.next(), Some(url)); + + let reaped = wait_for_detached_process_reap(&mut app, pid).await; + if !reaped { + unsafe { + libc::waitpid(pid as libc::pid_t, std::ptr::null_mut(), 0); + } + } + + let _ = std::fs::remove_dir_all(&opener_dir); + assert!(reaped, "failed URL opener child {pid} was not reaped"); + } + #[cfg(unix)] #[tokio::test] async fn ctrl_click_url_does_not_forward_release_to_mouse_reporting_pane() { -- cgit v1.2.3