Merge save-result: fix(file): saving never reports success without a file (#741, #708)

This commit is contained in:
aiongg
2026-10-13 14:27:03 +00:00
13 changed files with 128 additions and 33 deletions
+1
View File
@@ -128,6 +128,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m
Newest first.
- **2026-10-13** · Saving never reports success without a file: a document that can't be written is an error from Save, Save As, Save a Copy, Package, snippet export and the web serializer, leaving the existing file, the unsaved state and the recovery checkpoint as they were. Placed files over 4 GiB are written with zip64 sizes. A web Save whose browser download can't start reports the error and keeps the document unsaved (#741, #708). No readiness percentage change.
- **2026-10-13** · Colour management runs on craft-libs' `craft-cms`. The default CMYK working space is its Coated FOGRA39 profile (Generic CMYK and Device CMYK stay selectable); CMYK swatches show relative colorimetric with the settings' black point compensation, and PDF/X-4 output intents embed the working CMYK profile (FOGRA39 by default). Profile names the built-in stands in for resolve to it; other unknown names are an error, and saved settings naming one start with the default. No readiness percentage change.
- **2026-10-13** · Command scripts look up only a well-formed `$N.path` / `$last.path` reference; other strings starting with `$`, such as a GREP replacement of `$1` and a nonbreaking space, stay as written, and `$$` writes a literal `$` (#749). No readiness percentage change.
- **2026-10-13** · Dialogs and the About window move when you drag their title row, so the page behind them can be seen. A dragged dialog may go partly off-screen, but its title row stays reachable; each dialog opens centred, and again after the window is resized. No readiness percentage change.
+5 -6
View File
@@ -140,11 +140,7 @@ fn services(inbox: Inbox, ctx: egui::Context) -> Services {
// Exports ask for a name; the browser decides where the download goes.
pick_save: Some(Box::new(|name: &str| Some(name.to_string()))),
write: Some(Box::new(|name: &str, bytes: &[u8]| download(name, bytes))),
download: Some(Box::new(|name: &str, bytes: &[u8]| {
if let Err(e) = download(name, bytes) {
log::error!("download of {name} failed: {e}");
}
})),
download: Some(Box::new(|name: &str, bytes: &[u8]| download(name, bytes))),
inbox: Some(inbox),
..Default::default()
}
@@ -173,6 +169,9 @@ 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 missed revoke only keeps the blob until the page closes.
if let Err(e) = window.set_timeout_with_callback_and_timeout_and_arguments_0(revoke.unchecked_ref(), 10_000) {
log::warn!("could not schedule revoking the download URL: {}", js(e));
}
Ok(())
}
+52 -7
View File
@@ -49,7 +49,7 @@ pub fn specs() -> Vec<CommandSpec> {
let path = with_document_extension(path);
let mut copy = (*s.doc()?.doc).clone();
super::datamerge::refresh_relative_paths(&mut copy, std::path::Path::new(&path));
let bytes = to_bytes(&copy);
let bytes = to_bytes(&copy)?;
#[cfg(not(target_arch = "wasm32"))]
crate::atomic::write_file(&path, &bytes)?;
Ok(json!({"path": path, "bytes": bytes.len()}))
@@ -71,7 +71,7 @@ pub fn specs() -> Vec<CommandSpec> {
let st = s.doc()?;
// Preview fill is session state. Serialize the unfilled template.
let doc = st.preview_stash.as_deref().unwrap_or(st.doc.as_ref());
let bytes = to_bytes(doc);
let bytes = to_bytes(doc)?;
Ok(json!({"base64": base64_encode(&bytes), "bytes": bytes.len()}))
}),
cmd!(noundo "file.close", "Close", ["File"], Some("Cmd+W"), "{index?}", has_doc, |s, p| {
@@ -131,8 +131,19 @@ pub fn specs() -> Vec<CommandSpec> {
]
}
pub fn to_bytes(d: &Document) -> Vec<u8> {
designcraft_format::save(d).unwrap_or_default()
#[cfg(test)]
thread_local! {
/// Makes [`to_bytes`] fail on this thread, standing in for writer errors that need gigabytes to reach.
pub(crate) static FAIL_TO_BYTES: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
}
/// The document as `.designcraft` bytes, or an error when it can't be written; never an empty file.
pub fn to_bytes(d: &Document) -> Result<Vec<u8>> {
#[cfg(test)]
if FAIL_TO_BYTES.with(std::cell::Cell::get) {
return Err(EngineError::Other("could not write the document: injected failure".into()));
}
designcraft_format::save(d).map_err(|e| EngineError::Other(format!("could not write the document: {e}")))
}
/// `path` ending in `.designcraft`: the extension is added when it is missing or another one, so
@@ -448,7 +459,7 @@ fn file_save(s: &mut Session, p: &Value) -> Result<Value> {
super::datamerge::refresh_relative_paths(&mut saved, std::path::Path::new(&path));
let paths_changed = saved.data_merge != st.doc.data_merge;
let saved = if paths_changed { Arc::new(saved) } else { st.doc.clone() };
let bytes = to_bytes(&saved);
let bytes = to_bytes(&saved)?;
#[cfg(not(target_arch = "wasm32"))]
crate::atomic::write_file(&path, &bytes)?;
if paths_changed {
@@ -777,12 +788,12 @@ fn place_drop(s: &mut Session, p: &Value) -> Result<Value> {
/// The selection as `.designcraft` snippet bytes.
pub(crate) fn snippet_bytes(s: &mut Session) -> Result<Vec<u8>> {
Ok(to_bytes(&super::edit::clip_doc(s)?))
to_bytes(&super::edit::clip_doc(s)?)
}
fn snippet_export(s: &mut Session, p: &Value) -> Result<Value> {
let d = super::edit::clip_doc(s)?;
let bytes = to_bytes(&d);
let bytes = to_bytes(&d)?;
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
@@ -1452,6 +1463,40 @@ mod save_tests {
assert!(e.to_string().contains("missing"), "{e}");
let _ = std::fs::remove_dir_all(&dir);
}
/// A document that can't be written is an error: the file on disk, the unsaved state and the
/// recovery checkpoint all stay as they were.
#[test]
fn a_save_that_cannot_write_the_document_changes_nothing() {
let dir = std::env::temp_dir().join(format!("dc-save-fails-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join("doc.designcraft");
let recovery = dir.join("recovery");
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
s.execute("file.save", &json!({"path": path.to_string_lossy()})).unwrap();
let before = std::fs::read(&path).unwrap();
s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text", "text": "Unsaved", "caret": false})).unwrap();
s.execute("file.recovery.save", &json!({"dir": recovery.to_string_lossy()})).unwrap();
let checkpoint = recovery.join(format!("{}.designcraft", s.doc().unwrap().uid));
assert!(checkpoint.exists());
super::FAIL_TO_BYTES.with(|f| f.set(true));
let save = s.execute("file.save", &json!({}));
let copy = s.execute("file.saveACopy", &json!({"path": dir.join("copy.designcraft").to_string_lossy()}));
let serialized = s.execute("file.serialize", &json!({}));
super::FAIL_TO_BYTES.with(|f| f.set(false));
let e = save.unwrap_err();
assert!(e.to_string().contains("could not write the document"), "{e}");
assert!(copy.is_err() && serialized.is_err(), "no empty copy and no empty download");
assert_eq!(std::fs::read(&path).unwrap(), before, "the saved file is untouched");
assert!(!dir.join("copy.designcraft").exists());
assert!(s.doc().unwrap().is_dirty(), "the document still has unsaved changes");
assert!(checkpoint.exists(), "the recovery checkpoint stays");
let _ = std::fs::remove_dir_all(&dir);
}
}
#[cfg(test)]
+1 -1
View File
@@ -1727,7 +1727,7 @@ mod parent_base_tests {
let (a, b) = (d.parents[0].id, d.parents[1].id);
std::sync::Arc::make_mut(&mut d.parents[0]).parent.as_mut().unwrap().based_on = Some(b);
std::sync::Arc::make_mut(&mut d.parents[1]).parent.as_mut().unwrap().based_on = Some(a);
let b64 = crate::cmd::base64_encode(&crate::cmd::to_bytes(&d));
let b64 = crate::cmd::base64_encode(&crate::cmd::to_bytes(&d).unwrap());
let r = s.execute("file.openBytes", &json!({"name": "cycle", "base64": b64})).unwrap();
assert!(r["warnings"].to_string().contains("cycle"), "{r}");
let d = &s.doc().unwrap().doc;
+2 -2
View File
@@ -375,7 +375,7 @@ mod tests {
}
fn file_of(s: &Session) -> String {
super::super::file::base64_encode(&super::super::file::to_bytes(&s.doc().unwrap().doc))
super::super::file::base64_encode(&super::super::file::to_bytes(&s.doc().unwrap().doc).unwrap())
}
/// A client's document: Body in the brand colour with a GREP style, Head and Quote based on
@@ -603,7 +603,7 @@ mod tests {
#[test]
fn styles_load_from_a_path() {
let path = std::env::temp_dir().join(format!("dc-load-styles-{}.designcraft", std::process::id()));
std::fs::write(&path, super::super::file::to_bytes(&client().doc().unwrap().doc)).unwrap();
std::fs::write(&path, super::super::file::to_bytes(&client().doc().unwrap().doc).unwrap()).unwrap();
let mut s = new_doc();
let r = s.execute("style.load", &json!({"path": path.to_str().unwrap(), "paragraph": [], "character": ["Small"]}));
std::fs::remove_file(&path).unwrap();
+1 -1
View File
@@ -320,7 +320,7 @@ pub(crate) fn ok() -> Result<Value> {
Ok(Value::Null)
}
pub fn file_bytes(d: &designcraft_doc::Document) -> Vec<u8> {
pub fn file_bytes(d: &designcraft_doc::Document) -> Result<Vec<u8>> {
file::to_bytes(d)
}
pub fn file_from(b: &[u8]) -> Result<designcraft_doc::Document> {
+1 -1
View File
@@ -383,7 +383,7 @@ fn package(s: &mut Session, p: &Value) -> Result<Value> {
let links_dir = root.child("Links")?;
let (packed, mut files) = write_links(&d, &root, &links_dir, None)?;
let doc_path = package_output_path(&dir, &title, "designcraft")?;
root.write(&doc_path, &super::to_bytes(&packed))?;
root.write(&doc_path, &super::to_bytes(&packed)?)?;
files.insert(0, doc_path);
// The font files, and what the report says about each font.
let db = designcraft_fonts::FontDb::global().scoped(d.font_scope);
+1 -1
View File
@@ -210,7 +210,7 @@ mod tests {
assert!(s.execute("type.pathOptions", &json!({"id": id, "end": "far"})).is_err());
// Saved with the document.
let d = s.doc().unwrap().doc.clone();
let back = crate::cmd::file_from(&crate::cmd::file_bytes(&d)).unwrap();
let back = crate::cmd::file_from(&crate::cmd::file_bytes(&d).unwrap()).unwrap();
let path = |d: &designcraft_doc::Document| d.item(designcraft_doc::ItemId(id)).unwrap().text_frame().unwrap().options.path.clone();
assert_eq!(path(&back), path(&d));
}
+3 -3
View File
@@ -278,7 +278,7 @@ fn sample_document_is_valid_and_composes() {
let i = s.execute("document.inspect", &json!({})).unwrap();
assert!(i["stories"].as_array().unwrap().len() >= 8);
// Save/load round trip.
let bytes = cmd::file_bytes(&s.doc().unwrap().doc);
let bytes = cmd::file_bytes(&s.doc().unwrap().doc).unwrap();
let back = cmd::file_from(&bytes).unwrap();
assert_eq!(back.page_count(), 4);
assert_eq!(back.assets.len(), s.doc().unwrap().doc.assets.len());
@@ -431,7 +431,7 @@ fn hyperlinks_and_bookmarks_reach_the_pdf() {
let pdf = hayro_syntax::Pdf::new(bytes).expect("valid pdf");
assert_eq!(pdf.pages().len(), 4);
// Round trip through the native format keeps them.
let back = cmd::file_from(&cmd::file_bytes(&s.doc().unwrap().doc)).unwrap();
let back = cmd::file_from(&cmd::file_bytes(&s.doc().unwrap().doc).unwrap()).unwrap();
assert_eq!(back.hyperlinks.len(), 2);
assert_eq!(back.bookmarks.len(), 2);
}
@@ -443,7 +443,7 @@ fn serialized_document_opens_again() {
s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text", "text": "Saved on the web"})).unwrap();
let r = s.execute("file.serialize", &json!({})).unwrap();
let bytes = cmd::base64_decode(r["base64"].as_str().expect("base64"));
assert_eq!(bytes, cmd::to_bytes(&s.doc().unwrap().doc));
assert_eq!(bytes, cmd::to_bytes(&s.doc().unwrap().doc).unwrap());
s.execute("file.openBytes", &json!({"name": "copy", "base64": r["base64"]})).unwrap();
let d = s.execute("document.inspect", &json!({})).unwrap();
assert_eq!(d["stories"][0]["preview"], "Saved on the web");
+5 -5
View File
@@ -170,7 +170,7 @@ fn file_roundtrip_keeps_tables() {
s.execute("table.insert", &json!({"rows": 2, "cols": 2})).unwrap();
s.execute("text.insert", &json!({"text": "A1"})).unwrap();
let d = s.doc().unwrap().doc.clone();
let bytes = crate::cmd::file_bytes(&d);
let bytes = crate::cmd::file_bytes(&d).unwrap();
let back = crate::cmd::file_from(&bytes).unwrap();
back.check().unwrap();
let st = back.story(designcraft_doc::StoryId(sid)).unwrap();
@@ -182,7 +182,7 @@ fn backspace_and_paste_between_tables_keep_both_and_the_file_reopens() {
let n = designcraft_doc::TABLE_ANCHOR.len_utf8();
let anchor_of = |s: &Session, sid: u64, tid: u64| s.doc().unwrap().doc.story(doc::StoryId(sid)).unwrap().table_anchor(tid).unwrap();
let reopens_with = |s: &Session, sid: u64, ids: &[u64]| {
let back = crate::cmd::file_from(&crate::cmd::file_bytes(&s.doc().unwrap().doc)).unwrap();
let back = crate::cmd::file_from(&crate::cmd::file_bytes(&s.doc().unwrap().doc).unwrap()).unwrap();
let st = back.story(doc::StoryId(sid)).unwrap();
assert!(ids.iter().all(|id| st.table_anchor(*id).is_some()), "{:?}", st.tables.keys());
assert_eq!(st.tables.len(), ids.len());
@@ -267,7 +267,7 @@ fn explicit_cell_edges_win_imported_priorities_and_keep_undo_and_file_roundtrip(
assert_eq!(table_for(&s, sid, tid), &before);
s.execute("edit.redo", &json!({})).unwrap();
assert_eq!(table_for(&s, sid, tid), &after);
let bytes = crate::cmd::file_bytes(&s.doc().unwrap().doc);
let bytes = crate::cmd::file_bytes(&s.doc().unwrap().doc).unwrap();
let back = crate::cmd::file_from(&bytes).unwrap();
assert_eq!(back.story(doc::StoryId(sid)).unwrap().tables.get(&tid).unwrap().as_ref(), &after);
check(&s);
@@ -468,7 +468,7 @@ fn cell_stroke_gap_colour_on_inner_horizontal_edges_of_a_selection_undoes_and_sa
assert!(s.execute("table.setCell", &json!({ "stroke": bad })).is_err(), "{bad}");
assert_eq!(table_for(&s, sid, tid), &t);
}
let bytes = crate::cmd::file_bytes(&s.doc().unwrap().doc);
let bytes = crate::cmd::file_bytes(&s.doc().unwrap().doc).unwrap();
let back = crate::cmd::file_from(&bytes).unwrap();
assert_eq!(back.story(doc::StoryId(sid)).unwrap().tables.get(&tid).unwrap().as_ref(), &t);
s.execute("edit.undo", &json!({})).unwrap();
@@ -682,7 +682,7 @@ fn split_selected_block_and_merged_cells_preserves_geometry_and_history() {
s.execute("edit.redo", &json!({})).unwrap();
assert!(std::sync::Arc::ptr_eq(&s.doc().unwrap().doc, &after_doc));
assert_eq!(s.doc().unwrap().selection, after_selection);
let bytes = crate::cmd::file_bytes(&s.doc().unwrap().doc);
let bytes = crate::cmd::file_bytes(&s.doc().unwrap().doc).unwrap();
let reopened = crate::cmd::file_from(&bytes).unwrap();
reopened.check().unwrap();
assert_eq!(reopened.story(doc::StoryId(sid)).unwrap().tables.get(&tid).unwrap().as_ref(), &after);
+23
View File
@@ -47,6 +47,14 @@ fn ext_for(mime: &str) -> &'static str {
/// Serialize a document to `.designcraft` bytes. The file holds each distinct placed file once
/// and leaves out assets nothing references; `doc` itself keeps them (undo can bring them back).
pub fn save(doc: &Document) -> Result<Vec<u8>, FormatError> {
save_with_large_entries_from(doc, LARGE_ENTRY)
}
/// Asset entries this size or larger are written with zip64 sizes. Zip rejects an entry over
/// 4 GiB written without them; the margin covers deflate output that is larger than its input.
const LARGE_ENTRY: usize = (u32::MAX as usize).saturating_sub(64 << 20);
fn save_with_large_entries_from(doc: &Document, large_entry: usize) -> Result<Vec<u8>, FormatError> {
let mut compact = doc.clone();
compact.compact_assets();
let doc = &compact;
@@ -70,6 +78,7 @@ pub fn save(doc: &Document) -> Result<Vec<u8>, FormatError> {
}
// Already-compressed images are stored as-is.
let opts = if matches!(a.mime.as_str(), "image/png" | "image/jpeg" | "image/gif" | "image/webp") { stored } else { deflate };
let opts = opts.large_file(a.data.len() >= large_entry);
z.start_file(format!("assets/{}.{}", id.0, ext_for(&a.mime)), opts).map_err(io)?;
z.write_all(&a.data).map_err(|e| FormatError::Io(e.to_string()))?;
}
@@ -471,6 +480,20 @@ mod tests {
assert!(now.styles.resolve_para(&now.story(sid).unwrap().paras[0]).0.drop_cap_align_left);
}
/// Assets over the zip size limit are written with zip64 sizes (scaled down here: the real
/// threshold is near 4 GiB); smaller ones are written exactly as before.
#[test]
fn large_assets_are_written_with_zip64_sizes_and_open_again() {
let mut d = Document::new(&NewDocument::default());
let aid = png_asset(&mut d, &[5; 64]);
place(&mut d, aid, 0.0);
let normal = save(&d).unwrap();
assert_eq!(normal, save_with_large_entries_from(&d, 65).unwrap(), "an asset under the threshold is unchanged");
let large = save_with_large_entries_from(&d, 64).unwrap();
assert!(large.len() > normal.len(), "the asset entry carries zip64 sizes");
assert_eq!(*load(&large).unwrap().assets[&aid].data, vec![5; 64]);
}
#[test]
fn saves_hold_each_used_file_once() {
let mut d = Document::new(&NewDocument::default());
+4 -3
View File
@@ -59,7 +59,8 @@ pub struct ImportedFile {
pub bytes: Vec<u8>,
}
pub type OpenAsyncFn = Box<dyn FnMut(ImportRequest)>;
pub type DownloadFn = Box<dyn FnMut(&str, &[u8])>;
/// Starts a browser download; an error means no file was handed to the user.
pub type DownloadFn = Box<dyn FnMut(&str, &[u8]) -> Result<(), String>>;
pub type ClipboardFn = Box<dyn FnMut() -> Option<String>>;
/// Files delivered asynchronously with their original operation and document identity.
pub type Inbox = std::sync::Arc<std::sync::Mutex<Vec<ImportedFile>>>;
@@ -2656,7 +2657,7 @@ mod browser_import_tests {
let mut source = Session::new();
source.execute("file.new", &json!({"title": "source"})).unwrap();
source.execute("frame.create", &json!({"rect": [10, 10, 50, 50], "content": "text", "text": "placed"})).unwrap();
let native = designcraft_engine::cmd::to_bytes(&source.doc().unwrap().doc);
let native = designcraft_engine::cmd::to_bytes(&source.doc().unwrap().doc).unwrap();
let idml = designcraft_idml::export_idml(&source.doc().unwrap().doc);
for (name, bytes) in [("source.designcraft", native), ("source.idml", idml)] {
let mut app = app();
@@ -2759,7 +2760,7 @@ mod browser_import_tests {
#[test]
fn drop_routing_and_nonlayout_open_remain_compatible() {
let mut app = app();
let native = designcraft_engine::cmd::to_bytes(&app.session.doc().unwrap().doc);
let native = designcraft_engine::cmd::to_bytes(&app.session.doc().unwrap().doc).unwrap();
let request = app.import_request("drop");
deliver(&mut app, request, "document.designcraft", native);
assert_eq!(app.session.documents().len(), 2);
+29 -3
View File
@@ -1699,9 +1699,8 @@ fn download_document(app: &mut DesignApp) -> Result<Value, String> {
let name = app.session.active().map(|d| format!("{}.designcraft", d.doc.title)).ok_or("no document")?;
let ser = app.run("file.serialize", json!({}))?;
let bytes = designcraft_engine::cmd::base64_decode(ser["base64"].as_str().unwrap_or_default());
if let Some(download) = app.services.download.as_mut() {
download(&name, &bytes);
}
let download = app.services.download.as_mut().ok_or("no download service")?;
download(&name, &bytes).map_err(|e| format!("could not download {name}: {e}"))?;
// Marks the document saved; on the web the engine doesn't touch a file system.
app.run("file.saveAs", json!({"path": name}))
}
@@ -4512,4 +4511,31 @@ mod tests {
assert!(entries.windows(2).all(|w| !matches!(w, [Item::Sep, Item::Sep])), "{title}: doubled separator");
}
}
/// Web save: a download that couldn't start is an error and leaves the document unsaved; one
/// that started marks it saved.
#[test]
fn web_save_marks_the_document_saved_only_after_the_download_starts() {
let dir = tempfile::tempdir().unwrap();
// The title names a temporary path: on native, file.saveAs writes where the web build doesn't.
let title = dir.path().join("doc").to_string_lossy().to_string();
let fails = std::rc::Rc::new(std::cell::Cell::new(true));
let f = fails.clone();
let services = crate::Services {
download: Some(Box::new(move |_: &str, _: &[u8]| if f.get() { Err("blocked".into()) } else { Ok(()) })),
..Default::default()
};
let mut app = DesignApp::new(designcraft_engine::Session::new(), services);
app.run("file.new", json!({"title": title})).unwrap();
app.run("frame.create", json!({"rect": [36, 36, 300, 200]})).unwrap();
assert!(app.session.active().unwrap().is_dirty());
let e = app.run("app.save", json!({})).unwrap_err();
assert!(e.contains("blocked"), "{e}");
assert!(app.session.active().unwrap().is_dirty(), "a failed download leaves the document unsaved");
fails.set(false);
app.run("app.save", json!({})).unwrap();
assert!(!app.session.active().unwrap().is_dirty());
}
}