Merge pull request #1950 from Silvertree2010/fix/h9583-format-paragraph-silently-left-a

Fix format.paragraph silently left-aligning known align names

Cargo.lock

@@ -935,10 +935,21 @@ dependencies = [
  "zip 9.1.0",
 ]
 
+[[package]]
+name = "craft-web-api"
+version = "0.1.0"
+source = "git+https://github.com/storytold/craft-libs?rev=fd5e2feff6ea25bed371a99cc732b3968d621505#fd5e2feff6ea25bed371a99cc732b3968d621505"
+dependencies = [
+ "js-sys",
+ "serde_json",
+ "wasm-bindgen",
+ "wasm-bindgen-futures",
+]
+
 [[package]]
 name = "craft-web-host"
 version = "0.1.0"
-source = "git+https://github.com/storytold/craft-libs?rev=2e2312ddaf21bc3d85cd3395d0955c7d239306f6#2e2312ddaf21bc3d85cd3395d0955c7d239306f6"
+source = "git+https://github.com/storytold/craft-libs?rev=fd5e2feff6ea25bed371a99cc732b3968d621505#fd5e2feff6ea25bed371a99cc732b3968d621505"
 dependencies = [
  "wasm-bindgen",
  "web-sys",
@@ -1331,6 +1342,7 @@ dependencies = [
 name = "deckcraft-web"
 version = "0.15.0"
 dependencies = [
+ "craft-web-api",
  "craft-web-host",
  "deckcraft-engine",
  "deckcraft-ui-egui",

apps/deckcraft-web/Cargo.toml

@@ -16,8 +16,10 @@ path = "src/main.rs"
 [dependencies]
 deckcraft-engine = { workspace = true }
 deckcraft-ui-egui = { workspace = true }
+# The page command API (storytold/craft-libs#413): window.deckcraft over the control channel.
+craft-web-api = { git = "https://github.com/storytold/craft-libs", rev = "fd5e2feff6ea25bed371a99cc732b3968d621505", version = "0.1.0" }
 # The host messages (storytold/craft-libs): one contract and one bridge for the web builds.
-craft-web-host = { git = "https://github.com/storytold/craft-libs", rev = "2e2312ddaf21bc3d85cd3395d0955c7d239306f6", version = "0.1.0" }
+craft-web-host = { git = "https://github.com/storytold/craft-libs", rev = "fd5e2feff6ea25bed371a99cc732b3968d621505", version = "0.1.0" }
 egui = { workspace = true }
 log = { workspace = true }
 serde_json = { workspace = true }

apps/deckcraft-web/src/api.rs

@@ -0,0 +1,151 @@
+//! `window.deckcraft`: the page command API (`craft-web-api`, storytold/craft-libs#413) over the
+//! app's own control channel, the one the desktop control server drives (`docs/control-protocol.md`).
+//!
+//! The crate owns the JavaScript side (`request`, `execute`, `commands`, `docInspect`, `uiInspect`,
+//! `screenshot`, as promises with a timeout). This module is DeckCraft's part: a [`Request`] that
+//! queues a [`ControlRequest`] for the app, which answers it on its next frame, and waits for the
+//! reply. A call the page gave up on is released ([`Pending`]), so the app skips it if it has not started it, instead of
+//! applying a command the page was told had failed.
+
+use std::sync::Arc;
+use std::sync::atomic::AtomicU8;
+use std::sync::mpsc::{Receiver, TryRecvError};
+
+use deckcraft_ui_egui::ControlRequest;
+use serde_json::{Value, json};
+
+/// How often a waiting call looks for the app's reply, in milliseconds (about a frame).
+#[cfg(target_arch = "wasm32")]
+const POLL_MS: i32 = 16;
+
+/// A queued control request whose reply has not arrived. Dropping it before the reply (the page's
+/// timeout fired, or the call was abandoned) releases the request: the app skips it if it has not started it (one it has taken still finishes).
+struct Pending {
+    reply: Receiver<Value>,
+    state: Arc<AtomicU8>,
+    answered: bool,
+}
+
+impl Pending {
+    /// `Some` once the app has answered or gone away; `None` while the request is still waiting.
+    fn poll(&mut self) -> Option<Value> {
+        match self.reply.try_recv() {
+            Ok(reply) => {
+                self.answered = true;
+                Some(reply)
+            }
+            Err(TryRecvError::Empty) => None,
+            Err(TryRecvError::Disconnected) => {
+                self.answered = true;
+                Some(json!({"ok": false, "error": "the app dropped the request"}))
+            }
+        }
+    }
+}
+
+impl Drop for Pending {
+    fn drop(&mut self) {
+        if !self.answered {
+            ControlRequest::abandon(&self.state);
+        }
+    }
+}
+
+#[cfg(target_arch = "wasm32")]
+pub use web::install;
+
+#[cfg(target_arch = "wasm32")]
+mod web {
+    use std::rc::Rc;
+    use std::sync::mpsc::channel;
+
+    use craft_web_api::methods::Methods;
+    use craft_web_api::page::{PageApi, Request};
+    use deckcraft_ui_egui::{ControlRequest, SlideApp};
+    use serde_json::json;
+    use wasm_bindgen_futures::JsFuture;
+
+    use super::{POLL_MS, Pending};
+
+    /// The app id in `window.<app>`.
+    const APP: &str = "deckcraft";
+
+    /// Give the app its control queue and put `window.deckcraft` on the page. An existing
+    /// `window.deckcraft` keeps the functions it already has.
+    pub fn install(app: &mut SlideApp, ctx: egui::Context) {
+        let (queue, requests) = channel::<ControlRequest>();
+        app.control_rx = Some(requests);
+        let request: Request = Rc::new(move |method, params| {
+            let (control, reply) = ControlRequest::new(method, params);
+            let mut pending = Pending { reply, state: control.state.clone(), answered: false };
+            if queue.send(control).is_err() {
+                pending.answered = true;
+                return Box::pin(async { json!({"ok": false, "error": "the app is not running"}) });
+            }
+            ctx.request_repaint();
+            Box::pin(async move {
+                loop {
+                    if let Some(reply) = pending.poll() {
+                        return reply;
+                    }
+                    sleep(POLL_MS).await;
+                }
+            })
+        });
+        if let Err(e) = Rc::new(PageApi::new(APP, Methods::default(), request)).install() {
+            log::warn!("couldn't install window.{APP}: {e:?}");
+        }
+    }
+
+    async fn sleep(ms: i32) {
+        let promise = js_sys::Promise::new(&mut |resolve, _| {
+            if let Some(window) = web_sys::window() {
+                let _ = window.set_timeout_with_callback_and_timeout_and_arguments_0(&resolve, ms);
+            }
+        });
+        let _ = JsFuture::from(promise).await;
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::sync::atomic::Ordering;
+
+    use super::*;
+
+    /// `ControlRequest`'s `state` values (`deckcraft_ui_egui::control`): the app has started it, or its
+    /// sender gave up.
+    const TAKEN: u8 = 1;
+    const ABANDONED: u8 = 2;
+
+    fn pending() -> (Pending, ControlRequest) {
+        let (control, reply) = ControlRequest::new("document.inspect", json!({}));
+        (Pending { reply, state: control.state.clone(), answered: false }, control)
+    }
+
+    #[test]
+    fn a_call_the_page_gave_up_on_is_released_so_the_app_skips_it() {
+        let (waiting, control) = pending();
+        drop(waiting);
+        assert_eq!(control.state.load(Ordering::SeqCst), ABANDONED);
+        assert!(!control.take(), "the app must not run a request the page was told had failed");
+    }
+
+    #[test]
+    fn an_answered_call_is_not_released() {
+        let (mut waiting, control) = pending();
+        assert!(control.take());
+        control.reply.send(json!({"ok": true, "result": 1})).unwrap_or_default();
+        assert_eq!(waiting.poll(), Some(json!({"ok": true, "result": 1})));
+        drop(waiting);
+        assert_eq!(control.state.load(Ordering::SeqCst), TAKEN);
+    }
+
+    #[test]
+    fn a_request_the_app_never_runs_waits_until_it_is_dropped() {
+        let (mut waiting, control) = pending();
+        assert_eq!(waiting.poll(), None);
+        drop(control);
+        assert_eq!(waiting.poll().and_then(|r| r["ok"].as_bool()), Some(false), "the app went away");
+    }
+}

apps/deckcraft-web/src/main.rs

@@ -21,6 +21,8 @@
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 #![forbid(unsafe_code)]
 
+#[cfg(any(target_arch = "wasm32", test))]
+mod api;
 #[cfg(target_arch = "wasm32")]
 mod host;
 #[cfg(target_arch = "wasm32")]

apps/deckcraft-web/src/web.rs

@@ -79,6 +79,7 @@ pub fn start() {
                     {
                         crate::host::Bridge::set_author(&mut app, &author);
                     }
+                    crate::api::install(&mut app, cc.egui_ctx.clone());
                     Ok(Box::new(WebShell { app, inbox, bridge: app_bridge }))
                 }),
             )

crates/engine/src/cmd/format/paragraph.rs

@@ -120,13 +120,7 @@ pub(super) fn line_spacing(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 pub(super) fn paragraph(s: &mut Session, p: &Value) -> Result<Value> {
-    let a = str_param(p, "align").map(|a| match a {
-        "center" => Align::Center,
-        "right" => Align::Right,
-        "justify" => Align::Justify,
-        "distributed" => Align::Distributed,
-        _ => Align::Left,
-    });
+    let a = str_param(p, "align").map(|a| parse_align("format.paragraph", a)).transpose()?;
     let ml = f64_param(p, "indentLeft");
     let ind = f64_param(p, "indentFirst");
     let sb = f64_param(p, "spaceBefore");
@@ -182,10 +176,10 @@ pub(super) fn clamp_pt(v: f64) -> f64 {
 fn parse_align(cmd: &str, s: &str) -> Result<Align> {
     Ok(match s {
         "left" | "l" => Align::Left,
-        "center" | "ctr" => Align::Center,
+        "center" | "centre" | "ctr" => Align::Center,
         "right" | "r" => Align::Right,
         "justify" | "just" => Align::Justify,
-        "justLow" => Align::JustLow,
+        "justLow" | "justifyLow" => Align::JustLow,
         "distributed" | "dist" => Align::Distributed,
         "thaiDist" => Align::ThaiDistributed,
         _ => return Err(bad(cmd, "`align` must be left, center, right, justify, justLow, distributed or thaiDist")),

crates/engine/src/cmd/shape.rs

@@ -10,7 +10,7 @@ use serde_json::{Value, json};
 
 use super::{
     CommandSpec, NO_ID_SPACE, bad, bool_or, bool_param, cmd, color_param, color_value, edit_geometry, edit_shapes, f64_param, has_selection,
-    has_slide, id_param, ok, str_param, targets, usize_param, with_param, xfrm_of,
+    has_slide, id_param, ok, opt_usize_param, str_param, targets, usize_param, with_param, xfrm_of,
 };
 use crate::{Result, Session};
 

crates/engine/src/cmd/shape/appearance.rs

@@ -1,11 +1,11 @@
 use super::{
     ColorRef, ColorTransform, Dash, Effects, Fill, Geom, Glow, Gradient, GradientShape, GradientStop, Line, LineEnd, PictureFill, PictureMode,
     Reflection, Result, Rgba, SchemeSlot, Session, Shadow, ShapeStyle, Value, apply_fill, bad, bool_or, color_param, color_value, edit_geometry,
-    edit_shapes, f64_param, inherited_fills, json, requested_fill, str_param, targets, usize_param,
+    edit_shapes, f64_param, inherited_fills, json, opt_usize_param, requested_fill, str_param, targets,
 };
 
 pub(super) fn adjust(s: &mut Session, p: &Value) -> Result<Value> {
-    let i = usize_param(p, "index").unwrap_or(0).min(16);
+    let i = opt_usize_param(p, "index", "shape.adjust")?.unwrap_or(0).min(16);
     let v = f64_param(p, "value").ok_or_else(|| bad("shape.adjust", "missing `value`"))?;
     edit_geometry(s, p, "shape.adjust", |sh| {
         if let Geom::Preset { name, adj } = &mut sh.geom {
@@ -556,7 +556,7 @@ pub fn quick_style_for(index: usize) -> (ShapeStyle, Option<Fill>, Option<Line>)
 }
 
 pub(super) fn quick_style(s: &mut Session, p: &Value) -> Result<Value> {
-    let i = usize_param(p, "index").unwrap_or(1).min(41);
+    let i = opt_usize_param(p, "index", "shape.quickStyle")?.unwrap_or(1).min(41);
     let (st, f, l) = quick_style_for(i);
     edit_shapes(s, p, "shape.quickStyle", |sh| {
         sh.style = Some(st.clone());

crates/engine/src/tests.rs

@@ -1713,6 +1713,31 @@ fn tab_moves_between_cells_skipping_merged_ones() {
     assert_eq!((cell(&s), table_of(&s, id).n_rows()), (Some((2, 0)), 3), "Tab in the last cell adds a row");
 }
 
+#[test]
+fn format_paragraph_align_names_match_format_align() {
+    use deckcraft_model::text::Align;
+    let mut s = session();
+    let title = s.doc().unwrap().current_slide().unwrap().shapes[0].id.0;
+    s.execute("text.edit", &json!({"id": title})).unwrap();
+    let para = |s: &Session| s.doc().unwrap().current_slide().unwrap().shapes[0].text.as_ref().unwrap().paragraphs[0].props.align;
+    for (name, want) in [
+        ("justLow", Align::JustLow),
+        ("justifyLow", Align::JustLow),
+        ("thaiDist", Align::ThaiDistributed),
+        ("ctr", Align::Center),
+        ("centre", Align::Center),
+        ("r", Align::Right),
+        ("just", Align::Justify),
+        ("dist", Align::Distributed),
+        ("distributed", Align::Distributed),
+        ("left", Align::Left),
+    ] {
+        s.execute("format.paragraph", &json!({"align": name})).unwrap();
+        assert_eq!(para(&s), Some(want), "{name}");
+    }
+    assert!(s.execute("format.paragraph", &json!({"align": "bogus"})).is_err());
+}
+
 #[test]
 fn right_to_left_paragraphs() {
     let mut s = session();
@@ -7029,3 +7054,18 @@ fn outline_export_skips_footer_date_and_slide_number() {
     assert!(rtf.contains("Agenda") && rtf.contains("First point"), "{rtf:?}");
     assert!(!rtf.contains("Company Confidential") && !rtf.contains("1 May 2026"), "{rtf:?}");
 }
+
+#[test]
+fn shape_adjust_and_quick_style_reject_invalid_index() {
+    let mut s = session();
+    let r = s.execute("shape.insert", &json!({"preset": "roundRect", "rect": [100, 100, 200, 100]})).unwrap();
+    let id = deckcraft_model::ShapeId(r["id"].as_u64().unwrap() as u32);
+    let before = s.doc().unwrap().shape(id).unwrap().clone();
+    for bad in [json!(-1), json!(1.5)] {
+        assert!(s.execute("shape.adjust", &json!({"index": bad, "value": 0.4})).is_err(), "adjust accepted {bad}");
+        assert!(s.execute("shape.quickStyle", &json!({"index": bad})).is_err(), "quickStyle accepted {bad}");
+    }
+    assert_eq!(s.doc().unwrap().shape(id).unwrap().geom, before.geom);
+    assert_eq!(s.doc().unwrap().shape(id).unwrap().style, before.style);
+    s.execute("shape.adjust", &json!({"value": 0.4})).unwrap();
+}

packaging/web/README.md

@@ -136,6 +136,18 @@ beside it. The page guards leaving itself (DeckCraft adds no guard of its own),
 
 The messages and both transports are the shared contract of [`craft-web-host`](https://github.com/storytold/craft-libs/tree/main/crates/web-host) (storytold/craft-libs#420), the same for every Craft app: its README lists every message, including `saved`, with the compatibility notes. In `deckcraft:ready`, `version` is the contract's (`"1"`) and `appVersion` is this app's release. DeckCraft reads bytes, so a host sends `bytes`, not a `File`.
 
+### Driving it from the page (`window.deckcraft`)
+
+Every DeckCraft web build puts `window.deckcraft` on the page, from [`craft-web-api`](https://github.com/storytold/craft-libs/tree/main/crates/web-api) (storytold/craft-libs#413), the same shape in every Craft app. `deckcraft.request(method, params)` calls any method of the control channel the desktop app serves (`docs/control-protocol.md`), and `execute(command, params)`, `commands()`, `docInspect(params)`, `uiInspect(params)` and `screenshot(params)` are shortcuts for the common ones. Each returns a Promise that resolves with the method's result and rejects with an `Error`; a call the app does not answer within 30 seconds rejects.
+
+```js
+const deck = await deckcraft.docInspect();                 // a summary of the open presentation
+await deckcraft.execute("shape.insert", { preset: "roundRect", rect: [100, 100, 240, 120], text: "Hello" });
+const { data } = await deckcraft.screenshot();            // the canvas as PNG (base64)
+```
+
+The API is there in every mode, with or without a host. Opening a file, handing a saved or exported file to the page, the file name and the unsaved flag stay with the host messages above.
+
 ## Renderer selection and fallback flags
 
 DeckCraft renders with wgpu. It uses **WebGPU** when the browser has it and falls back to