Fix #1693: a browser download that can't be started fails the save it is for

The web app's download callback returned nothing, so a save whose Blob, URL or
anchor couldn't be made was reported as written: the document counted as
saved, its recovery copy was dropped, and Save Changes closed it. The callback
now returns the error, which fails the save, export or file command like a
refused write on the desktop. A timer that can't be set after the download
started is not a failure.

apps/vectorcraft-web/src/web.rs

@@ -434,11 +434,8 @@ fn services(inbox: Inbox, place_inbox: PlaceInbox, ctx: egui::Context, locks: Op
                 ctx.request_repaint();
             });
         })),
-        download: Some(Box::new(|name: &str, bytes: &[u8]| {
-            if let Err(e) = download(name, bytes) {
-                log::error!("download of {name} failed: {e}");
-            }
-        })),
+        // A download that couldn't be started fails the save or export it is for (#1693).
+        download: Some(Box::new(|name: &str, bytes: &[u8]| download(name, bytes).inspect_err(|e| log::error!("download of {name} failed: {e}")))),
         inbox: Some(inbox),
         recovery_store: Some(Arc::new(BrowserStore { locks })),
         // File → Print: the browser's print dialog.
@@ -517,7 +514,8 @@ fn download(path: &str, bytes: &[u8]) -> Result<(), String> {
     let revoke = wasm_bindgen::closure::Closure::once_into_js(move || {
         web_sys::Url::revoke_object_url(&url).ok();
     });
-    window.set_timeout_with_callback_and_timeout_and_arguments_0(revoke.unchecked_ref(), 10_000).map_err(js)?;
+    // The download has started: a timer that can't be set leaves the URL allocated, no more.
+    let _ = window.set_timeout_with_callback_and_timeout_and_arguments_0(revoke.unchecked_ref(), 10_000);
     Ok(())
 }
 

crates/ui-egui/src/dialogs/save_options.rs

