diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 479d15b9c..940d24873 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -152,7 +152,7 @@ pub fn specs() -> Vec { "Move Object", [], None, - "{kind: chart|image|shape|slicer, id, at?: \"C3\", dx?, dy?, width?, height?}", + "{kind: chart|image|shape|slicer, id, at?: \"C3\", dx?, dy?, width?, height? (each at most 10,000 points, or the object's current size when a file made it larger)}", has_doc, move_object ), @@ -933,10 +933,10 @@ fn chart_set(s: &mut Session, p: &Json) -> Result { c.anchor.cell = at; } if let Some(w) = f64_param(p, "width") { - c.anchor.width = w.clamp(20.0, 10000.0) as f32; + c.anchor.width = w.clamp(20.0, size_cap(c.anchor.width)) as f32; } if let Some(h) = f64_param(p, "height") { - c.anchor.height = h.clamp(20.0, 10000.0) as f32; + c.anchor.height = h.clamp(20.0, size_cap(c.anchor.height)) as f32; } if let Some(v) = f64_param(p, "dx") { c.anchor.dx = v as f32; @@ -1248,6 +1248,18 @@ fn delete_object(s: &mut Session, p: &Json) -> Result { /// Largest offset (points) `object.move` accepts from the anchor cell. const MAX_OBJECT_OFFSET: f64 = 100_000.0; +/// Largest width or height (points) `object.move` and `chart.set` give an object that is not +/// already larger: see [`size_cap`]. +const MAX_OBJECT_SIZE: f64 = 10_000.0; + +/// The most a command may make a dimension that is `current` now: [`MAX_OBJECT_SIZE`], or the +/// current size when a file made the object larger (the XLSX reader accepts anchors far past the +/// limit). Such an object keeps its size when it is moved and shrinks by what a resize asks, but +/// it never grows further, so hostile input is still capped. Never NaN: `f64::max` drops one. +fn size_cap(current: f32) -> f64 { + MAX_OBJECT_SIZE.max(f64::from(current)) +} + fn move_object(s: &mut Session, p: &Json) -> Result { let (sheet, o) = object_param(s, p, "object.move")?; let at = cell_param(s, p, "at")?; @@ -1263,10 +1275,10 @@ fn move_object(s: &mut Session, p: &Json) -> Result { a.dy = v.clamp(-MAX_OBJECT_OFFSET, MAX_OBJECT_OFFSET) as f32; } if let Some(v) = f64_param(p, "width") { - a.width = v.clamp(1.0, 10000.0) as f32; + a.width = v.clamp(1.0, size_cap(a.width)) as f32; } if let Some(v) = f64_param(p, "height") { - a.height = v.clamp(0.0, 10000.0) as f32; + a.height = v.clamp(0.0, size_cap(a.height)) as f32; } Ok(Json::Null) }) diff --git a/crates/engine/src/tests/objects.rs b/crates/engine/src/tests/objects.rs index e590f4851..c10844fb8 100644 --- a/crates/engine/src/tests/objects.rs +++ b/crates/engine/src/tests/objects.rs @@ -301,3 +301,61 @@ fn object_ids_stay_unique_when_a_file_used_the_largest_id() { ids.dedup(); assert_eq!(ids.len(), 6, "every shape id is distinct across both sheets"); } + +/// A file's anchors are not held to the 10,000 points `object.move` gives a new size. A picture +/// a file made 12,000 wide keeps that width when it is moved, shrinks by what a resize asks +/// (11,990 stays 11,990, no jump to 10,000) and never grows further; a dimension the file kept +/// within the limit is still capped there. +#[test] +fn an_oversized_imported_object_keeps_its_size_when_moved_and_shrinks_only_by_a_resize() { + let mut s = s(); + let image = super::add_image(&mut s, "B2", AnchorMode::MoveOnly); + // The file's picture is 12,000 by 300: set on the model, then written and read back as a file. + { + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + let a = wb.sheet_mut(0).unwrap().object_anchor_mut(ObjRef::new(ObjKind::Image, image)).unwrap(); + a.width = 12_000.0; + a.height = 300.0; + } + let saved = s.execute("file.saveBytes", json!({"format": "xlsx"})).unwrap(); + s.execute("file.open", json!({"name": "wide.xlsx", "base64": saved["base64"]})).unwrap(); + let o = ObjRef::new(ObjKind::Image, sheet(&s, 0).images[0].id); + let size = |s: &Session| { + let a = sheet(s, 0).object_anchor(o).unwrap(); + (a.width.round(), a.height.round()) + }; + assert_eq!(size(&s), (12_000.0, 300.0), "the reader keeps the file's size"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "at": "D5", "dx": 3, "dy": 4})).unwrap(); + let a = sheet(&s, 0).object_anchor(o).unwrap(); + assert_eq!((a.cell, a.dx, a.dy), (at("D5"), 3.0, 4.0)); + assert_eq!(size(&s), (12_000.0, 300.0), "a move without a size keeps the file's size"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "width": 12_010})).unwrap(); + assert_eq!(size(&s), (12_000.0, 300.0), "the object does not grow past what the file gave"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "width": 11_990})).unwrap(); + assert_eq!(size(&s), (11_990.0, 300.0), "a resize shrinks by what it asks, with no jump to 10,000"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "width": 50_000, "height": 50_000})).unwrap(); + assert_eq!(size(&s), (11_990.0, 10_000.0), "hostile sizes are capped: at the current width, and at 10,000 for a height the file kept within it"); +} + +/// `chart.set` follows the same rule: the Format Chart pane's Size fields send the unchanged +/// dimension back with the edited one, so a chart a file made 12,000 wide keeps that width when +/// its height is edited, shrinks by what is asked and never grows further. +#[test] +fn chart_set_keeps_a_chart_size_a_file_made_larger_than_the_limit() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap(); + let chart = id_of(&s.execute("insert.chart", json!({"range": "A1:A3", "type": "column", "at": "C2"})).unwrap(), "chart"); + // The file's chart is 12,000 wide (set on the model, as a loaded file would). + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.sheet_mut(0).unwrap().charts.iter_mut().find(|c| c.id == chart).unwrap().anchor.width = 12_000.0; + let size = |s: &Session| { + let c = sheet(s, 0).charts.iter().find(|c| c.id == chart).unwrap(); + (c.anchor.width, c.anchor.height) + }; + s.execute("chart.set", json!({"chart": chart, "width": 12_000, "height": 250})).unwrap(); + assert_eq!(size(&s), (12_000.0, 250.0), "the unchanged width sent back with a new height stays"); + s.execute("chart.set", json!({"chart": chart, "width": 11_990})).unwrap(); + assert_eq!(size(&s), (11_990.0, 250.0), "a resize shrinks by what it asks"); + s.execute("chart.set", json!({"chart": chart, "width": 50_000, "height": 50_000})).unwrap(); + assert_eq!(size(&s), (11_990.0, 10_000.0), "hostile sizes are capped at the current width and at 10,000"); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index b5822da64..9c3d0bc13 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -94,6 +94,9 @@ them as their own parameters. `ui.inspect` reports the selected floating object as `selectedObject: {kind, id}` (or `null`), where `kind` is `chart`, `image`, `shape` or `slicer`; the same names go in the `kind` parameter of `object.delete`, `object.move`, `object.setAnchorMode` and the `arrange.*` commands. +`object.move` and `chart.set` cap `width` and `height` at 10,000 points, or at the object's +current size when a file made it larger: such an object keeps its size when it is moved and +shrinks by what a resize asks, but does not grow further. `selectedChart` remains for existing clients: the id when the selection is a chart, otherwise `null`. With an object selected, `ui.key {key: "Delete"}` (or Backspace) deletes the object instead of clearing cells, and Escape clears the selection.