diff --git a/Cargo.lock b/Cargo.lock index 7e5ebb85..05729755 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1160,6 +1160,7 @@ dependencies = [ "unicode-bidi", "unicode-bidi-mirroring", "unicode-segmentation", + "zip", ] [[package]] diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 97ded38f..ed126eee 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -18,7 +18,7 @@ pub fn specs() -> Vec { cmd!(noundo "file.newSample", "Sample Document", ["Help"], None, "{} — a multi-page magazine sample", always, file_sample), cmd!(query "file.presets", "Document Presets", [], None, "{}", always, |_, _| Ok(serde_json::to_value(PRESETS).unwrap_or_default())), cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), - "{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped}", + "{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped (and, for .idml, package parts missing)}", always, file_open), cmd!(noundo "file.openBytes", "Open Bytes", [], None, "{name, base64} — DesignCraft JSON or an IDML package", always, file_open_bytes), cmd!(noundo "file.save", "Save", ["File"], Some("Cmd+S"), "{path?}", has_doc, file_save), diff --git a/crates/engine/src/cmd/interchange.rs b/crates/engine/src/cmd/interchange.rs index 8a65ec91..a0af09f5 100644 --- a/crates/engine/src/cmd/interchange.rs +++ b/crates/engine/src/cmd/interchange.rs @@ -13,7 +13,7 @@ pub fn specs() -> Vec { "{path?, embedImages?: true} — writes an IDML package to `path`, or returns {base64} without a path", has_doc, export_idml), cmd!(noundo "file.openIdml", "Open IDML", [], None, - "{path | base64, name?} — opens an IDML package as a new document (linked images are read next to the file or from its Links/ folder; the fonts in a `Document Fonts` folder beside it load first) → {index, documentFonts, warnings}", + "{path | base64, name?} — opens an IDML package as a new document (linked images are read next to the file or from its Links/ folder; the fonts in a `Document Fonts` folder beside it load first) → {index, documentFonts, warnings: package parts missing, font files skipped}", always, open_idml), ] } @@ -34,6 +34,11 @@ fn export_idml(s: &mut Session, p: &Value) -> Result { /// Import IDML bytes; `dir` is the folder the package came from (for relative link lookup). pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result { + import_report(bytes, dir).map(|(document, _)| document) +} + +/// [`import`], with the problems that did not stop it (parts missing from the package, …). +pub fn import_report(bytes: &[u8], dir: Option<&std::path::Path>) -> Result<(Document, Vec)> { #[cfg(not(target_arch = "wasm32"))] let resolved = std::cell::RefCell::new(std::collections::HashMap::new()); let read = |link: &str| -> Option> { @@ -49,7 +54,8 @@ pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result { None } }; - let document = designcraft_idml::import_idml_with(bytes, &read).map_err(|e| EngineError::Other(e.to_string()))?; + let designcraft_idml::Imported { document, warnings } = + designcraft_idml::import_idml_report(bytes, &read).map_err(|e| EngineError::Other(e.to_string()))?; #[cfg(not(target_arch = "wasm32"))] let document = { let mut document = document; @@ -62,7 +68,7 @@ pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result { resolve_packaged_links(&mut document, dir); document }; - Ok(document) + Ok((document, warnings)) } #[cfg(not(target_arch = "wasm32"))] @@ -142,14 +148,15 @@ pub(crate) fn open_idml(s: &mut Session, p: &Value) -> Result { } else { return Err(bad("file.openIdml", "missing `path` or `base64`")); }; - let mut d = import(&bytes, dir.as_deref())?; + let (mut d, mut warnings) = import_report(&bytes, dir.as_deref())?; if let Some(n) = name { d.title = n; } - let (fonts, faces, warnings) = match str_param(p, "path").filter(|_| str_param(p, "base64").is_none()) { + let (fonts, faces, font_warnings) = match str_param(p, "path").filter(|_| str_param(p, "base64").is_none()) { Some(path) => super::file::load_document_fonts(&mut d, path), None => (None, 0, Vec::new()), }; + warnings.extend(font_warnings); // Never save over the .idml with the native format: the document starts unsaved. let mut st = DocState::new(d, None); st.fonts = fonts; diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 13877526..46c3253b 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -48,8 +48,24 @@ fn default_reader(path: &str) -> Option> { /// Import with a custom reader for linked files (`path` → bytes). pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option>) -> Result { + import_idml_report(bytes, read_link).map(|r| r.document) +} + +/// An imported IDML document and what the import could not bring in. +#[derive(Debug)] +pub struct Imported { + pub document: Document, + /// One line per problem that did not stop the import (a part listed in `designmap.xml` but + /// absent from the package, …). + pub warnings: Vec, +} + +/// Import with a custom reader for linked files, reporting the problems that did not stop it. +pub fn import_idml_report(bytes: &[u8], read_link: &dyn Fn(&str) -> Option>) -> Result { let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).map_err(|e| IdmlError::NotIdml(e.to_string()))?; let mut files: HashMap> = HashMap::new(); + // Normalised part name → entry name, first entry wins (zip order, so the choice is stable). + let mut lenient: HashMap = HashMap::new(); for i in 0..zip.len() { let mut f = zip.by_index(i).map_err(|e| IdmlError::NotIdml(e.to_string()))?; if f.is_dir() { @@ -59,35 +75,81 @@ pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option // The size is what the archive claims: don't reserve more than a sane part up front. let mut buf = Vec::with_capacity((f.size() as usize).min(1 << 24)); f.read_to_end(&mut buf).map_err(|e| IdmlError::Part { part: name.clone(), msg: e.to_string() })?; + lenient.entry(normalize_part_name(&name)).or_insert_with(|| name.clone()); files.insert(name, buf); } - if let Some(m) = files.get("mimetype") + let part = |name: &str| files.get(name).or_else(|| lenient.get(&normalize_part_name(name)).and_then(|n| files.get(n))); + if let Some(m) = part("mimetype") && String::from_utf8_lossy(m).trim() != MIMETYPE { return Err(IdmlError::NotIdml(format!("unexpected mimetype `{}`", String::from_utf8_lossy(m).trim()))); } - let dm = files.get("designmap.xml").ok_or_else(|| IdmlError::NotIdml("missing designmap.xml".into()))?; + let dm = part("designmap.xml").ok_or_else(|| IdmlError::NotIdml("missing designmap.xml".into()))?; let root = parse(dm).map_err(|msg| IdmlError::Part { part: "designmap.xml".into(), msg })?; if root.local() != "Document" { return Err(IdmlError::NotIdml(format!("designmap root is <{}>", root.name))); } // Flatten includes: every `idPkg:*` child is replaced by the children of its part's root. let mut top: Vec = Vec::new(); + let mut warnings = Vec::new(); for c in root.elements() { if c.name.starts_with("idPkg:") { let Some(src) = c.get("src") else { continue }; - let Some(data) = files.get(src) else { continue }; - let part = parse(data).map_err(|msg| IdmlError::Part { part: src.into(), msg })?; - top.extend(part.elements().cloned()); + let Some(data) = part(src) else { + warnings.push(format!("{src}: listed in designmap.xml but missing from the package; its contents were not imported")); + continue; + }; + let el = parse(data).map_err(|msg| IdmlError::Part { part: src.into(), msg })?; + top.extend(el.elements().cloned()); } else { top.push(c.clone()); } } let mut im = Importer::new(read_link); im.run(&root, &top)?; - let d = im.finish(&root)?; - d.check().map_err(|e| IdmlError::Invalid(e.to_string()))?; - Ok(d) + let document = im.finish(&root).map_err(|e| match e { + // Without its spread parts there is nothing to open: say which parts were missing. + IdmlError::Invalid(msg) if !warnings.is_empty() => IdmlError::Invalid(format!("{msg} ({})", warnings.join("; "))), + e => e, + })?; + document.check().map_err(|e| IdmlError::Invalid(e.to_string()))?; + Ok(Imported { document, warnings }) +} + +/// A package part name in the form the lookup compares: InDesign and other IDML writers differ +/// in percent-encoding, case, path separators and a leading `./` or `/`. +fn normalize_part_name(name: &str) -> String { + let decoded = percent_decode(&name.replace('\\', "/")); + let mut rest = decoded.as_str(); + loop { + if let Some(r) = rest.strip_prefix("./") { + rest = r; + } else if let Some(r) = rest.strip_prefix('/') { + rest = r; + } else { + break; + } + } + rest.to_lowercase() +} + +/// Decode `%XX` escapes; anything else (including a malformed escape) is kept as written. +pub(crate) fn percent_decode(s: &str) -> String { + let b = s.as_bytes(); + let mut out = Vec::with_capacity(b.len()); + let mut i = 0; + while let Some(&c) = b.get(i) { + if c == b'%' + && let (Some(h), Some(l)) = (b.get(i + 1).and_then(|h| (*h as char).to_digit(16)), b.get(i + 2).and_then(|l| (*l as char).to_digit(16))) + { + out.push((h * 16 + l) as u8); + i += 3; + continue; + } + out.push(c); + i += 1; + } + String::from_utf8_lossy(&out).into_owned() } struct ItemCtx { @@ -2560,21 +2622,9 @@ fn path_of(pg: &El) -> PathData { pub(crate) fn uri_to_path(uri: &str) -> String { let rest = uri.strip_prefix("file://").or_else(|| uri.strip_prefix("file:")).unwrap_or(uri); // `file:///C:/…` → `/C:/…` → `C:/…` - let rest = if rest.len() > 3 && rest.starts_with('/') && rest.as_bytes()[2] == b':' { &rest[1..] } else { rest }; - let mut out = Vec::with_capacity(rest.len()); - let b = rest.as_bytes(); - let mut i = 0; - while i < b.len() { - if b[i] == b'%' - && i + 2 < b.len() - && let Ok(v) = u8::from_str_radix(&rest[i + 1..i + 3], 16) - { - out.push(v); - i += 3; - continue; - } - out.push(b[i]); - i += 1; - } - String::from_utf8_lossy(&out).to_string() + let rest = match rest.as_bytes() { + [b'/', _, b':', _, ..] => rest.get(1..).unwrap_or(rest), + _ => rest, + }; + percent_decode(rest) } diff --git a/crates/idml/src/lib.rs b/crates/idml/src/lib.rs index 71d68825..0526070d 100644 --- a/crates/idml/src/lib.rs +++ b/crates/idml/src/lib.rs @@ -49,7 +49,7 @@ mod xml; mod tests; pub use export::{ExportOptions, export_idml, export_idml_with}; -pub use import::{import_idml, import_idml_with}; +pub use import::{Imported, import_idml, import_idml_report, import_idml_with}; /// The IDML package mimetype (content of the first, stored `mimetype` entry). pub const MIMETYPE: &str = "application/vnd.adobe.indesign-idml-package"; diff --git a/crates/idml/src/tests.rs b/crates/idml/src/tests.rs index 4c781700..23ec99bd 100644 --- a/crates/idml/src/tests.rs +++ b/crates/idml/src/tests.rs @@ -428,6 +428,60 @@ fn imports_scalar_insets_in_object_style_without_replacing_frame_override() { assert_eq!(frame_inset(&d), [3.0; 4]); } +/// A one-page spread part `Self` with one rectangle. +fn one_frame_spread(id: &str) -> String { + format!( + r#" + + + + + + + + + + "# + ) +} + +/// Writers differ in how a designmap `src` spells its part: percent-encoded, another case, +/// backslashes, a leading `./`. Those resolve; a part absent from the package is a warning that +/// names it, not silently dropped objects. +#[test] +fn designmap_parts_match_leniently_and_missing_ones_are_reported() { + let designmap = r#" + + + + "#; + let zip = zip_files(&[ + ("designmap.xml", designmap), + ("Spreads/Spread_u1a b.xml", &one_frame_spread("u1")), + ("Spreads/Spread_u2.xml", &one_frame_spread("u2")), + ]); + let r = import_idml_report(&zip, &|_| None).unwrap(); + assert_eq!(r.document.spreads.len(), 2, "both lenient spellings resolve"); + assert!(r.document.spreads.iter().all(|sp| sp.items.len() == 1), "the spreads keep their frames"); + assert_eq!(r.warnings.len(), 1, "{:?}", r.warnings); + assert!(r.warnings[0].contains("Spreads/Spread_gone.xml"), "{:?}", r.warnings); + + // With every spread part missing the import fails, and the error names the part. + let only_missing = r#" + + "#; + let err = import_idml(&zip_files(&[("designmap.xml", only_missing)])).unwrap_err(); + assert!(err.to_string().contains("Spreads/Spread_gone.xml"), "{err}"); +} + +#[test] +fn percent_decode_keeps_malformed_escapes() { + assert_eq!(import::percent_decode("a%20b%2"), "a b%2"); + assert_eq!(import::percent_decode("%é%zz"), "%é%zz"); + assert_eq!(import::uri_to_path("file:///C:/a%20b"), "C:/a b"); + assert_eq!(import::uri_to_path("file:/a%bé"), "/a%bé"); +} + #[test] fn rejects_non_idml() { assert!(import_idml(b"not a zip").is_err()); diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 92bff320..85090219 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -30,6 +30,7 @@ log = { workspace = true } [dev-dependencies] designcraft-idml = { workspace = true } +zip = { workspace = true } # Headless (offscreen wgpu) rendering of the whole window for UI screenshots: `examples/ui_shot.rs`. egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] } designcraft-fonts = { workspace = true, features = ["testing"] } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c003154a..3d94bdb0 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -812,6 +812,7 @@ impl DesignApp { /// Open (`file.open`, `file.openBytes`) or place (`file.place`) a file the user chose. A file /// that can't be opened or placed gets an alert with the reason, as well as the status line. + /// A successful import with warnings keeps the document and shows the warnings in an alert. pub fn open_file(&mut self, cmd: &str, params: Value) -> Result { let file = match params.get("path").and_then(Value::as_str) { Some(path) => std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()), @@ -821,6 +822,20 @@ impl DesignApp { if let Err(e) = &r { let title = if cmd == "file.place" { "Can't Place the File" } else { "Can't Open the File" }; self.ui.dialog = Some(dialogs::Dialog::new("alert", json!({"title": title, "file": file, "message": e}))); + } else if let Ok(result) = &r { + let warnings: Vec<&str> = result + .get("warnings") + .and_then(Value::as_array) + .into_iter() + .flatten() + .filter_map(Value::as_str) + .filter(|warning| !warning.is_empty()) + .collect(); + if !warnings.is_empty() { + self.status(format!("{file}: {}", warnings.join("; "))); + self.ui.dialog = + Some(dialogs::Dialog::new("alert", json!({"title": "Import Warnings", "file": file, "message": warnings.join("\n")}))); + } } r } @@ -1319,6 +1334,99 @@ mod tests { } } +#[cfg(test)] +mod import_warning_tests { + use super::*; + use std::io::Write; + + fn idml(missing_parts: bool) -> Vec { + let missing = if missing_parts { r#""# } else { "" }; + let map = format!( + r#"{missing}"# + ); + let spread = r#""#; + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let options = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + for (name, text) in [("mimetype", designcraft_idml::MIMETYPE), ("designmap.xml", &map), ("Spreads/one.xml", spread)] { + zip.start_file(name, options).unwrap(); + zip.write_all(text.as_bytes()).unwrap(); + } + zip.finish().unwrap().into_inner() + } + + fn app() -> DesignApp { + let mut app = DesignApp::new(Session::new(), Services { inbox: Some(Inbox::default()), ..Default::default() }); + app.run("file.new", json!({"title": "Existing work"})).unwrap(); + app.run("frame.create", json!({"rect": [10, 10, 50, 50]})).unwrap(); + app + } + + fn assert_partial_import_warning(app: &mut DesignApp, file: &str) { + let dialog = app.ui.dialog.as_ref().expect("partial import must show an alert"); + assert_eq!(dialog.id, "alert"); + assert_eq!(dialog.fields["title"], "Import Warnings"); + assert_eq!(dialog.fields["file"], file); + for part in ["Spreads/missing.xml", "Stories/missing.xml"] { + assert!(dialog.fields["message"].as_str().unwrap().contains(part), "every missing part is named"); + assert!(app.ui.status.contains(part), "the status line retains the warning"); + } + assert_eq!(app.session.documents().len(), 2); + assert_eq!(app.session.documents()[0].doc.title, "Existing work"); + assert!(app.session.documents()[0].is_dirty(), "existing unsaved work stays intact"); + assert_eq!(app.session.documents()[0].doc.spreads[0].items.len(), 1); + assert_eq!(app.session.doc().unwrap().doc.spreads.len(), 1); + assert_eq!(app.session.doc().unwrap().doc.spreads[0].items.len(), 1, "available document content is kept"); + dialogs::confirm(app).unwrap(); + assert!(app.ui.dialog.is_none(), "OK dismisses the warning"); + assert_eq!(app.session.documents().len(), 2, "dismissing the warning keeps the imported document"); + } + + #[test] + fn byte_open_reports_partial_import_warnings_without_losing_documents() { + let mut app = app(); + let result = + app.open_file("file.openBytes", json!({"name": "partial.idml", "base64": designcraft_engine::cmd::base64_encode(&idml(true))})).unwrap(); + assert_eq!(result["warnings"].as_array().unwrap().len(), 2); + assert_partial_import_warning(&mut app, "partial.idml"); + } + + #[test] + fn native_open_reports_partial_import_warnings_without_losing_documents() { + let path = std::env::temp_dir().join(format!("designcraft-partial-import-{}", std::process::id())).join("partial.idml"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, idml(true)).unwrap(); + let picked = path.to_string_lossy().to_string(); + let mut app = app(); + app.services.pick_open = Some(Box::new(move |_| Some(picked.clone()))); + let result = app.run("app.openDialog", json!({})); + std::fs::remove_file(&path).unwrap(); + std::fs::remove_dir(path.parent().unwrap()).unwrap(); + assert_eq!(result.unwrap()["warnings"].as_array().unwrap().len(), 2); + assert_partial_import_warning(&mut app, "partial.idml"); + } + + #[test] + fn inbox_open_reports_partial_import_warnings_without_losing_documents() { + let mut app = app(); + let request = app.import_request("open"); + app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request, name: "partial.idml".into(), bytes: idml(true) }); + app.drain_inbox(); + assert_partial_import_warning(&mut app, "partial"); + } + + #[test] + fn complete_import_does_not_show_a_warning() { + let mut app = app(); + let result = app + .open_file("file.openBytes", json!({"name": "complete.idml", "base64": designcraft_engine::cmd::base64_encode(&idml(false))})) + .unwrap(); + assert_eq!(result["warnings"], json!([])); + assert!(app.ui.dialog.is_none()); + assert!(app.ui.status.is_empty()); + assert_eq!(app.session.documents().len(), 2); + } +} + /// The whole window in a headless test harness, at a chosen window size. #[cfg(test)] pub(crate) mod test_window {