Merge PR #785

# Conflicts:
#	crates/engine/src/cmd/export.rs
#	crates/epub/src/lib.rs

crates/engine/src/cmd/export.rs

@@ -12,7 +12,7 @@ pub fn specs() -> Vec<CommandSpec> {
         "{path?, pages?: \"1-3,5\" | [1,3] (1-based positions; default all), flatten?: high|medium|low|ppi (Transparency Flattener: spreads with transparency are rasterised), spreads?: bool, fullScreen?: bool, bookmarksPanel?: bool, pageLayout?: single|continuous|twoUp|twoUpCover|twoUpContinuous, view?: fitPage|fitWidth|actual, advanceSeconds?: number (interactive PDF), bleed?: bool (document bleed), marks?: bool | {crop?, bleed?, pageInfo?, weight?, offset?}, standard?: \"none\"|\"x4\"|\"a2b\", compressImages?: bool, tagged?: bool (structure tree: stories as paragraphs, figures with alt text), media?: bool (interactive: embed placed video and sound), title?, author?} → {path, bytes, pages, warnings} (no path: {base64, …})",
         has_doc, export_pdf),
         cmd!(noundo "file.exportEpub", "Export EPUB (Reflowable)…", ["File"], None,
-        "{path?, title?, author?, language?: \"en\", tocTitle?: str (the table of contents' title; from the language by default), cover?: bool (the first page as the cover image), fixedLayout?: bool (pre-paginated: each page as an image with its text)} → {path, bytes} (no path: {base64, bytes})",
+        "{path?, title?, author?, language?: \"en\", tocTitle?: str (the table of contents' title; from the language by default), cover?: bool (the first page as the cover image), fixedLayout?: bool (pre-paginated: each page as an image with its text)} → {path, bytes, warnings} (no path: {base64, bytes, warnings}); warnings name images without alt text, exported as decorative",
         has_doc, export_epub),
         cmd!(noundo "file.exportFixedEpub", "Export EPUB (Fixed Layout)…", ["File"], None,
         "{path?, title?, author?, language?} — pre-paginated EPUB → {path, bytes} (no path: {base64, bytes})",
@@ -158,13 +158,17 @@ fn export_epub(s: &mut Session, p: &Value) -> Result<Value> {
         };
     }
     let bytes = designcraft_epub::export_epub(epub_doc, &opts).map_err(|e| crate::EngineError::Other(e.to_string()))?;
+    let warnings: Vec<String> = designcraft_epub::images_without_alt(epub_doc)
+        .into_iter()
+        .map(|name| format!("{name}: no alt text (Object Export Options); exported as decorative (alt=\"\")"))
+        .collect();
     match p.get("path").and_then(Value::as_str) {
         Some(path) => {
             #[cfg(not(target_arch = "wasm32"))]
             crate::io::write_file(path, &bytes).map_err(crate::EngineError::Other)?;
-            Ok(serde_json::json!({"path": path, "bytes": bytes.len()}))
+            Ok(serde_json::json!({"path": path, "bytes": bytes.len(), "warnings": warnings}))
         }
-        None => Ok(serde_json::json!({"base64": super::file::base64_encode(&bytes), "bytes": bytes.len()})),
+        None => Ok(serde_json::json!({"base64": super::file::base64_encode(&bytes), "bytes": bytes.len(), "warnings": warnings})),
     }
 }
 
