diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index 0c2c490ba1..c17adf2be9 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -8,12 +8,14 @@ use wordcraft_layout::{NumeralMode, ViewMode}; use super::sel_result; use crate::{CmdError, CmdResult, CommandSpec, Session, p}; +/// Toggles or sets a boolean view property, returning the updated state. fn toggle(s: &mut Session, v: &Value, f: fn(&mut crate::ViewState) -> &mut bool) -> CmdResult { let cur = *f(&mut s.view); *f(&mut s.view) = p::bool(v, "value").unwrap_or(!cur); Ok(json!({"value": *f(&mut s.view)})) } +/// Returns the command specifications for the View ribbon tab and window management commands. pub fn specs() -> Vec { vec![ CommandSpec::new("view.marks", "Show/Hide ¶", "Home › Paragraph", |s, v| { @@ -143,7 +145,7 @@ pub fn specs() -> Vec { }) .pure(), CommandSpec::new("view.switchWindows", "Switch Windows", "View › Window", switch_windows) - .params(r#"{"id"?: n (make this open document the active one)} → {"documents": [{"id", "title", "path", "dirty", "active"}]} in the order they were opened"#) + .params(r#"{"id"?: n (make this open document the active one), "window"?: n (make this open window the active one)} → {"documents": [{"id", "title", "path", "dirty", "active"}]} in the order they were opened"#) .pure(), CommandSpec::new("view.split", "Split", "View › Window", |s, _| { s.push_ui_request(json!({"toggle": "split"})); @@ -213,6 +215,8 @@ pub fn specs() -> Vec { pub struct Windows { /// The host can list and move its windows (the desktop app). pub available: bool, + /// The active window, when known. + pub current: Option, /// The other open windows, by id. pub others: Vec, /// View Side by Side is on, with this window. @@ -431,6 +435,7 @@ pub(crate) fn restore_zoom(s: &mut Session, z: Zoom) { s.view.single_column = fit == "onePage" || fit == "pageWidth"; } +/// Sets the view zoom level or fit mode, or requests opening the Zoom dialog. fn zoom(s: &mut Session, v: &Value) -> CmdResult { match v.get("value") { Some(Value::String(f)) if ["pageWidth", "onePage", "multiplePages", "textWidth"].contains(&f.as_str()) => { @@ -456,7 +461,13 @@ fn zoom(s: &mut Session, v: &Value) -> CmdResult { /// View › Window › Switch Windows: the open documents, or make one of them active. fn switch_windows(s: &mut Session, v: &Value) -> CmdResult { - if let Some(x) = v.get("id").filter(|x| !x.is_null()) { + if let Some(w) = v.get("window").filter(|x| !x.is_null()) { + let wid = w.as_u64().ok_or_else(|| CmdError::Params("`window` must be an open window's id".into()))?; + if s.windows.available && s.windows.current != Some(wid) && !s.windows.others.iter().any(|other| other.id == wid) { + return Err(CmdError::Params(format!("no open window {wid}"))); + } + s.push_ui_request(json!({"windows": "switchWindow", "window": wid})); + } else if let Some(x) = v.get("id").filter(|x| !x.is_null()) { let id = x.as_u64().ok_or_else(|| CmdError::Params("`id` must be an open document's id".into()))?; s.activate(id)?; } diff --git a/crates/engine/src/tests/documents.rs b/crates/engine/src/tests/documents.rs index 7dd108b893..cdeeff5135 100644 --- a/crates/engine/src/tests/documents.rs +++ b/crates/engine/src/tests/documents.rs @@ -214,6 +214,7 @@ fn read_aloud_belongs_to_its_document() { assert_eq!(s.read_aloud.document, Some(b)); } +/// Tests that `view.switchWindows` lists open documents, activates by document ID, and requests switching by window ID. #[test] fn switch_windows_lists_and_activates() { let mut s = s(); @@ -228,10 +229,23 @@ fn switch_windows_lists_and_activates() { assert!(list[0].get("path").is_some() && list[0].get("dirty").is_some()); run(&mut s, "view.switchWindows", json!({"id": a})); assert_eq!(s.document_id(), a); + s.ui_requests.clear(); + run(&mut s, "view.switchWindows", json!({"window": 1})); + assert_eq!(s.ui_requests, vec![json!({"windows": "switchWindow", "window": 1})]); + s.windows.available = true; + s.windows.current = Some(1); + s.windows.others = vec![crate::cmd::view::OtherWindow { id: 2, title: "Second".into() }]; + s.ui_requests.clear(); + run(&mut s, "view.switchWindows", json!({"window": 2})); + assert_eq!(s.ui_requests, vec![json!({"windows": "switchWindow", "window": 2})]); + assert!(s.run("view.switchWindows", &json!({"window": 3})).is_err(), "unknown window key fails when windows are available"); for bad in [json!({"id": 0}), json!({"id": u64::MAX}), json!({"id": -5}), json!({"id": "1"}), json!({"id": 1e300}), json!({"id": {}})] { assert!(s.run("view.switchWindows", &bad).is_err(), "{bad}"); assert_eq!(s.document_id(), a); } + for bad in [json!({"window": "1"}), json!({"window": -1}), json!({"window": {}})] { + assert!(s.run("view.switchWindows", &bad).is_err(), "{bad}"); + } let junk = [json!(null), json!([]), json!({"id": null}), json!({"text": 5})]; for id in ["view.switchWindows", "file.closeDocument", "file.new"] { for j in &junk { diff --git a/crates/ui-egui/src/control/geometry.rs b/crates/ui-egui/src/control/geometry.rs index 0363eea447..20a97795a5 100644 --- a/crates/ui-egui/src/control/geometry.rs +++ b/crates/ui-egui/src/control/geometry.rs @@ -67,6 +67,10 @@ pub(crate) struct CaptureGeometry { pub(super) document: u64, } +/// Explanation returned when a capture is attempted on a secondary viewport. +pub(crate) const UNSUPPORTED_SECONDARY_WINDOW_ERROR: &str = + "capturing a secondary document window is not supported by the viewport renderer; ui.screenshot captures only the window in the root viewport"; + impl CaptureGeometry { /// Returns the document id the capture was admitted for. pub(crate) fn document(self) -> u64 { @@ -85,10 +89,12 @@ impl CaptureGeometry { } } - /// Explains why the surface cannot be captured, when it is minimized or fully occluded, and - /// returns `None` when it can. + /// Explains why the surface cannot be captured: when it is in a secondary viewport that the + /// viewport renderer cannot capture, or when the window is minimized or fully occluded. pub(crate) fn unavailable_reason(self) -> Option<&'static str> { - if self.surface.minimized { + if self.viewport != egui::ViewportId::ROOT { + Some(UNSUPPORTED_SECONDARY_WINDOW_ERROR) + } else if self.surface.minimized { Some("the window is minimized") } else if self.surface.occluded { Some("the window is fully occluded") diff --git a/crates/ui-egui/src/control/screenshots.rs b/crates/ui-egui/src/control/screenshots.rs index b421915ed8..47044ff35a 100644 --- a/crates/ui-egui/src/control/screenshots.rs +++ b/crates/ui-egui/src/control/screenshots.rs @@ -1,4 +1,4 @@ -use super::geometry::CaptureGeometry; +use super::geometry::{CaptureGeometry, UNSUPPORTED_SECONDARY_WINDOW_ERROR}; use super::{CaptureSlot, Sender, Value, WordApp, json}; /// A page rendered by the engine, as base64 PNG in the reply (the same shape as the headless @@ -128,6 +128,19 @@ pub(crate) fn admit_screenshot(app: &mut WordApp, ctx: &egui::Context, path: Opt reject_shot(reply, token, "screenshot failed: the window geometry is not known yet"); return; }; + if geometry.viewport != egui::ViewportId::ROOT { + let msg = match app.root_window_info() { + Some((root_key, _doc_id, root_title)) if !root_title.is_empty() => format!( + "screenshot failed: {UNSUPPORTED_SECONDARY_WINDOW_ERROR}. Select the root window first (view.switchWindows {{\"window\": {root_key}}}) for {root_title:?}." + ), + Some((root_key, ..)) => format!( + "screenshot failed: {UNSUPPORTED_SECONDARY_WINDOW_ERROR}. Select the root window first (view.switchWindows {{\"window\": {root_key}}})." + ), + None => format!("screenshot failed: {UNSUPPORTED_SECONDARY_WINDOW_ERROR}."), + }; + reject_shot(reply, token, msg); + return; + } if let Some(reason) = geometry.unavailable_reason() { reject_shot(reply, token, format!("screenshot failed: {reason}")); return; diff --git a/crates/ui-egui/src/windows.rs b/crates/ui-egui/src/windows.rs index 4acbed1af7..fefbe96d73 100644 --- a/crates/ui-egui/src/windows.rs +++ b/crates/ui-egui/src/windows.rs @@ -33,6 +33,7 @@ use craft_window::{Document, Frame, Viewports, WindowEvent, WindowId, WindowList use egui::{ViewportCommand, ViewportId}; use serde_json::{Value, json}; use wordcraft_engine::WindowView; +use wordcraft_engine::cmd::view::OtherWindow; use crate::{WordApp, documents, window_ui::WindowUi}; @@ -306,6 +307,17 @@ impl WordApp { (viewport, crate::control::SurfaceGeometry::from_viewport(&info, screen_rect, max_texture_side, ctx)) } + /// Returns the window key, document ID and display title of the window currently occupying + /// the root viewport, if one exists and native viewports are in use. + pub(crate) fn root_window_info(&self) -> Option<(u64, u64, String)> { + let viewports = self.windows.viewports.as_ref()?; + let root_id = viewports.root()?; + let root_win = self.windows.open.iter().find(|w| w.id == Some(root_id))?; + let doc_id = root_win.doc; + let title = self.session.documents().iter().find(|d| d.id == doc_id).map(documents::display_title).unwrap_or_default(); + Some((root_win.key, doc_id, title)) + } + /// The viewport new windows start from: the host's size, icon and app id. Their title and /// place are the app's. pub fn set_window_builder(&mut self, builder: egui::ViewportBuilder) { @@ -663,6 +675,15 @@ impl WordApp { self.settle_windows(); if let Some(&active) = self.windows.recent_keys().first() { self.enter_window(active); + self.session.windows.current = Some(active); + if self.session.windows.available { + self.session.windows.others = self + .switch_windows_entries() + .into_iter() + .filter(|(k, ..)| *k != active) + .map(|(id, title, _)| OtherWindow { id, title }) + .collect(); + } } } diff --git a/crates/ui-egui/src/windows/arrange.rs b/crates/ui-egui/src/windows/arrange.rs index 9ee478dd41..5525bb8368 100644 --- a/crates/ui-egui/src/windows/arrange.rs +++ b/crates/ui-egui/src/windows/arrange.rs @@ -71,6 +71,7 @@ pub(crate) fn poll(app: &mut WordApp, _ctx: &egui::Context) { }; let w = &mut app.session.windows; w.available = native; + w.current = Some(key); w.others = others; w.side_by_side = partner; w.sync_scroll = sync; @@ -99,6 +100,11 @@ pub(crate) fn request(app: &mut WordApp, req: &Value) { let key = app.windows.shown; let id = |k: &str| req.get(k).and_then(Value::as_u64); match what { + "switchWindow" => { + if let Some(w) = id("window") { + app.show_window(w); + } + } "sideBySide" => { app.canvas.sync_in = 0.0; if id("off").is_some() { diff --git a/crates/ui-egui/src/windows/tests.rs b/crates/ui-egui/src/windows/tests.rs index fd038cb751..444d353606 100644 --- a/crates/ui-egui/src/windows/tests.rs +++ b/crates/ui-egui/src/windows/tests.rs @@ -386,11 +386,14 @@ fn control_typing_reaches_the_active_window() { assert_eq!(text(&a), "First", "the other window got none of it"); } -/// `ui.screenshot` captures the active window: the capture is sized from its viewport and the -/// command goes to that viewport. A capture still pending when another window comes forward -/// fails with an error instead of capturing that window. +/// `ui.screenshot` captures the window in the root viewport: capturing a secondary document +/// window in a separate native viewport is not supported by the viewport renderer and is refused +/// immediately at admission with an error directing to `view.switchWindows` (#6159). A capture +/// still pending when another window comes forward fails with an error instead of capturing that +/// window. When a window is adopted into the root viewport after the first window closes, it can +/// be captured. #[test] -fn screenshot_captures_the_active_window() { +fn screenshot_captures_the_root_window_and_refuses_secondary_windows() { let ctx = backend(); let mut a = app("First"); frames(&ctx, &mut a, 2); @@ -403,26 +406,102 @@ fn screenshot_captures_the_active_window() { let (reply, replies) = std::sync::mpsc::channel(); crate::control::admit_screenshot(&mut a, &ctx, None, reply); - assert!(replies.try_recv().is_err(), "the capture is admitted"); - assert_eq!(a.shots.requests.iter().map(|s| (s.viewport, s.expected)).collect::>(), [(child, [800, 600])]); + let v = replies.try_recv().expect("a secondary window capture is refused at admission"); + assert_eq!(v["ok"], false, "{v}"); + let err = v["error"].as_str().expect("error message"); + assert!(err.contains("capturing a secondary document window is not supported by the viewport renderer"), "error explains why: {err}"); + assert!(err.contains("view.switchWindows {\"window\": 1}"), "error tells how to activate the root window: {err}"); + assert!(a.shots.requests.is_empty(), "refused capture is never queued"); + + focus(ViewportId::ROOT); + frames(&ctx, &mut a, 2); + assert_eq!(a.active_viewport(), ViewportId::ROOT, "the root window is now active"); + let (reply, replies) = std::sync::mpsc::channel(); + crate::control::admit_screenshot(&mut a, &ctx, None, reply); + assert!(replies.try_recv().is_err(), "the capture of the root window is admitted"); + assert_eq!(a.shots.requests.len(), 1); + assert_eq!(a.shots.requests[0].viewport, ViewportId::ROOT); a.shots.requests.iter_mut().for_each(|s| s.ready_at = 0.0); COMMANDS.with(|c| c.borrow_mut().clear()); frames(&ctx, &mut a, 4); let shot_to = |id: ViewportId| COMMANDS.with(|c| c.borrow().get(&id).is_some_and(|v| v.iter().any(|c| matches!(c, ViewportCommand::Screenshot(_))))); - assert!(shot_to(child), "the active window is captured"); - assert!(!shot_to(ViewportId::ROOT), "the first window is not"); + assert!(shot_to(ViewportId::ROOT), "the root window is captured"); + assert!(!shot_to(child), "the child window is not"); let (reply, later) = std::sync::mpsc::channel(); crate::control::admit_screenshot(&mut a, &ctx, None, reply); + focus(child); + frames(&ctx, &mut a, 2); + let v = later.try_recv().expect("the capture was answered"); + assert_eq!(v["ok"], false); + assert!(v["error"].as_str().is_some_and(|e| e.contains("another window")), "{v}"); + assert!(a.shots.requests.is_empty()); + + // When the first window closes, the child window is adopted into the root viewport: + let first_key = a.windows.open.iter().find(|w| w.id == a.windows.viewports.as_ref().unwrap().root()).unwrap().key; + let child_id = a.windows.open.iter().find(|w| w.key != first_key).unwrap().id; + a.close_window(first_key); + a.run("ui.saveChanges", json!({"answer": "dontSave"})).unwrap(); + frames(&ctx, &mut a, 2); + assert_eq!(a.window_count(), 1); + assert_eq!(a.windows.viewports.as_ref().unwrap().root(), child_id); + assert_eq!(a.active_viewport(), ViewportId::ROOT, "the adopted window now occupies the root viewport"); + let (reply, replies) = std::sync::mpsc::channel(); + crate::control::admit_screenshot(&mut a, &ctx, None, reply); + assert!(replies.try_recv().is_err(), "the adopted window's capture is admitted"); + assert_eq!(a.shots.requests.len(), 1); + assert_eq!(a.shots.requests[0].viewport, ViewportId::ROOT); +} + +/// #6159: when two windows are open on the same document (`view.newWindow`), `ui.screenshot` +/// while the secondary window is active is refused at admission. Using `view.switchWindows` with +/// `window` selects the root window, allowing a screenshot retry to be admitted and issued. +#[test] +fn screenshot_refuses_secondary_window_on_same_document_and_recovers_via_switch_windows() { + let ctx = backend(); + let mut a = app("Shared Doc"); + frames(&ctx, &mut a, 2); + a.run("view.newWindow", json!({})).unwrap(); + let child = a.active_viewport(); + let rect = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(800.0, 600.0)); + INFOS.with(|i| i.borrow_mut().entry(child).or_default().inner_rect = Some(rect)); + focus(child); + frames(&ctx, &mut a, 4); + assert_ne!(a.active_viewport(), ViewportId::ROOT); + + let (reply, replies) = std::sync::mpsc::channel(); + crate::control::admit_screenshot(&mut a, &ctx, None, reply); + let v = replies.try_recv().expect("secondary window screenshot refused at admission"); + assert_eq!(v["ok"], false, "{v}"); + let err = v["error"].as_str().expect("error message"); + assert!(err.contains("capturing a secondary document window is not supported by the viewport renderer"), "error: {err}"); + assert!(err.contains("view.switchWindows {\"window\": 1}"), "error recommends root window selection: {err}"); + + // Recovery path: select the root window via view.switchWindows {"window": 1} + COMMANDS.with(|c| c.borrow_mut().clear()); + a.run("view.switchWindows", json!({"window": 1})).unwrap(); + assert_eq!(a.active_viewport(), ViewportId::ROOT, "switchWindows selected the root window"); + // Simulate the desktop applying the focus request: focus(ViewportId::ROOT); frames(&ctx, &mut a, 2); - for r in [replies, later] { - let v = r.try_recv().expect("the capture was answered"); - assert_eq!(v["ok"], false); - assert!(v["error"].as_str().is_some_and(|e| e.contains("another window")), "{v}"); - } - assert!(a.shots.requests.is_empty()); + let root_focused = COMMANDS.with(|c| c.borrow().get(&ViewportId::ROOT).is_some_and(|v| v.iter().any(|c| matches!(c, ViewportCommand::Focus)))); + assert!(root_focused, "switchWindows requested focus on root viewport"); + assert_eq!(a.active_viewport(), ViewportId::ROOT, "root window is now active"); + + let (reply, replies) = std::sync::mpsc::channel(); + crate::control::admit_screenshot(&mut a, &ctx, None, reply); + assert!(replies.try_recv().is_err(), "screenshot admitted on retry"); + assert_eq!(a.shots.requests.len(), 1); + assert_eq!(a.shots.requests[0].viewport, ViewportId::ROOT); + + a.shots.requests.iter_mut().for_each(|s| s.ready_at = 0.0); + COMMANDS.with(|c| c.borrow_mut().clear()); + frames(&ctx, &mut a, 4); + let shot_to = + |id: ViewportId| COMMANDS.with(|c| c.borrow().get(&id).is_some_and(|v| v.iter().any(|c| matches!(c, ViewportCommand::Screenshot(_))))); + assert!(shot_to(ViewportId::ROOT), "root window screenshot was issued"); + assert!(!shot_to(child), "secondary window was not issued"); } /// Quitting with two documents of unsaved changes asks about each in its window, the window diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 6fc0451d7e..5f5d1529cf 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -1,6 +1,6 @@ # Control protocol -> **Last updated:** 2026-10-31 · #2596: accept-thread startup failure disables the server and removes its key file; MCP retry policy distinguishes file exports from in-memory images. +> **Last updated:** 2026-11-09 · #6159: refuse ui.screenshot on secondary document windows and support root window selection with view.switchWindows. Start the app with a control port: `wordcraft --control 7981` (or `WORDCRAFT_CONTROL_PORT=7981`). A missing or invalid port after `--control` (not 1–65535) prints an error and exits with status 2. @@ -72,9 +72,11 @@ Unauthenticated connections are limited to 64 while they wait for their first au The app's native windows (one per document, or several on one document) take part in View › Window (#322): `view.arrangeAll` tiles them over the monitor, `view.sideBySide {value?, window?}` places this window and another one in the left and right halves and turns on Synchronous -Scrolling, `view.syncScroll {value?}` turns that on or off, and `view.switchWindows {id?}` lists -and switches documents. `window` is a window id from `ui.inspect` → `windows.others`, the windows -other than the one the command runs in (optional when only one other window is open). The +Scrolling, `view.syncScroll {value?}` turns that on or off, and `view.switchWindows {id?, window?}` +lists documents and switches documents or windows. For Side by Side, `window` is an id from +`ui.inspect` → `windows.others` and is optional when only one other window is open. For Switch +Windows, `window` is an open window's key. A secondary-window screenshot refusal can include +the root window key in the suggested `view.switchWindows` command. The commands are disabled where there is one window (web, CLI, MCP without the app). Wayland doesn't let apps move windows, so there Arrange All and Side by Side only resize. @@ -93,7 +95,7 @@ let apps move windows, so there Arrange All and Side by Side only resize. | `ui.wheel` | `x`, `y`, `dy`, `dx?`, `unit?` (`point`/`line`/`page`), `phase?` (`start`/`move`/`end`), modifiers | mouse-wheel or trackpad scroll at window coordinates (`dy` < 0 scrolls down) | | `ui.key` | `key` (`B`, `Enter`, `Left`…), `shift?`, `alt?`, `cmd?` | key press through egui (shortcuts apply) | | `ui.text` | `text` | typed text through egui | -| `ui.screenshot` | `path?` | PNG of the window: written to `path`, or without one returned as base64 `png` in the reply | +| `ui.screenshot` | `path?` | PNG of the window in the root viewport: written to `path`, or without one returned as base64 `png` in the reply. Capturing a secondary document window is not supported by the viewport renderer; select the root window with `view.switchWindows {"window": root}` first | | `ui.render` | `path?`, `page?` (1-based), `scale?`, `doc?` | PNG of a page, rendered by the engine: written to `path`, or without one returned as base64 `png` in the reply (never a fixed name in the shared temporary folder) | | `ui.parity` | — | feature-catalog parity | | `ui.resize` | `width`, `height` | asks for that window content size in points; sizes that aren't positive and finite are refused, the rest are clamped to the app's minimum (760 × 480) and to the largest surface the GPU can draw (at most 16384; that limit wins over the minimum), and the reply's `width` and `height` are the size asked for | @@ -131,7 +133,7 @@ window); in the browser, the one the window shows. Commands work on it unless to (`"alreadyOpen": true`). Its result carries the document's `id`. - `view.switchWindows` lists the open documents (`{"documents": [{"id", "title", "path", "dirty", "active"}]}`); with `{"id": n}` it makes that document active and brings its window to - the front. + the front. With `{"window": key}` it selects that window directly and brings it to the front. - `file.closeDocument {"id"?, "force"?}` closes a document; its window closes with it unless it is the last window. - `engine.execute`, `document.inspect` and `ui.render` take `doc`, the id of the document to work @@ -154,6 +156,14 @@ Input and window methods go to the active window, the one focused last: `ui.clic `ui.screenshot` (a PNG of that window), `ui.inspect` (its size), `ui.resize` and `ui.focus`. To work in another window, make its document active first with `view.switchWindows {"id": n}`. +`ui.screenshot` is the exception when multiple document windows are open: the desktop renderer +captures only the window in the root viewport (the first window opened, until that window closes and +another is adopted in its place). While the active window is a secondary document window in a separate +native viewport, `ui.screenshot` returns an error directing the caller to select the root window +instead of hanging for 8 seconds and timing out. Select the root window with +`view.switchWindows {"window": root}` and request the capture again. The error gives the root +window key when it is available. + The `CharProps` and `ParaProps` that `format.set`, `para.set` and `styles.modify` take are listed in [MCP server](mcp.md#character-and-paragraph-properties).