mirror of
https://github.com/storytold/wordcraft.git
synced 2026-11-09 11:50:20 -06:00
ODT: a frame offering several picture formats shows one it can read (gap 19) (#5706)
* ODT gap 19: a frame offering several picture formats shows one it can read LibreOffice writes its own .svm metafile before a PNG of the same picture. WordCraft took the first whatever it was, so the picture was missing, and gone from the file once the paragraph was written from the model. Take the first it can read, as the frame's order of preference means. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ODT: a replacement picture before its object is kept (gap 19) A frame's picture WordCraft can't show was dropped before the object came, so a save from the model lost the object's replacement. Found by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ODT: a readable picture after an unreadable replacement is shown and kept (gap 19) With an unreadable replacement first, the readable picture after it was skipped: WordCraft drew a placeholder and a save from the model lost the picture. It is now shown and written after the replacement. A placeholder is still not written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Aleksander Helgaker
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
da914ce674
commit
595cddaaef
+3
-1
File diff suppressed because one or more lines are too long
@@ -128,6 +128,9 @@ pub(super) struct Body<'a> {
|
||||
paras_done: usize,
|
||||
/// `(width, height, picture bytes, alt text, equation read from an embedded `draw:object`)`.
|
||||
frame: Option<Frame>,
|
||||
/// The first picture the open frame offers that WordCraft can't show: an object coming after it
|
||||
/// keeps it as its replacement.
|
||||
frame_unread: Option<Vec<u8>>,
|
||||
in_desc: bool,
|
||||
/// The master pages' page setups.
|
||||
pages: &'a Pages,
|
||||
@@ -259,6 +262,7 @@ impl<'a> Body<'a> {
|
||||
pending_master: None,
|
||||
paras_done: 0,
|
||||
frame: None,
|
||||
frame_unread: None,
|
||||
in_desc: false,
|
||||
pages,
|
||||
sec: page.clone(),
|
||||
@@ -1323,6 +1327,7 @@ impl Body<'_> {
|
||||
let w = get(a, "svg:width").and_then(length);
|
||||
let h = get(a, "svg:height").and_then(length);
|
||||
self.frame = Some((w, h, None, String::new(), None, None, None));
|
||||
self.frame_unread = None;
|
||||
let g = get(a, "draw:style-name").map(|s| self.styles.graphic(s)).unwrap_or_default();
|
||||
self.frame_float = Some(super::frames::place(a, w.unwrap_or(0.0), h.unwrap_or(0.0), &g, self.text_size()));
|
||||
self.member_start(a, w.unwrap_or(0.0), h.unwrap_or(0.0), None);
|
||||
@@ -1429,12 +1434,25 @@ impl Body<'_> {
|
||||
&& fr.2.is_none()
|
||||
&& fr.4.is_none()
|
||||
&& fr.5.is_none()
|
||||
&& fr.6.as_ref().is_none_or(|o| o.replacement.is_none())
|
||||
{
|
||||
let path = href.trim_start_matches("./");
|
||||
fr.2 = (self.zip)(path);
|
||||
if let Some(o) = &mut fr.6 {
|
||||
o.replacement = fr.2.clone().map(std::sync::Arc::new);
|
||||
// A frame may offer one picture in several formats, in order of preference
|
||||
// (LibreOffice writes its own `.svm` metafile before a PNG): the first
|
||||
// WordCraft can read. An object's replacement is the first, kept whatever it
|
||||
// is, also when the object is still to come.
|
||||
if let Some(d) = (self.zip)(path) {
|
||||
let readable = drawing::readable_picture(&d);
|
||||
match &mut fr.6 {
|
||||
Some(o) if o.replacement.is_none() => o.replacement = Some(std::sync::Arc::new(d.clone())),
|
||||
Some(_) => {}
|
||||
None if !readable => {
|
||||
self.frame_unread.get_or_insert_with(|| d.clone());
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
if readable {
|
||||
fr.2 = Some(d);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1478,9 +1496,9 @@ impl Body<'_> {
|
||||
// A chart is kept whole too, to be written back as it was while it is unchanged.
|
||||
if fr.4.is_none() {
|
||||
fr.6 = super::objects::gather(href, ole, self.entries, self.manifest, &mut *self.zip);
|
||||
// A replacement picture read before the object.
|
||||
// A replacement picture read before the object: the first the frame offered.
|
||||
if let Some(o) = &mut fr.6 {
|
||||
o.replacement = fr.2.clone().map(std::sync::Arc::new);
|
||||
o.replacement = self.frame_unread.take().or_else(|| fr.2.clone()).map(std::sync::Arc::new);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -385,6 +385,11 @@ impl Body<'_> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether WordCraft can show these bytes as a picture (see the function below).
|
||||
pub(in crate::odt) fn readable_picture(data: &[u8]) -> bool {
|
||||
wordcraft_metafile::is_metafile(data) || model::sniff_image(data).is_some()
|
||||
}
|
||||
|
||||
/// A picture from a package's bytes: a raster image, or a Windows metafile (WMF, EMF: LibreOffice
|
||||
/// keeps clip art and logos so), sized `w` × `h` (points) when the frame says.
|
||||
pub(in crate::odt) fn picture(data: Vec<u8>, w: Option<f32>, h: Option<f32>, alt: &str) -> Option<model::Img> {
|
||||
|
||||
@@ -367,3 +367,33 @@ fn a_frame_keeps_its_place_in_the_stacking_order() {
|
||||
assert_eq!(float(&d).z, 0);
|
||||
assert!(crate::odt::tests::entry(&crate::odt::tests::export_model(&d).unwrap(), "content.xml").contains(r#"draw:z-index="0""#));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_frame_offering_several_formats_shows_the_first_it_can_read() {
|
||||
// LibreOffice's letter templates: its own metafile, then a PNG. Taking the first left nothing
|
||||
// to show, and the picture was gone once the paragraph was written from the model.
|
||||
let png = [b"\x89PNG\r\n\x1a\n".as_slice(), &[7u8; 64]].concat();
|
||||
let other = [b"\x89PNG\r\n\x1a\n".as_slice(), &[9u8; 64]].concat();
|
||||
let svm = b"VCLMTF a metafile WordCraft can't draw".to_vec();
|
||||
let frame = |images: &[&str]| {
|
||||
let images: String = images.iter().map(|h| format!(r#"<draw:image xlink:href="{h}"/>"#)).collect();
|
||||
format!(
|
||||
r#"<office:document-content {NS_T} {NS_D}><office:body><office:text><text:p><draw:frame svg:width="2in" svg:height="1in" text:anchor-type="as-char">{images}</draw:frame></text:p></office:text></office:body></office:document-content>"#
|
||||
)
|
||||
};
|
||||
let pictures = |d: &Document| d.body[0].as_para().unwrap().objects.iter().filter(|o| matches!(o, InlineObject::Image { .. })).count();
|
||||
let both = frame(&["Pictures/a.svm", "Pictures/a.png"]);
|
||||
let d = import(&zip_of(&[("content.xml", both.as_bytes()), ("Pictures/a.svm", &svm), ("Pictures/a.png", &png)])).unwrap();
|
||||
assert_eq!(pictures(&d), 1, "the PNG");
|
||||
let back = import(&crate::odt::tests::export_model(&d).unwrap()).unwrap();
|
||||
assert_eq!(pictures(&back), 1, "written from the model");
|
||||
// The first it can read, not the last.
|
||||
let order = frame(&["Pictures/a.png", "Pictures/b.png"]);
|
||||
let d = import(&zip_of(&[("content.xml", order.as_bytes()), ("Pictures/a.png", &png), ("Pictures/b.png", &other)])).unwrap();
|
||||
assert_eq!(pictures(&d), 1);
|
||||
assert_eq!(d.media.values().next().and_then(|m| m.get(8).copied()), Some(7), "the first");
|
||||
// Nothing it can read: no picture, and no failure.
|
||||
let only = frame(&["Pictures/a.svm"]);
|
||||
let d = import(&zip_of(&[("content.xml", only.as_bytes()), ("Pictures/a.svm", &svm)])).unwrap();
|
||||
assert_eq!(pictures(&d), 0);
|
||||
}
|
||||
|
||||
@@ -167,3 +167,84 @@ fn an_object_in_a_header_is_kept_too() {
|
||||
assert!(z.file_names().any(|n| n.ends_with("/content.xml") && n.starts_with("Object M")), "{:?}", z.file_names().collect::<Vec<_>>());
|
||||
assert!(entry(&saved, "styles.xml").contains("<draw:object xlink:href=\"./Object M"));
|
||||
}
|
||||
|
||||
/// A replacement picture may come before its object. It is kept whatever it is, though WordCraft
|
||||
/// can't show it, and a save from the model writes it back.
|
||||
#[test]
|
||||
fn a_replacement_picture_before_its_object_is_kept() {
|
||||
let content = format!(
|
||||
r#"<office:document-content {NS_T} {NS_D}><office:body><office:text><text:p>Sheet <draw:frame draw:name="Object1" text:anchor-type="as-char" svg:width="3in" svg:height="1in"><draw:image xlink:href="./ObjectReplacements/Object 1" xlink:type="simple"/><draw:object xlink:href="./Object 1" xlink:type="simple"/></draw:frame></text:p></office:text></office:body></office:document-content>"#
|
||||
);
|
||||
let bytes = zip_of(&[
|
||||
("META-INF/manifest.xml", MANIFEST.as_bytes()),
|
||||
("content.xml", content.as_bytes()),
|
||||
("Object 1/content.xml", SHEET.as_bytes()),
|
||||
("Object 1/styles.xml", b"<office:document-styles/>"),
|
||||
("ObjectReplacements/Object 1", METAFILE),
|
||||
]);
|
||||
let replacement = |bytes: &[u8]| {
|
||||
let flow = parse(bytes).unwrap();
|
||||
paras(&flow)
|
||||
.into_iter()
|
||||
.flat_map(|p| &p.inlines)
|
||||
.find_map(|i| if let Inline::Image(img) = i { img.object.clone() } else { None })
|
||||
.and_then(|o| o.replacement.clone())
|
||||
};
|
||||
assert_eq!(replacement(&bytes).as_deref().map(Vec::as_slice), Some(METAFILE));
|
||||
let saved = crate::odt::tests::export_model(&import(&bytes).unwrap()).unwrap();
|
||||
assert_eq!(replacement(&saved).as_deref().map(Vec::as_slice), Some(METAFILE), "written from the model");
|
||||
}
|
||||
|
||||
/// A readable picture after an unreadable replacement is what WordCraft shows, wherever the object
|
||||
/// comes between them, and a save from the model keeps both.
|
||||
#[test]
|
||||
fn a_readable_picture_after_an_unreadable_replacement_is_shown() {
|
||||
let mut png = std::io::Cursor::new(Vec::new());
|
||||
image::RgbImage::new(4, 4).write_to(&mut png, image::ImageFormat::Png).unwrap();
|
||||
let png = png.into_inner();
|
||||
let object = r#"<draw:object xlink:href="./Object 1"/>"#;
|
||||
let unread = r#"<draw:image xlink:href="./ObjectReplacements/Object 1"/>"#;
|
||||
let readable = r#"<draw:image xlink:href="./Pictures/a.png"/>"#;
|
||||
for frame in [format!("{unread}{object}{readable}"), format!("{object}{unread}{readable}")] {
|
||||
let content = format!(
|
||||
r#"<office:document-content {NS_T} {NS_D}><office:body><office:text><text:p>Sheet <draw:frame draw:name="Object1" text:anchor-type="as-char" svg:width="3in" svg:height="1in">{frame}</draw:frame></text:p></office:text></office:body></office:document-content>"#
|
||||
);
|
||||
let bytes = zip_of(&[
|
||||
("META-INF/manifest.xml", MANIFEST.as_bytes()),
|
||||
("content.xml", content.as_bytes()),
|
||||
("Object 1/content.xml", SHEET.as_bytes()),
|
||||
("Object 1/styles.xml", b"<office:document-styles/>"),
|
||||
("ObjectReplacements/Object 1", METAFILE),
|
||||
("Pictures/a.png", &png),
|
||||
]);
|
||||
let check = |bytes: &[u8], what: &str| {
|
||||
let flow = parse(bytes).unwrap();
|
||||
let img = paras(&flow)
|
||||
.into_iter()
|
||||
.flat_map(|p| &p.inlines)
|
||||
.find_map(|i| if let Inline::Image(img) = i { Some(img.clone()) } else { None })
|
||||
.unwrap();
|
||||
assert_eq!(img.data.as_slice(), png.as_slice(), "{what}: shows the picture it can read");
|
||||
assert_eq!(img.object.unwrap().replacement.as_deref().map(Vec::as_slice), Some(METAFILE), "{what}: keeps the replacement");
|
||||
};
|
||||
check(&bytes, "read");
|
||||
check(&crate::odt::tests::export_model(&import(&bytes).unwrap()).unwrap(), "written from the model");
|
||||
}
|
||||
}
|
||||
|
||||
/// With only an unreadable replacement, the placeholder drawn for it isn't written as a picture.
|
||||
#[test]
|
||||
fn the_placeholder_for_an_object_is_not_saved_as_its_picture() {
|
||||
let content = format!(
|
||||
r#"<office:document-content {NS_T} {NS_D}><office:body><office:text><text:p>Sheet <draw:frame draw:name="Object1" text:anchor-type="as-char" svg:width="3in" svg:height="1in"><draw:object xlink:href="./Object 1"/><draw:image xlink:href="./ObjectReplacements/Object 1"/></draw:frame></text:p></office:text></office:body></office:document-content>"#
|
||||
);
|
||||
let bytes = zip_of(&[
|
||||
("META-INF/manifest.xml", MANIFEST.as_bytes()),
|
||||
("content.xml", content.as_bytes()),
|
||||
("Object 1/content.xml", SHEET.as_bytes()),
|
||||
("Object 1/styles.xml", b"<office:document-styles/>"),
|
||||
("ObjectReplacements/Object 1", METAFILE),
|
||||
]);
|
||||
let saved = crate::odt::tests::export_model(&import(&bytes).unwrap()).unwrap();
|
||||
assert_eq!(entry(&saved, "content.xml").matches("<draw:image ").count(), 1);
|
||||
}
|
||||
|
||||
@@ -244,11 +244,19 @@ impl Writer {
|
||||
None => self.pictures.push((replacement.clone(), img.data.clone(), mime_of(&img.ext).to_string())),
|
||||
}
|
||||
let el = if o.ole { "draw:object-ole" } else { "draw:object" };
|
||||
format!(
|
||||
let mut inner = format!(
|
||||
"<{el} xlink:href=\"./{}\" xlink:type=\"simple\" xlink:show=\"embed\" xlink:actuate=\"onLoad\"/><draw:image xlink:href=\"./{}\" xlink:type=\"simple\" xlink:show=\"embed\" xlink:actuate=\"onLoad\"/>",
|
||||
x(&name),
|
||||
x(&replacement)
|
||||
)
|
||||
);
|
||||
// A picture shown that isn't the replacement (one WordCraft can't read) follows it; a
|
||||
// placeholder is WordCraft's own, not the file's.
|
||||
let other = o.replacement.as_ref().is_some_and(|r| r.as_slice() != img.data.as_slice());
|
||||
if other && crate::odt::objects::placeholder(&o.mime).as_deref() != Some(img.data.as_slice()) {
|
||||
let href = self.picture(img);
|
||||
inner.push_str(&format!("<draw:image xlink:href=\"{href}\" xlink:type=\"simple\" xlink:show=\"embed\" xlink:actuate=\"onLoad\"/>"));
|
||||
}
|
||||
inner
|
||||
}
|
||||
|
||||
/// Object `o` (a chart as read) in the package under the next object name, with its
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user