UI: password dialog fixes from review (#11)

OK on the untouched field says the password is incorrect instead of
asking again; the Backstage closes once the workbook opens; app.open on
the control channel takes a password and never opens the dialog. Control
protocol docs cover file.open {password}.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/ui-egui/src/control.rs

@@ -14,7 +14,7 @@
 //! - `ui.edit.begin {text?}`, `ui.edit.commit {move?}`, `ui.edit.cancel`
 //! - `ui.screenshot {path?}` — PNG of the window (base64 when no path); animations are settled
 //! - `ui.resize {width, height}`, `ui.set {dark?, formulaBar?, ribbonCollapsed?, animations?}`
-//! - `app.open {path}`, `app.save {path?}`, `app.quit`
+//! - `app.open {path, password?}`, `app.save {path?}`, `app.quit`
 
 use std::sync::mpsc::Sender;
 
@@ -351,10 +351,17 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) -
         "ui.screenshot" => return Outcome::Screenshot { path: s("path").map(str::to_string) },
         "app.open" => {
             let Some(path) = s("path") else { return err("missing `path`") };
-            app.open_path(path);
-            match app.message.take() {
-                Some((_, m)) => err(m),
-                None => ok(json!({"title": app.session.active().map(|d| d.display_title())})),
+            // Not `open_path`: that asks for a missing password in a dialog, and programmatic
+            // calls never open one. A password-protected workbook takes `password` instead.
+            let r = app.run("file.open", json!({"path": path, "password": s("password")}));
+            if let Ok(r) = &r {
+                app.after_open(Some(path), r);
+            }
+            app.after_engine();
+            match (r, app.message.take()) {
+                (Err(e), _) => err(crate::clean_error(&e)),
+                (Ok(_), Some((_, m))) => err(m),
+                (Ok(_), None) => ok(json!({"title": app.session.active().map(|d| d.display_title())})),
             }
         }
         "app.save" => wrap(app.run("file.save", json!({"path": s("path")}))),

crates/ui-egui/src/dialogs.rs

@@ -241,12 +241,16 @@ impl Dialog {
                 // on the web) and goes back to `file.open` with the password.
                 let source = p("path").as_str().or(p("name").as_str()).map(str::to_string).unwrap_or_default();
                 let file = std::path::Path::new(&source).file_name().and_then(|n| n.to_str()).unwrap_or("This workbook").to_string();
+                // An empty password from the start, so OK on the untouched field says "incorrect"
+                // rather than asking for the password again.
+                let mut values = params.clone();
+                values["password"] = json!("");
                 Dialog::form(
                     "openPassword",
                     "Password",
                     "file.open",
                     vec![Note(format!("'{file}' is protected.")), Password { key: "password", label: "Password:" }],
-                    params.clone(),
+                    values,
                 )
             }
             "unprotectSheet" => Dialog::form(

crates/ui-egui/src/lib.rs

@@ -632,9 +632,11 @@ impl SheetApp {
         self.after_engine();
     }
 
-    /// What follows a successful `file.open`: the Recent list (for a file on disk) and the open
-    /// warnings notice. Shared by File › Open and the password dialog.
+    /// What follows a successful `file.open`: the workbook shows (not the Backstage), the Recent
+    /// list (for a file on disk) and the open warnings notice. Shared by File › Open, the
+    /// password dialog and the control channel.
     pub fn after_open(&mut self, path: Option<&str>, r: &Json) {
+        self.backstage = None;
         if let Some(path) = path {
             self.ui.recent.retain(|p| p != path);
             self.ui.recent.insert(0, path.to_string());

crates/ui-egui/tests/open_encrypted.rs

@@ -118,3 +118,55 @@ fn a_file_picked_on_the_web_asks_for_its_password_too() {
     assert_eq!(a1(&h), Value::Number(42.0));
     assert_eq!(h.state().session.active().unwrap().path, None);
 }
+
+#[test]
+fn an_untouched_password_field_is_incorrect_not_required() {
+    let path = encrypted_file("untouched");
+    let mut h = harness(Services::default());
+    h.state_mut().open_path(&path.to_string_lossy());
+    h.run_steps(3);
+    h.get_by_label("   OK   ").click();
+    h.run_steps(3);
+    let msg = h.state().message.as_ref().map(|(_, m)| m.clone()).unwrap_or_default();
+    assert!(msg.contains(gridcraft_engine::io::WRONG_PASSWORD), "{msg}");
+    assert_eq!(dialog(&h).as_deref(), Some("openPassword"));
+    std::fs::remove_dir_all(path.parent().unwrap()).unwrap();
+}
+
+#[test]
+fn the_backstage_closes_once_the_password_opens_the_workbook() {
+    let path = encrypted_file("backstage");
+    let mut h = harness(Services::default());
+    h.state_mut().backstage = Some(gridcraft_ui_egui::backstage::Backstage::new(gridcraft_ui_egui::backstage::Page::Open));
+    h.state_mut().open_path(&path.to_string_lossy());
+    h.run_steps(3);
+    h.state_mut().dialog.as_mut().unwrap().values.insert("password".into(), json!(PW));
+    h.get_by_label("   OK   ").click();
+    h.run_steps(3);
+    assert_eq!(a1(&h), Value::Number(42.0));
+    assert!(h.state().backstage.is_none(), "the opened workbook shows, not the Backstage");
+    std::fs::remove_dir_all(path.parent().unwrap()).unwrap();
+}
+
+#[test]
+fn app_open_over_the_control_channel_never_opens_a_dialog() {
+    use gridcraft_ui_egui::control::{Outcome, handle};
+    let path = encrypted_file("control");
+    let p = path.to_string_lossy().to_string();
+    let mut h = harness(Services::default());
+    let ctx = h.ctx.clone();
+    let reply = |o: Outcome| match o {
+        Outcome::Done(v) => v,
+        Outcome::Screenshot { .. } => json!(null),
+    };
+    let r = reply(handle(h.state_mut(), &ctx, "app.open", &json!({"path": p})));
+    assert_eq!(r["ok"], json!(false), "{r}");
+    assert!(r["error"].as_str().unwrap_or_default().contains(gridcraft_engine::io::PASSWORD_REQUIRED), "{r}");
+    assert_eq!(dialog(&h), None, "programmatic calls never open dialogs");
+    let r = reply(handle(h.state_mut(), &ctx, "app.open", &json!({"path": p, "password": PW})));
+    assert_eq!(r["ok"], json!(true), "{r}");
+    assert_eq!(r["result"]["title"], json!("Secret.xlsx"));
+    assert_eq!(a1(&h), Value::Number(42.0));
+    assert_eq!(h.state().ui.recent.first(), Some(&p));
+    std::fs::remove_dir_all(path.parent().unwrap()).unwrap();
+}

docs/control-protocol.md

@@ -36,7 +36,7 @@ Errors come back as `{"id": …, "ok": false, "error": "…"}`.
 | `ui.screenshot` | `{path?}` | PNG of the window (base64 when no path). Animations are settled first: the picture shows the final state |
 | `ui.set` | `{dark?, formulaBar?, ribbonCollapsed?, animations?}` | UI preferences (`formulaBar` runs `view.formulaBar`) |
 | `ui.resize` / `ui.focus` | `{width, height}` | Window control |
-| `app.open` / `app.save` / `app.quit` | `{path}` / `{path?}` | Files and lifetime |
+| `app.open` / `app.save` / `app.quit` | `{path, password?}` / `{path?}` | Files and lifetime. `password` opens a password-protected workbook; without it, such a workbook returns an error (never a dialog) |
 
 Any other method name that is a command id (e.g. `home.bold`) runs that command.
 
@@ -56,5 +56,12 @@ the View ▸ Animations checkbox, saved with the other UI preferences and on by
 document text. The choice is saved in `ui.json`; with nothing saved the desktop app follows the
 system language.
 
+`{command: "file.open", params: {path: "Secret.xlsx", password: "…"}}` opens a password-protected
+workbook (Agile or Standard encryption). Without `password` it returns an error containing
+"is password-protected. Enter its password to open it." and never opens a dialog; a wrong
+password returns "The password is incorrect." The password is left out of the journal and never
+quoted in an error. The opened workbook has no save path, so Save asks where to save it, without
+the password, and never replaces the encrypted original.
+
 For a headless equivalent (no window), use `gridcraft-cli mcp` or `gridcraft-cli run`;
 see [`mcp.md`](mcp.md) and [`cli.md`](cli.md).