@@ -660,6 +664,7 @@ mod text_tests {
         s.execute("file.new", &json!({"pages": 2})).unwrap();
         s.execute("frame.create", &json!({"rect": [72, 72, 400, 200], "content": "text", "text": "Page one words", "caret": false})).unwrap();
         let r = s.execute("file.exportEpub", &json!({"cover": true})).unwrap();
+        assert_eq!(r["warnings"], json!([]), "no images, nothing to report");
         let mut z = unzip(r["base64"].as_str().unwrap());
         let opf = read(&mut z, "OEBPS/content.opf");
         assert!(opf.contains("cover-image") && opf.contains("<itemref idref=\"cover\"/>"));

crates/epub/src/lib.rs

@@ -1005,6 +1005,17 @@ pub fn reading_order(doc: &Document) -> Vec<Block> {
     keyed.into_iter().map(|(_, b)| b).collect()
 }
 
+/// Package path of an asset: `{dir}/{name}-{id}.{ext}`, the name from its original file name
+/// (lowercase ASCII, so the package stays readable) and the id keeping two assets of the same
+/// name apart.
+fn asset_path(dir: &str, a: &designcraft_doc::Asset, mime: &str) -> String {
+    let stem = a.name.rsplit(['/', '\\']).next().unwrap_or("");
+    let stem = stem.rsplit_once('.').map_or(stem, |(s, _)| s);
+    let name: String = stem.chars().map(|c| if c.is_ascii_alphanumeric() { c.to_ascii_lowercase() } else { '-' }).collect();
+    let name: String = name.split('-').filter(|p| !p.is_empty()).collect::<Vec<_>>().join("-").chars().take(40).collect();
+    if name.is_empty() { format!("{dir}/{}.{}", a.id.0, ext(mime)) } else { format!("{dir}/{name}-{}.{}", a.id.0, ext(mime)) }
+}
+
 fn ext(mime: &str) -> &'static str {
     match mime {
         "image/jpeg" => "jpg",
@@ -1041,7 +1052,7 @@ fn graphic_html<'a>(
     let mime = safe_mime(&a.mime);
     // Video and sound: an HTML5 player (with the poster, when there is one).
     if let Some(kind) = designcraft_doc::media_kind(mime) {
-        let name = format!("media/{}.{}", g.asset.0, ext(mime));
+        let name = asset_path("media", a, mime);
         images.insert(name.clone(), (mime, a.data.as_slice()));
         let m = it.media.clone().unwrap_or_default();
         let mut attrs = String::new();
@@ -1052,7 +1063,7 @@ fn graphic_html<'a>(
         }
         if let Some(pa) = m.poster.and_then(|p| doc.assets.get(&p)) {
             let poster_mime = safe_mime(&pa.mime);
-            let pn = format!("images/{}.{}", pa.id.0, ext(poster_mime));
+            let pn = asset_path("images", pa, poster_mime);
             images.insert(pn.clone(), (poster_mime, pa.data.as_slice()));
             if kind == "video" {
                 attrs.push_str(&format!(" poster=\"{}\"", xml_attr(&img_src(&pn, poster_mime, &pa.data))));
@@ -1061,10 +1072,11 @@ fn graphic_html<'a>(
         let markup = format!("<{kind} src=\"{}\"{attrs}>{}</{kind}>", xml_attr(&img_src(&name, mime, &a.data)), xml_text(&a.name));
         return Some((markup, true));
     }
-    let name = format!("images/{}.{}", g.asset.0, ext(mime));
+    let name = asset_path("images", a, mime);
     images.insert(name.clone(), (mime, a.data.as_slice()));
-    // Alt text from Object Export Options, else the file name.
-    let alt = if it.alt_text.is_empty() { a.name.as_str() } else { it.alt_text.as_str() };
+    // Alt text from Object Export Options; without one the image counts as decorative
+    // (`alt=""`): a file name read aloud helps nobody.
+    let alt = it.alt_text.as_str();
     Some((format!("<img src=\"{}\" alt=\"{}\"/>", xml_attr(&img_src(&name, mime, &a.data)), xml_attr(alt)), false))
 }
 
@@ -1144,6 +1156,22 @@ fn body_html<'a>(
     (body, images, toc)
 }
 
+/// Placed images (not video or sound) without alt text, by asset name: they are exported as
+/// decorative (`alt=""`), which an export can report.
+pub fn images_without_alt(doc: &Document) -> Vec<String> {
+    let mut out = Vec::new();
+    for b in reading_order(doc) {
+        let Block::Image(iid) = b else { continue };
+        let Some(it) = doc.item(iid) else { continue };
+        let Content::Graphic(g) = &it.content else { continue };
+        let Some(a) = doc.assets.get(&g.asset) else { continue };
+        if !a.data.is_empty() && designcraft_doc::media_kind(safe_mime(&a.mime)).is_none() && it.alt_text.trim().is_empty() {
+            out.push(a.name.clone());
+        }
+    }
+    out
+}
+
 /// Options for [`export_html`].
 #[derive(Clone, Debug, Default)]
 pub struct HtmlOptions {
@@ -1355,10 +1383,10 @@ mod tests {
         let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap();
         let mut html = String::new();
         z.by_name("OEBPS/content.xhtml").unwrap().read_to_string(&mut html).unwrap();
-        let img = |alt: &str| format!("<img src=\"images/{}.png\" alt=\"{alt}\"/>", aid.0);
+        let img = |alt: &str| format!("<img src=\"images/dot-{}.png\" alt=\"{alt}\"/>", aid.0);
         assert!(html.contains(&format!("A{}B</p>", img("inline"))), "{html}");
         assert!(html.contains(&format!("C<span class=\"anchored\" style=\"display:block;text-align:center\">{}</span>D", img("above"))), "{html}");
-        assert!(z.by_name(&format!("OEBPS/images/{}.png", aid.0)).is_ok(), "the image is in the package");
+        assert!(z.by_name(&format!("OEBPS/images/dot-{}.png", aid.0)).is_ok(), "the image is in the package");
         let page = export_html(&d, &HtmlOptions::default());
         assert!(page.contains("alt=\"inline\"") && page.contains("alt=\"above\""), "{page}");
         // story_html on its own still leaves objects out.
@@ -1626,6 +1654,44 @@ mod tests {
         assert_eq!(base64(b"M"), "TQ==");
     }
 
+    /// #774: an image without alt text is decorative (`alt=""`) and reported, and package names
+    /// come from the original file names.
+    #[test]
+    fn images_get_readable_names_and_no_file_name_alt() {
+        let mut d = Document::new(&NewDocument::default());
+        let lid = d.default_layer();
+        let place = |d: &mut Document, name: &str, mime: &str, alt: &str, y: f64| {
+            let aid = AssetId(d.alloc());
+            d.assets.insert(
+                aid,
+                Arc::new(Asset { page: 0, id: aid, name: name.into(), mime: mime.into(), link: None, data: Arc::new(vec![1]), pixels: Some((1, 1)) }),
+            );
+            let mut item = Item::new(ItemId(d.alloc()), lid, Shape::Rectangle, shapes::rectangle(Rect::new(36.0, y, 72.0, y + 36.0)));
+            item.alt_text = alt.into();
+            item.content = Content::Graphic(Graphic {
+                asset: aid,
+                size: (1.0, 1.0),
+                xf: Affine::IDENTITY,
+                auto_fit: Default::default(),
+                fit_align: 4,
+                crop: [0.0; 4],
+            });
+            d.insert_item(SpreadRef::Doc(0), item, None).unwrap();
+            aid.0
+        };
+        let a = place(&mut d, "/tmp/Anim Frame (1).GIF", "image/gif", "", 36.0);
+        let b = place(&mut d, "photo.jpg", "image/jpeg", "A heron", 120.0);
+        let c = place(&mut d, "???.png", "image/png", "Mark", 200.0);
+        assert_eq!(images_without_alt(&d), vec!["/tmp/Anim Frame (1).GIF".to_string()]);
+        let mut z = zip::ZipArchive::new(std::io::Cursor::new(export_epub(&d, &EpubOptions::default()).unwrap())).unwrap();
+        let mut x = String::new();
+        z.by_name("OEBPS/content.xhtml").unwrap().read_to_string(&mut x).unwrap();
+        assert!(x.contains(&format!("<img src=\"images/anim-frame-1-{a}.gif\" alt=\"\"/>")), "{x}");
+        assert!(x.contains(&format!("<img src=\"images/photo-{b}.jpg\" alt=\"A heron\"/>")), "{x}");
+        assert!(x.contains(&format!("<img src=\"images/{c}.png\" alt=\"Mark\"/>")), "{x}");
+        assert!(z.by_name(&format!("OEBPS/images/anim-frame-1-{a}.gif")).is_ok());
+    }
+
     #[test]
     fn metadata_is_serialized_for_its_output_context() {
         let mut d = Document::new(&NewDocument::default());