@@ -268,7 +268,10 @@ mod tests {
         let written = Written::default();
         let w = written.clone();
         let services = Services {
-            download: Some(Box::new(move |name: &str, b: &[u8]| w.borrow_mut().push((name.to_string(), b.to_vec())))),
+            download: Some(Box::new(move |name: &str, b: &[u8]| {
+                w.borrow_mut().push((name.to_string(), b.to_vec()));
+                Ok(())
+            })),
             ..Default::default()
         };
         (with_art(services), written)

crates/ui-egui/src/dialogs/tests_package.rs

@@ -95,7 +95,10 @@ fn the_web_downloads_a_zip() {
     let mut app = saved(&dir);
     let got: Rc<RefCell<Vec<(String, usize)>>> = Rc::default();
     let g = got.clone();
-    app.services.download = Some(Box::new(move |name: &str, b: &[u8]| g.borrow_mut().push((name.to_string(), b.len()))));
+    app.services.download = Some(Box::new(move |name: &str, b: &[u8]| {
+        g.borrow_mut().push((name.to_string(), b.len()));
+        Ok(())
+    }));
     app.run("ui.packageDialog", json!({})).unwrap();
     assert!(!frame(&mut app).contains("Location:"), "no folder to choose");
     confirm(&mut app).unwrap();
@@ -124,7 +127,7 @@ fn fonts_left_out_of_the_package_are_named() {
     assert!(detail.contains(gone) && !detail.contains("Source Sans 3"), "{detail}");
     // The web's download says so too.
     app.ui.dialog = None;
-    app.services.download = Some(Box::new(|_: &str, _: &[u8]| {}));
+    app.services.download = Some(Box::new(|_: &str, _: &[u8]| Ok(())));
     app.run("ui.packageDialog", json!({})).unwrap();
     confirm(&mut app).unwrap();
     assert!(app.ui.status.contains(gone), "{}", app.ui.status);

crates/ui-egui/src/dialogs/tests_save_for_web.rs

@@ -24,7 +24,10 @@ fn app(dir: &std::path::Path, web: bool) -> (VectorcraftApp, Log) {
         ..Default::default()
     };
     if web {
-        services.download = Some(Box::new(move |name: &str, b: &[u8]| l2.borrow_mut().push(format!("download {name} {}", b.len()))));
+        services.download = Some(Box::new(move |name: &str, b: &[u8]| {
+            l2.borrow_mut().push(format!("download {name} {}", b.len()));
+            Ok(())
+        }));
     }
     let mut app = VectorcraftApp::new(Session::new(), services);
     app.run("file.new", json!({"width": 60, "height": 40})).unwrap();

crates/ui-egui/src/dialogs/tests_screen_assets.rs

@@ -21,7 +21,10 @@ fn app(web: bool) -> (VectorcraftApp, Log, [u64; 2]) {
     let l = log.clone();
     let mut services = Services::default();
     if web {
-        services.download = Some(Box::new(move |name: &str, bytes: &[u8]| l.borrow_mut().push((name.to_string(), bytes.to_vec()))));
+        services.download = Some(Box::new(move |name: &str, bytes: &[u8]| {
+            l.borrow_mut().push((name.to_string(), bytes.to_vec()));
+            Ok(())
+        }));
     } else {
         services.reveal = Some(Box::new(move |path: &str| {
             l.borrow_mut().push((path.to_string(), vec![]));

crates/ui-egui/src/dialogs/tests_screens.rs

@@ -21,7 +21,10 @@ fn app(artboards: usize, web: bool) -> (VectorcraftApp, Log) {
     let l = log.clone();
     let mut services = Services::default();
     if web {
-        services.download = Some(Box::new(move |name: &str, bytes: &[u8]| l.borrow_mut().push((name.to_string(), bytes.to_vec()))));
+        services.download = Some(Box::new(move |name: &str, bytes: &[u8]| {
+            l.borrow_mut().push((name.to_string(), bytes.to_vec()));
+            Ok(())
+        }));
     } else {
         services.reveal = Some(Box::new(move |path: &str| {
             l.borrow_mut().push((path.to_string(), vec![]));

crates/ui-egui/src/io.rs

@@ -239,8 +239,7 @@ pub fn new_from_template(app: &mut VectorcraftApp, path: Option<String>) -> Resu
 /// Web: download `bytes` under `path`'s file name; desktop: write them to `path`.
 pub(crate) fn write_to(services: &mut Services, path: &str, bytes: &[u8]) -> Result<(), String> {
     if let Some(dl) = services.download.as_mut() {
-        dl(&fileio::file_name(path), bytes);
-        return Ok(());
+        return dl(&fileio::file_name(path), bytes);
     }
     services.write.as_mut().ok_or("no file writer")?(path, bytes)
 }
@@ -768,7 +767,7 @@ pub(crate) fn export_files(app: &mut VectorcraftApp, id: &str, params: Value) ->
     } else if let Some(download) = app.services.download.as_mut() {
         for (name, data) in &returned {
             let bytes = vectorcraft_format::base64_decode(data).ok_or("the export returned unreadable data")?;
-            download(name, &bytes);
+            download(name, &bytes)?;
         }
         let what = match returned.as_slice() {
             [(name, _)] => (*name).to_string(),
@@ -830,6 +829,35 @@ mod tests {
         assert!(app.ui.dialog.is_none());
     }
 
+    /// #1693: a browser download that can't be started is a failed save: the document stays
+    /// modified and without a file, and Save Changes doesn't close it.
+    #[test]
+    fn a_download_that_cannot_start_leaves_the_document_unsaved_and_open() {
+        let started = Rc::new(RefCell::new(false));
+        let ok = started.clone();
+        let download: crate::DownloadFn = Box::new(move |_: &str, _: &[u8]| if *ok.borrow() { Ok(()) } else { Err("the browser refused the download".into()) });
+        let mut app = VectorcraftApp::new(Session::new(), Services { download: Some(download), ..Default::default() });
+        app.run("file.new", json!({"width": 100, "height": 80})).unwrap();
+        app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 30, "height": 20})).unwrap();
+        let unsaved = |app: &VectorcraftApp| {
+            let st = app.session.doc().unwrap();
+            (st.is_dirty(), st.path.clone())
+        };
+        let e = save(&mut app, SaveMode::Save, &json!({"path": "Poster.vectorcraft"}), false).unwrap_err();
+        assert!(e.contains("the browser refused the download"), "{e}");
+        assert_eq!(unsaved(&app), (true, None));
+        // Save Changes › Save: not saved, so not closed.
+        let (i, uid) = (app.session.active_index().unwrap(), app.session.doc().unwrap().uid);
+        app.ui.dialog = Some(crate::state::Dialog::new(crate::unsaved::KIND, json!({"index": i, "uid": uid, "name": "Poster", "then": "close"})));
+        app.session.doc_mut().unwrap().path = Some("Poster.vectorcraft".into());
+        assert!(crate::unsaved::confirm(&mut app).is_err());
+        assert_eq!((app.session.documents().len(), app.session.doc().unwrap().is_dirty()), (1, true));
+        // Once the download starts, the save counts.
+        *started.borrow_mut() = true;
+        save(&mut app, SaveMode::Save, &json!({}), false).unwrap();
+        assert!(!app.session.doc().unwrap().is_dirty());
+    }
+
     fn bytes_of(app: &mut VectorcraftApp, cmd: &str, p: Value) -> Vec<u8> {
         let v = app.session.execute(cmd, &p).unwrap();
         vectorcraft_format::base64_decode(v["dataBase64"].as_str().unwrap()).unwrap()

crates/ui-egui/src/lib.rs

@@ -188,7 +188,9 @@ pub type PickSave = Box<dyn FnMut(&FilePick) -> Option<String>>;
 pub type ReadFn = Box<dyn Fn(&str) -> Result<Vec<u8>, String>>;
 pub type WriteFn = Box<dyn FnMut(&str, &[u8]) -> Result<(), String>>;
 pub type Inbox = Arc<Mutex<Vec<(String, Vec<u8>)>>>;
-pub type DownloadFn = Box<dyn FnMut(&str, &[u8])>;
+/// Starts a browser download of the bytes under a file name; an error when it couldn't be started
+/// (the save or export then fails as a refused write does: #1693).
+pub type DownloadFn = Box<dyn FnMut(&str, &[u8]) -> Result<(), String>>;
 
 /// Opens a URL in the system browser.
 pub type OpenUrlFn = Box<dyn FnMut(&str)>;

crates/ui-egui/src/panels/library_panel.rs

@@ -646,8 +646,13 @@ mod tests {
         // The web downloads the file under that name.
         let downloads = std::rc::Rc::new(std::cell::RefCell::new(vec![]));
         let dl = downloads.clone();
-        let services =
-            crate::Services { download: Some(Box::new(move |name: &str, _: &[u8]| dl.borrow_mut().push(name.to_string()))), ..Default::default() };
+        let services = crate::Services {
+            download: Some(Box::new(move |name: &str, _: &[u8]| {
+                dl.borrow_mut().push(name.to_string());
+                Ok(())
+            })),
+            ..Default::default()
+        };
         let mut web = VectorcraftApp::new(Session::new(), services);
         web.run("file.new", json!({"width": 100, "height": 100})).unwrap();
         web.run("ui.saveSwatchLibrary", json!({})).unwrap();

crates/ui-egui/src/panels/links.rs

@@ -155,11 +155,12 @@ pub(crate) fn go_to(app: &mut VectorcraftApp, id: u64) {
 
 /// Write embedded image `id` to a picked file (the web downloads it) and link it there.
 pub(crate) fn unembed(app: &mut VectorcraftApp, id: u64, name: &str) {
-    if let Some(dl) = app.services.download.as_mut() {
+    if app.services.download.is_some() {
         if let Ok(v) = app.session.execute("links.unembed", &json!({ "id": id }))
             && let (Some(name), Some(bytes)) = (v["name"].as_str(), v["dataBase64"].as_str().and_then(vectorcraft_format::base64_decode))
+            && let Err(e) = crate::io::write_to(&mut app.services, name, &bytes)
         {
-            dl(name, &bytes);
+            app.status(e);
         }
         return;
     }

crates/ui-egui/src/panels/tests_asset_export.rs

@@ -21,7 +21,10 @@ fn app(web: bool) -> (VectorcraftApp, Log, [u64; 2]) {
     let l = log.clone();
     let mut services = Services::default();
     if web {
-        services.download = Some(Box::new(move |name: &str, bytes: &[u8]| l.borrow_mut().push((name.to_string(), bytes.to_vec()))));
+        services.download = Some(Box::new(move |name: &str, bytes: &[u8]| {
+            l.borrow_mut().push((name.to_string(), bytes.to_vec()));
+            Ok(())
+        }));
     }
     let mut app = VectorcraftApp::new(Session::new(), services);
     app.run("file.new", json!({"width": 200, "height": 100})).unwrap();

crates/ui-egui/src/shortcut_editor.rs

@@ -485,10 +485,7 @@ pub fn run_command(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option<Resu
                 // Its save dialog is open: the file is written when it answers.
                 None if app.file_dialog_open() => Err("cancelled".into()),
                 None => match app.services.download.as_mut() {
-                    Some(dl) => {
-                        dl("VectorCraft Shortcuts.json", &bytes);
-                        Ok(Value::Null)
-                    }
+                    Some(dl) => dl("VectorCraft Shortcuts.json", &bytes).map(|()| Value::Null),
                     None => Ok(v),
                 },
             }

crates/ui-egui/src/tests_nativeoptions.rs

@@ -86,7 +86,10 @@ fn the_web_save_as_dialog_shows_the_native_options() {
     let written = crate::tests_svg::Written::default();
     let w = written.clone();
     let services = crate::Services {
-        download: Some(Box::new(move |name: &str, b: &[u8]| w.borrow_mut().push((name.to_string(), b.to_vec())))),
+        download: Some(Box::new(move |name: &str, b: &[u8]| {
+            w.borrow_mut().push((name.to_string(), b.to_vec()));
+            Ok(())
+        })),
         ..Default::default()
     };
     let mut app = crate::VectorcraftApp::new(vectorcraft_engine::Session::new(), services);