From 5c1fd984f01060c152ee9a0cdef7a2b92c72feab Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 7 Jul 2026 19:48:02 -0300 Subject: [PATCH] fix(terminal-browser): dispatch complete key events --- .../terminal-browser/src/accessibility.rs | 28 +-- codex-rs/terminal-browser/src/actions.rs | 60 +----- .../terminal-browser/src/actions_tests.rs | 95 ++++++++- codex-rs/terminal-browser/src/key_event.rs | 197 ++++++++++++++++++ codex-rs/terminal-browser/src/lib.rs | 1 + codex-rs/terminal-browser/src/scripts.rs | 7 + .../terminal-browser/src/terminal_input.rs | 1 - .../src/terminal_input_tests.rs | 2 +- 8 files changed, 306 insertions(+), 85 deletions(-) create mode 100644 codex-rs/terminal-browser/src/key_event.rs diff --git a/codex-rs/terminal-browser/src/accessibility.rs b/codex-rs/terminal-browser/src/accessibility.rs index 7011797f83..3247f506de 100644 --- a/codex-rs/terminal-browser/src/accessibility.rs +++ b/codex-rs/terminal-browser/src/accessibility.rs @@ -8,6 +8,7 @@ use crate::actions::bounded_snapshot_json; use crate::actions::page_metadata; use crate::cdp::CdpClient; use crate::handles::BrowserHandles; +use crate::key_event; const MAX_AX_NODES: usize = 300; const MAX_SNAPSHOT_TEXT_CHARS: usize = 6_000; @@ -147,31 +148,8 @@ pub(crate) async fn fill( client .call("DOM.focus", json!({ "backendNodeId": backend_node_id })) .await?; - let select_modifier = if cfg!(target_os = "macos") { 4 } else { 2 }; - client - .call( - "Input.dispatchKeyEvent", - json!({ "type": "rawKeyDown", "key": "a", "code": "KeyA", "modifiers": select_modifier }), - ) - .await?; - client - .call( - "Input.dispatchKeyEvent", - json!({ "type": "keyUp", "key": "a", "code": "KeyA", "modifiers": select_modifier }), - ) - .await?; - client - .call( - "Input.dispatchKeyEvent", - json!({ "type": "rawKeyDown", "key": "Backspace", "code": "Backspace" }), - ) - .await?; - client - .call( - "Input.dispatchKeyEvent", - json!({ "type": "keyUp", "key": "Backspace", "code": "Backspace" }), - ) - .await?; + key_event::dispatch_select_all(client).await?; + key_event::dispatch_backspace(client).await?; client .call("Input.insertText", json!({ "text": text })) .await?; diff --git a/codex-rs/terminal-browser/src/actions.rs b/codex-rs/terminal-browser/src/actions.rs index 681f87183c..663c5143ee 100644 --- a/codex-rs/terminal-browser/src/actions.rs +++ b/codex-rs/terminal-browser/src/actions.rs @@ -5,6 +5,7 @@ use serde_json::json; use crate::cdp::CdpClient; use crate::input::BrowserKeyInput; +use crate::key_event; use crate::scripts; const MAX_SCREENSHOT_BYTES: usize = 4 * 1024 * 1024; @@ -43,31 +44,7 @@ pub(crate) async fn page_metadata(client: &CdpClient) -> Result { pub(crate) async fn press(client: &CdpClient, key: &str) -> Result { anyhow::ensure!(!key.is_empty(), "key must not be empty"); anyhow::ensure!(key.chars().count() <= 32, "key is too long"); - let code = scripts::key_code(key); - let text = scripts::key_text(key); - let event_type = if text.is_empty() { - "rawKeyDown" - } else { - "keyDown" - }; - client - .call( - "Input.dispatchKeyEvent", - json!({ - "type": event_type, - "key": key, - "code": code, - "text": text, - "unmodifiedText": text, - }), - ) - .await?; - client - .call( - "Input.dispatchKeyEvent", - json!({ "type": "keyUp", "key": key, "code": code }), - ) - .await?; + key_event::dispatch_tool_key(client, key).await?; Ok(BrowserToolOutput::Text(format!("pressed {key}"))) } @@ -104,38 +81,7 @@ pub(crate) async fn screenshot(client: &CdpClient) -> Result } pub(crate) async fn dispatch_human_key(client: &CdpClient, input: &BrowserKeyInput) -> Result<()> { - let text = input.text.as_deref().unwrap_or_default(); - let event_type = if text.is_empty() { - "rawKeyDown" - } else { - "keyDown" - }; - let modifiers = input.modifiers.cdp_mask(); - client - .call( - "Input.dispatchKeyEvent", - json!({ - "type": event_type, - "key": input.key, - "code": input.code, - "text": text, - "unmodifiedText": text, - "modifiers": modifiers, - }), - ) - .await?; - client - .call( - "Input.dispatchKeyEvent", - json!({ - "type": "keyUp", - "key": input.key, - "code": input.code, - "modifiers": modifiers, - }), - ) - .await?; - Ok(()) + key_event::dispatch_human_key(client, input).await } pub(crate) async fn insert_human_text(client: &CdpClient, text: &str) -> Result<()> { diff --git a/codex-rs/terminal-browser/src/actions_tests.rs b/codex-rs/terminal-browser/src/actions_tests.rs index 91e2ec204d..dec1a2e658 100644 --- a/codex-rs/terminal-browser/src/actions_tests.rs +++ b/codex-rs/terminal-browser/src/actions_tests.rs @@ -12,6 +12,7 @@ use super::BrowserToolOutput; use super::dispatch_human_key; use super::press; use crate::accessibility::click; +use crate::accessibility::fill; use crate::accessibility::snapshot; use crate::cdp::CdpClient; use crate::handles::BrowserHandles; @@ -188,18 +189,28 @@ async fn keyboard_input_uses_key_down_when_chromium_needs_text() { let (url, server) = test_server(|mut socket| { tokio::spawn(async move { initialize(&mut socket).await; - for (expected_key, expected_text) in [("Enter", "\r"), ("?", "?")] { + for (expected_key, expected_text, expected_virtual_key_code) in + [("Enter", "\r", 13), ("?", "?", 191), ("Enter", "\r", 13)] + { let key_down = request(&mut socket).await; assert_eq!(key_down["method"], "Input.dispatchKeyEvent"); assert_eq!(key_down["params"]["type"], "keyDown"); assert_eq!(key_down["params"]["key"], expected_key); assert_eq!(key_down["params"]["text"], expected_text); + assert_eq!( + key_down["params"]["windowsVirtualKeyCode"], + expected_virtual_key_code + ); respond(&mut socket, &key_down, json!({})).await; let key_up = request(&mut socket).await; assert_eq!(key_up["method"], "Input.dispatchKeyEvent"); assert_eq!(key_up["params"]["type"], "keyUp"); assert_eq!(key_up["params"]["key"], expected_key); + assert_eq!( + key_up["params"]["windowsVirtualKeyCode"], + expected_virtual_key_code + ); respond(&mut socket, &key_up, json!({})).await; } }) @@ -222,10 +233,92 @@ async fn keyboard_input_uses_key_down_when_chromium_needs_text() { ) .await .expect("dispatch human key"); + dispatch_human_key( + &client, + &BrowserKeyInput { + key: "Enter".to_string(), + code: "Enter".to_string(), + text: None, + modifiers: BrowserInputModifiers::default(), + }, + ) + .await + .expect("dispatch human Enter"); server.await.expect("server task"); } +#[tokio::test] +async fn fill_selects_and_replaces_existing_text() { + let select_modifier = if cfg!(target_os = "macos") { 4 } else { 2 }; + let (url, server) = test_server(move |mut socket| { + tokio::spawn(async move { + initialize(&mut socket).await; + let focus = request(&mut socket).await; + assert_eq!(focus["method"], "DOM.focus"); + assert_eq!(focus["params"], json!({ "backendNodeId": 7 })); + respond(&mut socket, &focus, json!({})).await; + + for expected_params in [ + json!({ + "type": "rawKeyDown", + "key": "a", + "code": "KeyA", + "text": "", + "unmodifiedText": "", + "modifiers": select_modifier, + "windowsVirtualKeyCode": 65, + "commands": ["selectAll"], + }), + json!({ + "type": "keyUp", + "key": "a", + "code": "KeyA", + "modifiers": select_modifier, + "windowsVirtualKeyCode": 65, + }), + json!({ + "type": "rawKeyDown", + "key": "Backspace", + "code": "Backspace", + "text": "", + "unmodifiedText": "", + "modifiers": 0, + "windowsVirtualKeyCode": 8, + }), + json!({ + "type": "keyUp", + "key": "Backspace", + "code": "Backspace", + "modifiers": 0, + "windowsVirtualKeyCode": 8, + }), + ] { + let key_event = request(&mut socket).await; + assert_eq!(key_event["method"], "Input.dispatchKeyEvent"); + assert_eq!(key_event["params"], expected_params); + respond(&mut socket, &key_event, json!({})).await; + } + + let insert_text = request(&mut socket).await; + assert_eq!(insert_text["method"], "Input.insertText"); + assert_eq!(insert_text["params"], json!({ "text": "Buy milk" })); + respond(&mut socket, &insert_text, json!({})).await; + }) + }) + .await; + let client = CdpClient::connect(&url).await.expect("connect client"); + let mut handles = BrowserHandles::default(); + let node_id = handles.insert(/*backend_node_id*/ 7); + + let output = fill(&client, &handles, &node_id, "Buy milk") + .await + .expect("fill textbox"); + + assert_eq!(output, BrowserToolOutput::Text(format!("filled {node_id}"))); + server.await.expect("server task"); +} + #[tokio::test] async fn navigation_waits_for_the_cdp_lifecycle_event() { let (url, server) = test_server(|mut socket| { diff --git a/codex-rs/terminal-browser/src/key_event.rs b/codex-rs/terminal-browser/src/key_event.rs new file mode 100644 index 0000000000..305d4442d8 --- /dev/null +++ b/codex-rs/terminal-browser/src/key_event.rs @@ -0,0 +1,197 @@ +use anyhow::Result; +use serde_json::Map; +use serde_json::Value; +use serde_json::json; + +use crate::cdp::CdpClient; +use crate::input::BrowserKeyInput; +use crate::scripts; + +const CONTROL_MODIFIER: u8 = 2; +const META_MODIFIER: u8 = 4; + +struct KeyEvent<'a> { + key: &'a str, + code: &'a str, + text: &'a str, + modifiers: u8, + windows_virtual_key_code: u32, + commands: &'static [&'static str], +} + +pub(crate) async fn dispatch_tool_key(client: &CdpClient, key: &str) -> Result<()> { + let event = KeyEvent { + key, + code: scripts::key_code(key), + text: scripts::key_text(key), + modifiers: 0, + windows_virtual_key_code: windows_virtual_key_code(key, scripts::key_code(key)), + commands: &[], + }; + dispatch(client, event).await +} + +pub(crate) async fn dispatch_human_key(client: &CdpClient, input: &BrowserKeyInput) -> Result<()> { + let text = input + .text + .as_deref() + .unwrap_or_else(|| scripts::control_key_text(&input.key)); + let event = KeyEvent { + key: &input.key, + code: &input.code, + text, + modifiers: input.modifiers.cdp_mask(), + windows_virtual_key_code: windows_virtual_key_code(&input.key, &input.code), + commands: &[], + }; + dispatch(client, event).await +} + +pub(crate) async fn dispatch_select_all(client: &CdpClient) -> Result<()> { + let modifiers = if cfg!(target_os = "macos") { + META_MODIFIER + } else { + CONTROL_MODIFIER + }; + dispatch( + client, + KeyEvent { + key: "a", + code: "KeyA", + text: "", + modifiers, + windows_virtual_key_code: 65, + commands: &["selectAll"], + }, + ) + .await +} + +pub(crate) async fn dispatch_backspace(client: &CdpClient) -> Result<()> { + dispatch( + client, + KeyEvent { + key: "Backspace", + code: "Backspace", + text: "", + modifiers: 0, + windows_virtual_key_code: 8, + commands: &[], + }, + ) + .await +} + +async fn dispatch(client: &CdpClient, event: KeyEvent<'_>) -> Result<()> { + let event_type = if event.text.is_empty() { + "rawKeyDown" + } else { + "keyDown" + }; + let mut key_down = Map::from_iter([ + ("type".to_string(), json!(event_type)), + ("key".to_string(), json!(event.key)), + ("code".to_string(), json!(event.code)), + ("text".to_string(), json!(event.text)), + ("unmodifiedText".to_string(), json!(event.text)), + ("modifiers".to_string(), json!(event.modifiers)), + ( + "windowsVirtualKeyCode".to_string(), + json!(event.windows_virtual_key_code), + ), + ]); + if !event.commands.is_empty() { + key_down.insert("commands".to_string(), json!(event.commands)); + } + client + .call("Input.dispatchKeyEvent", Value::Object(key_down)) + .await?; + client + .call( + "Input.dispatchKeyEvent", + json!({ + "type": "keyUp", + "key": event.key, + "code": event.code, + "modifiers": event.modifiers, + "windowsVirtualKeyCode": event.windows_virtual_key_code, + }), + ) + .await?; + Ok(()) +} + +fn windows_virtual_key_code(key: &str, code: &str) -> u32 { + match key { + "Backspace" => 8, + "Tab" => 9, + "Enter" => 13, + "Shift" => 16, + "Control" => 17, + "Alt" => 18, + "Escape" => 27, + "Space" => 32, + "PageUp" => 33, + "PageDown" => 34, + "End" => 35, + "Home" => 36, + "ArrowLeft" => 37, + "ArrowUp" => 38, + "ArrowRight" => 39, + "ArrowDown" => 40, + "Insert" => 45, + "Delete" => 46, + "Meta" => 91, + _ => code_virtual_key_code(code).unwrap_or_else(|| character_virtual_key_code(key)), + } +} + +fn code_virtual_key_code(code: &str) -> Option { + if let Some(letter) = code.strip_prefix("Key") + && letter.len() == 1 + { + return letter.chars().next().map(u32::from); + } + if let Some(digit) = code.strip_prefix("Digit") + && digit.len() == 1 + { + return digit.chars().next().map(u32::from); + } + match code { + "Semicolon" => Some(186), + "Equal" => Some(187), + "Comma" => Some(188), + "Minus" => Some(189), + "Period" => Some(190), + "Slash" => Some(191), + "Backquote" => Some(192), + "BracketLeft" => Some(219), + "Backslash" => Some(220), + "BracketRight" => Some(221), + "Quote" => Some(222), + _ => None, + } +} + +fn character_virtual_key_code(key: &str) -> u32 { + let Some(character) = key.chars().next().filter(|_| key.chars().count() == 1) else { + return 0; + }; + if character.is_ascii_alphabetic() { + return u32::from(character.to_ascii_uppercase()); + } + match character { + ';' | ':' => 186, + '=' | '+' => 187, + ',' | '<' => 188, + '-' | '_' => 189, + '.' | '>' => 190, + '/' | '?' => 191, + '`' | '~' => 192, + '[' | '{' => 219, + '\\' | '|' => 220, + ']' | '}' => 221, + '\'' | '"' => 222, + _ => u32::from(character), + } +} diff --git a/codex-rs/terminal-browser/src/lib.rs b/codex-rs/terminal-browser/src/lib.rs index 54b18e5116..736846d3ab 100644 --- a/codex-rs/terminal-browser/src/lib.rs +++ b/codex-rs/terminal-browser/src/lib.rs @@ -10,6 +10,7 @@ mod handles; mod human_control; mod human_navigation; mod input; +mod key_event; mod navigation; mod network; mod process; diff --git a/codex-rs/terminal-browser/src/scripts.rs b/codex-rs/terminal-browser/src/scripts.rs index c984aa277f..ca6c2179ce 100644 --- a/codex-rs/terminal-browser/src/scripts.rs +++ b/codex-rs/terminal-browser/src/scripts.rs @@ -28,3 +28,10 @@ pub(crate) fn key_text(key: &str) -> &str { _ => "", } } + +pub(crate) fn control_key_text(key: &str) -> &str { + match key { + "Enter" => "\r", + _ => "", + } +} diff --git a/codex-rs/terminal-browser/src/terminal_input.rs b/codex-rs/terminal-browser/src/terminal_input.rs index 7ab0cbbaf7..1d181a4926 100644 --- a/codex-rs/terminal-browser/src/terminal_input.rs +++ b/codex-rs/terminal-browser/src/terminal_input.rs @@ -22,7 +22,6 @@ pub(crate) fn key_bytes(input: &BrowserKeyInput) -> Option> { return text.is_ascii().then(|| text.as_bytes().to_vec()); } match input.key.as_str() { - "Enter" => Some(vec![b'\r']), "Tab" if !input.modifiers.shift => Some(vec![b'\t']), "Backspace" => Some(vec![0x7f]), "Escape" => Some(vec![0x1b]), diff --git a/codex-rs/terminal-browser/src/terminal_input_tests.rs b/codex-rs/terminal-browser/src/terminal_input_tests.rs index 83cd8e222d..a6b899ff4c 100644 --- a/codex-rs/terminal-browser/src/terminal_input_tests.rs +++ b/codex-rs/terminal-browser/src/terminal_input_tests.rs @@ -30,7 +30,7 @@ fn plain_text_and_navigation_keys_use_carbonyl_terminal_input() { }; assert_eq!(key_bytes(&text), Some(b"?".to_vec())); - assert_eq!(key_bytes(&enter), Some(b"\r".to_vec())); + assert_eq!(key_bytes(&enter), None); assert_eq!(key_bytes(&command_left), Some(b"\x1b[1;9D".to_vec())); }